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

[Samuel Iglesias Gonsálvez]

This patch is unreviewed.

On Tue, 2017-08-29 at 12:04 +0200, Samuel Iglesias Gonsálvez 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.
> 
> 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:
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170907/d658d5ab/attachment.sig>