[Mesa-dev] [PATCH] nir/spirv: fix crashes when dereferencing a pointer for an OpVariable

[Jason Ekstrand]

On Tue, Aug 29, 2017 at 3:04 AM, Samuel Iglesias Gonsálvez <
siglesias at igalia.com> wrote:

> When creating a vtn_pointer for an OpVariable, the block_index and
> offsets fields are null because there is not ssa to take the data
> from.
>
> However, we can dereference that pointer when processing an
> SpvOp*AccessChain opcodes through vtn_ssa_offset_pointer_dereference()
> when the OpVariable when the StorageClass is Uniform or StorageBuffer.
>
> Inside vtn_ssa_offset_pointer_dereference() we have the code to
> initialize block_index and offset if they are null, but it is called
> after checking if the pointer has then non-null.
>

This seems fishy.  The code you're moving only triggers for
OpPtrAccessChain.  In order to run into an issue, they would have to be
doing an OpPtrAccessChain an an array of resources.  This shouldn't be
allowed by the spec.  I'll have to look at the actual SPIR-V to be sure,
but I think this is probably a CTS bug.


> Reordering that code fixes crashes in:
>
>    dEQP-VK.spirv_assembly.instruction.*.indexing.*
>
> Signed-off-by: Samuel Iglesias Gonsálvez <siglesias at igalia.com>
> ---
>  src/compiler/spirv/vtn_variables.c | 29 +++++++++++++++--------------
>  1 file changed, 15 insertions(+), 14 deletions(-)
>
> diff --git a/src/compiler/spirv/vtn_variables.c b/src/compiler/spirv/vtn_
> variables.c
> index 4f6acd2e07..baf1edde4c 100644
> --- a/src/compiler/spirv/vtn_variables.c
> +++ b/src/compiler/spirv/vtn_variables.c
> @@ -146,20 +146,6 @@ vtn_ssa_offset_pointer_dereference(struct
> vtn_builder *b,
>     struct vtn_type *type = base->type;
>
>     unsigned idx = 0;
> -   if (deref_chain->ptr_as_array) {
> -      /* We need ptr_type for the stride */
> -      assert(base->ptr_type);
> -      /* This must be a pointer to an actual element somewhere */
> -      assert(block_index && offset);
> -      /* We need at least one element in the chain */
> -      assert(deref_chain->length >= 1);
> -
> -      nir_ssa_def *elem_offset =
> -         vtn_access_link_as_ssa(b, deref_chain->link[idx],
> -                                base->ptr_type->stride);
> -      offset = nir_iadd(&b->nb, offset, elem_offset);
> -      idx++;
> -   }
>
>     if (!block_index) {
>        assert(base->var);
> @@ -182,6 +168,21 @@ vtn_ssa_offset_pointer_dereference(struct
> vtn_builder *b,
>     }
>     assert(offset);
>
> +   if (deref_chain->ptr_as_array) {
> +      /* We need ptr_type for the stride */
> +      assert(base->ptr_type);
> +      /* This must be a pointer to an actual element somewhere */
> +      assert(block_index && offset);
> +      /* We need at least one element in the chain */
> +      assert(deref_chain->length >= 1);
> +
> +      nir_ssa_def *elem_offset =
> +         vtn_access_link_as_ssa(b, deref_chain->link[idx],
> +                                base->ptr_type->stride);
> +      offset = nir_iadd(&b->nb, offset, elem_offset);
> +      idx++;
> +   }
> +
>     for (; idx < deref_chain->length; idx++) {
>        switch (glsl_get_base_type(type->type)) {
>        case GLSL_TYPE_UINT:
> --
> 2.14.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170907/eb9e6cd9/attachment-0001.html>