Fix crash in non-struct physical storage pointers

spirv_reflect.c

@@ -1986,6 +1986,13 @@ static SpvReflectResult ParseType(SpvReflectPrvParser* p_parser, SpvReflectPrvNo
         p_type->type_flags |= SPV_REFLECT_TYPE_FLAG_REF;
         IF_READU32_CAST(result, p_parser, p_node->word_offset + 2, SpvStorageClass, p_type->storage_class);
 
+        SpvReflectPrvNode* p_next_node = FindNode(p_parser, p_node->type_id);
+        if (IsNull(p_next_node)) {
+          result = SPV_REFLECT_RESULT_ERROR_SPIRV_INVALID_ID_REFERENCE;
+          SPV_REFLECT_ASSERT(false);
+          break;
+        }
+
         bool found_recursion = false;
         if (p_type->storage_class == SpvStorageClassPhysicalStorageBuffer) {
           // Need to make sure we haven't started an infinite recursive loop
@@ -1997,7 +2004,7 @@ static SpvReflectResult ParseType(SpvReflectPrvParser* p_parser, SpvReflectPrvNo
               return SPV_REFLECT_RESULT_SUCCESS;
             }
           }
-          if (!found_recursion) {
+          if (!found_recursion && p_next_node->op == SpvOpTypeStruct) {
             p_parser->physical_pointer_struct_count++;
             p_parser->physical_pointer_check[p_parser->physical_pointer_count] = p_type;
             p_parser->physical_pointer_count++;
@@ -2007,12 +2014,7 @@ static SpvReflectResult ParseType(SpvReflectPrvParser* p_parser, SpvReflectPrvNo
           }
         }
 
-        // Parse type
-        SpvReflectPrvNode* p_next_node = FindNode(p_parser, p_node->type_id);
-        if (IsNull(p_next_node)) {
-          result = SPV_REFLECT_RESULT_ERROR_SPIRV_INVALID_ID_REFERENCE;
-          SPV_REFLECT_ASSERT(false);
-        } else if (!found_recursion) {
+        if (!found_recursion) {
           if (p_next_node->op == SpvOpTypeStruct) {
             p_type->struct_type_description = FindType(p_module, p_next_node->result_id);
           }
@@ -2545,15 +2547,19 @@ static SpvReflectResult ParseDescriptorBlockVariable(SpvReflectPrvParser* p_pars
           }
         }
         if (!found_recursion) {
-          uint32_t struct_id = FindType(p_module, p_member_type->id)->struct_type_description->id;
-          p_parser->physical_pointer_structs[p_parser->physical_pointer_struct_count].struct_id = struct_id;
-          p_parser->physical_pointer_structs[p_parser->physical_pointer_struct_count].p_var = p_member_var;
-          p_parser->physical_pointer_struct_count++;
-
-          p_parser->physical_pointer_check[p_parser->physical_pointer_count] = p_member_type;
-          p_parser->physical_pointer_count++;
-          if (p_parser->physical_pointer_count >= MAX_RECURSIVE_PHYSICAL_POINTER_CHECK) {
-            return SPV_REFLECT_RESULT_ERROR_SPIRV_MAX_RECURSIVE_EXCEEDED;
+          SpvReflectTypeDescription* struct_type = FindType(p_module, p_member_type->id);
+          // could be pointer directly to non-struct type here
+          if (struct_type->struct_type_description) {
+            uint32_t struct_id = struct_type->struct_type_description->id;
+            p_parser->physical_pointer_structs[p_parser->physical_pointer_struct_count].struct_id = struct_id;
+            p_parser->physical_pointer_structs[p_parser->physical_pointer_struct_count].p_var = p_member_var;
+            p_parser->physical_pointer_struct_count++;
+
+            p_parser->physical_pointer_check[p_parser->physical_pointer_count] = p_member_type;
+            p_parser->physical_pointer_count++;
+            if (p_parser->physical_pointer_count >= MAX_RECURSIVE_PHYSICAL_POINTER_CHECK) {
+              return SPV_REFLECT_RESULT_ERROR_SPIRV_MAX_RECURSIVE_EXCEEDED;
+            }
           }
         }
 
@@ -2724,7 +2730,13 @@ static SpvReflectResult ParseDescriptorBlockVariableSizes(SpvReflectPrvParser* p
         if (IsNull(p_member_type)) {
           return SPV_REFLECT_RESULT_ERROR_SPIRV_INVALID_ID_REFERENCE;
         }
-        assert(p_member_type->op == SpvOpTypeStruct);
+
+        // If we found a struct, we need to fall through and get the size of it or else we grab the size here
+        if (p_member_type->op != SpvOpTypeStruct) {
+          // TODO - we need to rework this loop as a function to get size for each type
+          // (or maybe determine this size doesn't matter if not a struct in the pointer)
+          break;
+        }
         FALLTHROUGH;
       }
 

tests/access_chains/pointer_access_chain_phy_storage_buffer.slang

@@ -0,0 +1,13 @@
+uniform uint* data_ptr; // creates a ptr chain access
+
+struct Data{
+   int x;
+}
+uniform Data* data_struct;
+
+[numthreads(1,1,1)]
+void computeMain()
+{
+    data_ptr[0] = 1;
+    data_struct->x = 1;
+}

tests/access_chains/pointer_access_chain_phy_storage_buffer.spv.yaml

@@ -0,0 +1,200 @@
+%YAML 1.1
+---
+all_type_descriptions:
+  - &td0
+    id: 4
+    op: 32
+    type_name: 
+    struct_member_name: 
+    storage_class: 5349 # PhysicalStorageBuffer
+    type_flags: 0x40000004 # REF INT 
+    decoration_flags: 0x00000000 # NONE
+    traits:
+      numeric:
+        scalar: { width: 32, signedness: 0 }
+        vector: { component_count: 0 }
+        matrix: { column_count: 0, row_count: 0, stride: 0 }
+      image: { dim: 0, depth: 0, arrayed: 0, ms: 0, sampled: 0, image_format: 0 } # dim=1D image_format=Unknown
+      array: { dims_count: 0, dims: [], stride: 0 }
+    member_count: 0
+    members:
+  - &td1
+    id: 19
+    op: 21
+    type_name: 
+    struct_member_name: 
+    storage_class: 0 # UniformConstant
+    type_flags: 0x00000004 # INT 
+    decoration_flags: 0x00000000 # NONE
+    traits:
+      numeric:
+        scalar: { width: 32, signedness: 1 }
+        vector: { component_count: 0 }
+        matrix: { column_count: 0, row_count: 0, stride: 0 }
+      image: { dim: 0, depth: 0, arrayed: 0, ms: 0, sampled: 0, image_format: 0 } # dim=1D image_format=Unknown
+      array: { dims_count: 0, dims: [], stride: 0 }
+    member_count: 0
+    members:
+  - &td2
+    id: 5
+    op: 32
+    type_name: 
+    struct_member_name: 
+    storage_class: 5349 # PhysicalStorageBuffer
+    type_flags: 0x50080000 # STRUCT REF EXTERNAL_BLOCK 
+    decoration_flags: 0x00000000 # NONE
+    traits:
+      numeric:
+        scalar: { width: 0, signedness: 0 }
+        vector: { component_count: 0 }
+        matrix: { column_count: 0, row_count: 0, stride: 0 }
+      image: { dim: 0, depth: 0, arrayed: 0, ms: 0, sampled: 0, image_format: 0 } # dim=1D image_format=Unknown
+      array: { dims_count: 0, dims: [], stride: 0 }
+    member_count: 1
+    members:
+      - *td1
+  - &td3
+    id: 6
+    op: 30
+    type_name: 
+    struct_member_name: 
+    storage_class: -1 # NOT APPLICABLE
+    type_flags: 0x10080000 # STRUCT EXTERNAL_BLOCK 
+    decoration_flags: 0x00000001 # BLOCK 
+    traits:
+      numeric:
+        scalar: { width: 0, signedness: 0 }
+        vector: { component_count: 0 }
+        matrix: { column_count: 0, row_count: 0, stride: 0 }
+      image: { dim: 0, depth: 0, arrayed: 0, ms: 0, sampled: 0, image_format: 0 } # dim=1D image_format=Unknown
+      array: { dims_count: 0, dims: [], stride: 0 }
+    member_count: 2
+    members:
+      - *td0
+      - *td2
+  - &td4
+    id: 19
+    op: 21
+    type_name: 
+    struct_member_name: 
+    storage_class: 0 # UniformConstant
+    type_flags: 0x00000004 # INT 
+    decoration_flags: 0x00000000 # NONE
+    traits:
+      numeric:
+        scalar: { width: 32, signedness: 1 }
+        vector: { component_count: 0 }
+        matrix: { column_count: 0, row_count: 0, stride: 0 }
+      image: { dim: 0, depth: 0, arrayed: 0, ms: 0, sampled: 0, image_format: 0 } # dim=1D image_format=Unknown
+      array: { dims_count: 0, dims: [], stride: 0 }
+    member_count: 0
+    members:
+all_block_variables:
+  - &bv0
+    name: 
+    offset: 0
+    absolute_offset: 0
+    size: 0
+    padded_size: 8
+    decorations: 0x00000000 # NONE
+    numeric:
+      scalar: { width: 32, signedness: 0 }
+      vector: { component_count: 0 }
+      matrix: { column_count: 0, row_count: 0, stride: 0 }
+    array: { dims_count: 0, dims: [], stride: 0 }
+    member_count: 0
+    members:
+    type_description: *td0
+  - &bv1
+    name: 
+    offset: 0
+    absolute_offset: 0
+    size: 4
+    padded_size: 16
+    decorations: 0x00000000 # NONE
+    numeric:
+      scalar: { width: 32, signedness: 1 }
+      vector: { component_count: 0 }
+      matrix: { column_count: 0, row_count: 0, stride: 0 }
+    array: { dims_count: 0, dims: [], stride: 0 }
+    member_count: 0
+    members:
+    type_description: *td4
+  - &bv2
+    name: 
+    offset: 8
+    absolute_offset: 8
+    size: 8
+    padded_size: 8
+    decorations: 0x00000000 # NONE
+    numeric:
+      scalar: { width: 0, signedness: 0 }
+      vector: { component_count: 0 }
+      matrix: { column_count: 0, row_count: 0, stride: 0 }
+    array: { dims_count: 0, dims: [], stride: 0 }
+    member_count: 1
+    members:
+      - *bv1
+    type_description: *td2
+  - &bv3
+    name: 
+    offset: 0
+    absolute_offset: 0
+    size: 16
+    padded_size: 16
+    decorations: 0x00000000 # NONE
+    numeric:
+      scalar: { width: 0, signedness: 0 }
+      vector: { component_count: 0 }
+      matrix: { column_count: 0, row_count: 0, stride: 0 }
+    array: { dims_count: 0, dims: [], stride: 0 }
+    member_count: 2
+    members:
+      - *bv0
+      - *bv2
+    type_description: *td3
+all_descriptor_bindings:
+  - &db0
+    spirv_id: 3
+    name: 
+    binding: 0
+    input_attachment_index: 0
+    set: 0
+    decoration_flags: 0x00000000 # NONE
+    descriptor_type: 6 # VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER
+    resource_type: 2 # CBV
+    image: { dim: 0, depth: 0, arrayed: 0, ms: 0, sampled: 0, image_format: 0 } # dim=1D image_format=Unknown
+    block: *bv3 # 
+    array: { dims_count: 0, dims: [] }
+    accessed: 1
+    uav_counter_id: 4294967295
+    uav_counter_binding:
+    type_description: *td3
+    word_offset: { binding: 86, set: 90 }
+all_interface_variables:
+module:
+  generator: 7 # Khronos SPIR-V Tools Assembler
+  entry_point_name: "main"
+  entry_point_id: 2
+  source_language: 0 # Unknown
+  source_language_version: 0
+  spirv_execution_model: 5 # GLCompute
+  shader_stage: 0x00000020 # CS
+  descriptor_binding_count: 1
+  descriptor_bindings:
+    - *db0 # 
+  descriptor_set_count: 1
+  descriptor_sets:
+  - set: 0
+    binding_count: 1
+    bindings:
+      - *db0 # 
+  input_variable_count: 0,
+  input_variables:
+  output_variable_count: 0,
+  output_variables:
+  push_constant_count: 0,
+  push_constants:
+  specialization_constant_count: 0,
+  specialization_constants:
+...