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

Assertion failure: "Default missing" #1830

Closed afd closed 5 years ago

afd commented 5 years ago

glslangValidator: ../glslang/MachineIndependent/../Include/../Include/ConstantUn ion.h:710: glslang::TConstUnion glslang::TConstUnion::operator<<(const glslang:: TConstUnion&) const: Assertion `false && "Default missing"' failed.

bug_report.zip

Issue found using GraphicsFuzz.

Tool versions:

To reproduce:

glslangValidator -V shader.frag -o shader.frag.spv

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

0_glsl/shader.frag:

precision highp float;

float nb_mod()
{
 switch(0)
  {
   case 88:
   uvec4(31209u, 25988u, 130214u, 95017u) << mix(21077u, 69387u, false);
   return 1.0;
  }
 return 1.0;
}
void main()
{
}
johnkslang commented 5 years ago

Are you really trying to translate a pre ES 3.1 shader to SPIR-V? Or is version information missing?

afd commented 5 years ago

No - please see the file 0_glsl/shader.frag in the zip file - this has "#version 310 es" at the top (I'll check on why this was excluded from the snippet shown in the top level report).

So this does occur when trying to translate an ES 3.10 shader.

johnkslang commented 5 years ago

I see. There is no code in the case before the return; just an orphaned constant.

afd commented 5 years ago

That's because our test case reducer has found the smallest example it could that triggers the assert. It does not appear to be the cause of the issue; I tried adding some code before the return and the assertion failure still triggers.

johnkslang commented 5 years ago

I found the cause: The constant folder for mix() is pre-integer support in mix(), so assumes the result is floating point. The built-in prototype knows the result is uint, but the folded constant is assigned a floating point type, leaving an inconsistency.

The spec. does not require constant folding for mix(), so I'll just change the folder to not fold for this newer case.

Since it's incredibly unlikely that one would use mix() to represent a constant uint, there should not be any impact to not folding it.

johnkslang commented 5 years ago

our test case reducer has found the smallest example it could that triggers the assert

Curious, as removing basically anything except the shift of the mix() still causes the problem. Here's the minimal shader, if you want to take that back to why the fuzzer leaves the function call, switch statement, precision statement, etc. when reducing.

#version 310 es

void main()
{
    1 << mix(1u, 1u, false);
}
johnkslang commented 5 years ago

Fixed by 1a6e8534cef32ce855f8b97ea84ffa93152242b0. The commit had a typo referring to #1833 instead of #1830.

afd commented 5 years ago

Thanks for noticing that the reducer could have been more aggressive here; I have filed a GraphicsFuzz issue for that:

https://github.com/google/graphicsfuzz/issues/610