KhronosGroup / glslang

Khronos-reference front end for GLSL/ESSL, partial front end for HLSL, and a SPIR-V generator.
Other
2.98k stars 823 forks source link

Output SPIR-V is invalid: OpStore Pointer <id> '11[%11]'s type does not match Object <id> '9[%false]'s type. #1823

Closed paulthomson closed 5 years ago

paulthomson commented 5 years ago

bug_report.zip

Issue found using GraphicsFuzz.

Tool versions:

To reproduce:

glslangValidator -V shader.comp -o shader.comp.spv

spirv-val shader.comp.spv

The following shader files are included in the attached archive, some of which are also shown inline below:

0_glsl/shader.comp:

#version 310 es

precision highp float;

layout(std430, binding = 0) buffer doesNotMatter {
 vec4 _compute_data[];
} ;
layout(local_size_x = 4, local_size_y = 8, local_size_z = 22) in;
void main()
{
 vec4(1.0, (false ? dFdy(1.0) : 1.0), 1.0, 1.0);
}

1_spirv_asm/shader.comp.asm:

; SPIR-V
; Version: 1.0
; Generator: Khronos Glslang Reference Front End; 7
; Bound: 28
; Schema: 0
               OpCapability Shader
          %1 = OpExtInstImport "GLSL.std.450"
               OpMemoryModel Logical GLSL450
               OpEntryPoint GLCompute %4 "main"
               OpExecutionMode %4 LocalSize 4 8 22
               OpSource ESSL 310
               OpName %4 "main"
               OpName %19 "doesNotMatter"
               OpMemberName %19 0 "_compute_data"
               OpName %21 ""
               OpDecorate %18 ArrayStride 16
               OpMemberDecorate %19 0 Offset 0
               OpDecorate %19 BufferBlock
               OpDecorate %21 DescriptorSet 0
               OpDecorate %21 Binding 0
               OpDecorate %27 BuiltIn WorkgroupSize
          %2 = OpTypeVoid
          %3 = OpTypeFunction %2
          %6 = OpTypeFloat 32
          %7 = OpConstant %6 1
          %8 = OpTypeBool
          %9 = OpConstantFalse %8
         %10 = OpTypePointer Function %6
         %16 = OpTypeVector %6 4
         %18 = OpTypeRuntimeArray %16
         %19 = OpTypeStruct %18
         %20 = OpTypePointer Uniform %19
         %21 = OpVariable %20 Uniform
         %22 = OpTypeInt 32 0
         %23 = OpTypeVector %22 3
         %24 = OpConstant %22 4
         %25 = OpConstant %22 8
         %26 = OpConstant %22 22
         %27 = OpConstantComposite %23 %24 %25 %26
          %4 = OpFunction %2 None %3
          %5 = OpLabel
         %11 = OpVariable %10 Function
               OpSelectionMerge %13 None
               OpBranchConditional %9 %12 %14
         %12 = OpLabel
               OpStore %11 %9
               OpBranch %13
         %14 = OpLabel
               OpStore %11 %7
               OpBranch %13
         %13 = OpLabel
         %15 = OpLoad %6 %11
         %17 = OpCompositeConstruct %16 %7 %15 %7 %7
               OpReturn
               OpFunctionEnd
afd commented 5 years ago

I believe dFdy is not allowed in a compute shader, and that we found this issue due to having a bug in GraphicsFuzz where fragment processing builtins were getting used in compute shaders.

@johnkslang should glslang complain when a fragment processing builtin is used in a compute shader?

johnkslang commented 5 years ago

Yes, it is a semantic error. I suspect some extensions broadened where these were accepted, without enough error checking.

johnkslang commented 5 years ago

It looks like GL_NV_compute_shader_derivatives, via beae2251b, unconditionally added these functions to the compute stage, and then conditionally required extensions to use them for the fragment stage, leaving them wide open in the compute stage.

The specification says:

This extension can be applied to OpenGL GLSL versions 4.50 (#version 450) and higher.

This extension can be applied to OpenGL ES ESSL versions 3.20 (#version 320) and higher.

This logic was attempted for setting the extensions, except done the fragment stage instead of the compute stage, while the logic was not applied at all for where the base functions are provided, hence ES picked them all up for free, no extension required.