Spirv-Val Does Not Validate Against Empty Structs

[cwfitzgerald]

Upstream issue shader-slang/slang#5351. You can see the problematic spirv includes a %5 = OpTypeStruct which is an OpTypeStruct with no arguments. This causes nvidia's drivers to segfault and causes mesa to outright the reject the pipeline.

As far as I can tell this is indeed invalid, so it would be nice to get validation against this, as it went unnoticed.

[alan-baker]

Vulkan allows empty structs (see Offset and Stride Assignment).

A structure has a base alignment equal to the largest base alignment of any of its members. An empty structure has a base alignment equal to the size of the smallest scalar type permitted by the capabilities declared in the SPIR-V module. (e.g., for a 1 byte aligned empty struct in the StorageBuffer storage class, StorageBuffer8BitAccess or UniformAndStorageBuffer8BitAccess must be declared in the SPIR-V module.)

So I need more context about how this is failing in drivers to determine if it is truly a validation issue.

[alan-baker]

For reference this was resolved in the internal Vulkan issue 2174.

[cwfitzgerald]

Interesting! So this is the minimized spirv involved. It passes spirv-val:

; SPIR-V
; Version: 1.4
; Generator: Khronos SPIR-V Tools Assembler; 0
; Bound: 30
; Schema: 0
               OpCapability VariablePointers
               OpCapability PhysicalStorageBufferAddresses
               OpCapability Int64
               OpCapability Int8
               OpCapability Shader
               OpExtension "SPV_KHR_variable_pointers"
               OpExtension "SPV_KHR_physical_storage_buffer"
               OpMemoryModel PhysicalStorageBuffer64 GLSL450
               OpEntryPoint Vertex %vertMain "main" %light_buffer %gl_Position
               OpSource Slang 1
               OpName %cbuffer__t "cbuffer__t"
               OpName %light_buffer "light_buffer"
               OpName %vertMain "vertMain"
               OpName %LightBuffer_natural "LightBuffer_natural"
               OpDecorate %_ptr_PhysicalStorageBuffer_LightBuffer_natural ArrayStride 0
               OpDecorate %cbuffer__t Block
               OpMemberDecorate %cbuffer__t 0 Offset 0
               OpDecorate %_ptr_PhysicalStorageBuffer_uchar ArrayStride 1
               OpDecorate %gl_Position BuiltIn Position
       %void = OpTypeVoid
          %9 = OpTypeFunction %void
               OpTypeForwardPointer %_ptr_PhysicalStorageBuffer_LightBuffer_natural PhysicalStorageBuffer
 %cbuffer__t = OpTypeStruct %_ptr_PhysicalStorageBuffer_LightBuffer_natural
%_ptr_PushConstant_cbuffer__t = OpTypePointer PushConstant %cbuffer__t
        %int = OpTypeInt 32 1
      %int_0 = OpConstant %int 0
%_ptr_PushConstant_6 = OpTypePointer PushConstant %_ptr_PhysicalStorageBuffer_LightBuffer_natural
      %int_1 = OpConstant %int 1
      %ulong = OpTypeInt 64 0
      %uchar = OpTypeInt 8 0
%_ptr_PhysicalStorageBuffer_uchar = OpTypePointer PhysicalStorageBuffer %uchar
      %float = OpTypeFloat 32
    %v4float = OpTypeVector %float 4
%_ptr_Output_v4float = OpTypePointer Output %v4float
%LightBuffer_natural = OpTypeStruct
%_ptr_PhysicalStorageBuffer_LightBuffer_natural = OpTypePointer PhysicalStorageBuffer %LightBuffer_natural
%light_buffer = OpVariable %_ptr_PushConstant_cbuffer__t PushConstant
%gl_Position = OpVariable %_ptr_Output_v4float Output
   %vertMain = OpFunction %void None %9
         %20 = OpLabel
         %21 = OpAccessChain %_ptr_PushConstant_6 %light_buffer %int_0
         %22 = OpLoad %_ptr_PhysicalStorageBuffer_LightBuffer_natural %21
         %23 = OpPtrAccessChain %_ptr_PhysicalStorageBuffer_LightBuffer_natural %22 %int_1
         %24 = OpBitcast %ulong %23
         %25 = OpBitcast %_ptr_PhysicalStorageBuffer_uchar %24
         %26 = OpPtrAccessChain %_ptr_PhysicalStorageBuffer_uchar %25 %int_0
         %27 = OpLoad %uchar %26 Aligned 1
         %28 = OpConvertUToF %float %27
         %29 = OpCompositeConstruct %v4float %28 %28 %28 %28
               OpStore %gl_Position %29
               OpReturn
               OpFunctionEnd

which came from the following slang. They removed any unbounded arrays from the underlying spirv struct, which let the LightBuffer_natural struct have zero members. They then bitcast the LightBuffer* to a uint8_t* to read from the unbounded array.

Something about this spirv caused nvidia to segfault in the driver and mesa to reject the pipeline, returning ERROR_UNKNOWN. I don't know if this is strictly the non-membered struct, or some other use of it, but slang ended up not emitting the struct if it has zero args, and that fixed the problem for me.

struct LightBuffer {
    uint8_t lights[];
}

[vk::push_constant]
LightBuffer* light_buffer;

[shader("vertex")]
float4 vertMain() : SV_Position {
    return float4(light_buffer.lights[0]);
}

[alan-baker]

What looks off to me is:

%cbuffer__t = OpTypeStruct %_ptr_PhysicalStorageBuffer_LightBuffer_natural
%_ptr_PushConstant_cbuffer__t = OpTypePointer PushConstant %cbuffer__t
%light_buffer = OpVariable %_ptr_PushConstant_cbuffer__t PushConstant

SPIR-V requires that %light_buffer be decorated with one of AliasedPointer or RestrictPointer, but the validator is only considering arrays and not structs to traverse. If you add one of those decorations with the empty struct does it still fail in those drivers?

[cwfitzgerald]

I added

               OpDecorate %light_buffer AliasedPointer

But nvidia still segfaults and mesa returns ERROR_UNKNOWN.