Bug 31371

Summary: glslparsertest: ir.cpp:358: ir_constant::ir_constant(const glsl_type*, const ir_constant_data*): Assertion `(type->base_type >= 0) && (type->base_type <= 3)' failed.
Product: Mesa Reporter: Vinson Lee <vlee>
Component: Mesa coreAssignee: Chad Versace <chadversary>
Status: CLOSED FIXED QA Contact:
Severity: critical    
Priority: medium CC: brianp, idr, kenneth
Version: git   
Hardware: x86 (IA32)   
OS: Linux (All)   
Whiteboard:
i915 platform: i915 features:
Attachments: Fix for bug.

Description Vinson Lee 2010-11-03 19:54:22 UTC
mesa: d3fcadf8400360f4db45a4deb45b3b260e880b49 (master)

Run piglit glslparsertest preprocess6.

swrast   - assert
softpipe - assert
llvmpipe - pass


$ ./bin/glslparsertest tests/glslparsertest/shaders/preprocess6.frag fail
glslparsertest: ir.cpp:358: ir_constant::ir_constant(const glsl_type*, const ir_constant_data*): Assertion `(type->base_type >= 0) && (type->base_type <= 3)' failed.

(gdb) bt
#0  0x00cb7416 in __kernel_vsyscall ()
#1  0x00834941 in raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
#2  0x00837e42 in abort () at abort.c:92
#3  0x0082d8e8 in __assert_fail (assertion=0x5a3f28 "(type->base_type >= 0) && (type->base_type <= 3)", 
    file=0x5a3e60 "ir.cpp", line=358, 
    function=0x5a4740 "ir_constant::ir_constant(const glsl_type*, const ir_constant_data*)") at assert.c:81
#4  0x004282fa in ir_constant::ir_constant (this=0x9717bd8, type=0x5e1180, data=0xbffa7100) at ir.cpp:357
#5  0x00422056 in ir_expression::constant_expression_value (this=0x99ce278) at ir_constant_expression.cpp:790
#6  0x0041eab2 in ir_expression::constant_expression_value (this=0x99c51f8) at ir_constant_expression.cpp:63
#7  0x003ff026 in ast_expression::hir (this=0x96f30e8, instructions=0x99c43a8, state=0x96ed818) at ast_to_hir.cpp:1056
#8  0x003fe2d7 in ast_expression::hir (this=0x96f3158, instructions=0x99c43a8, state=0x96ed818) at ast_to_hir.cpp:868
#9  0x003fe1da in ast_expression::hir (this=0x96f31c8, instructions=0x99c43a8, state=0x96ed818) at ast_to_hir.cpp:855
#10 0x003fdfcc in ast_expression::hir (this=0x96f3238, instructions=0x99c43a8, state=0x96ed818) at ast_to_hir.cpp:820
#11 0x00400e9e in ast_expression_statement::hir (this=0x96f32a8, instructions=0x99c43a8, state=0x96ed818)
    at ast_to_hir.cpp:1582
#12 0x00400efe in ast_compound_statement::hir (this=0x96f3930, instructions=0x99c43a8, state=0x96ed818)
    at ast_to_hir.cpp:1598
#13 0x00402fed in ast_function_definition::hir (this=0x96f3990, instructions=0x96ee5c0, state=0x96ed818)
    at ast_to_hir.cpp:2595
#14 0x003fc9ce in _mesa_ast_to_hir (instructions=0x96ee5c0, state=0x96ed818) at ast_to_hir.cpp:85
#15 0x003fb226 in _mesa_glsl_compile_shader (ctx=0x93bd538, shader=0x96ed560) at program/ir_to_mesa.cpp:2589
#16 0x0031edc2 in compile_shader (ctx=0x93bd538, shaderObj=1) at main/shaderapi.c:853
#17 0x0031f74e in _mesa_CompileShaderARB (shaderObj=1) at main/shaderapi.c:1191
#18 0x0804a029 in test () at piglit/tests/glslparsertest/glslparsertest.c:108
#19 0x0804a48f in main (argc=3, argv=0xbffa8054) at piglit/tests/glslparsertest/glslparsertest.c:224
(gdb) frame 4
#4  0x004282fa in ir_constant::ir_constant (this=0x9717bd8, type=0x5e1180, data=0xbffa7100) at ir.cpp:357
357	   assert((type->base_type >= GLSL_TYPE_UINT)
(gdb) l
352	}
353	
354	ir_constant::ir_constant(const struct glsl_type *type,
355				 const ir_constant_data *data)
356	{
357	   assert((type->base_type >= GLSL_TYPE_UINT)
358		  && (type->base_type <= GLSL_TYPE_BOOL));
359	
360	   this->ir_type = ir_type_constant;
361	   this->type = type;
(gdb) print type->base_type
$2 = 9
Comment 1 Vinson Lee 2010-11-03 20:04:48 UTC
git bisect good 4af293741635aea8630e8734a8b4caf58047e91d
git bisect bad 16333e1fc489a414233bc485f1230b847d11b020
Comment 2 Vinson Lee 2010-11-04 11:37:56 UTC
cfdbf8bc8497b29fbdd9fa7bd00da554aecb5962 is the first bad commit
commit cfdbf8bc8497b29fbdd9fa7bd00da554aecb5962
Author: Chad Versace <chad.versace@intel.com>
Date:   Fri Oct 15 11:28:05 2010 -0700

    glsl: Define bit_logic_result_type() in ast_to_hir.cpp
    
    This function type checks the operands of and returns the result type of
    bit-logic operations.  It replaces the type checking performed in the
    following cases of ast_expression::hir() :
        - ast_bit_and
        - ast_bit_or
        - ast_bit_xor

:040000 040000 00d9f41690488d71e665e7e8008df051eee45d24 d4b4be0baaa01eb4783af67fa94272bc266bda9f M	src
bisect run success
Comment 3 Kenneth Graunke 2010-11-04 13:23:31 UTC
My preprocess6 branch (in ~kwg/mesa) fixes this assertion failure, but may be just papering over the problem.

Looking into why/how error_type values are getting mishandled in ast_to_hir is probably valuable.  It may reveal a better fix.
Comment 4 Ian Romanick 2010-11-08 15:28:32 UTC
I'm confused as to why this passes on llvmpipe but asserts on other software drivers.  At the point of the assertion, nothing device dependent should have come into play.  What does the test do on a hardware driver?

(In reply to comment #3)
> My preprocess6 branch (in ~kwg/mesa) fixes this assertion failure, but may be
> just papering over the problem.
> 
> Looking into why/how error_type values are getting mishandled in ast_to_hir is
> probably valuable.  It may reveal a better fix.

I don't think treating error expressions as some sort of odd constant error is right.  Shouldn't ir_expression::constant_expression_value just return NULL if the type is error?  That seems like a more correct fix (assuming that fixes it).
Comment 5 Chad Versace 2010-11-08 17:39:28 UTC
Created attachment 40130 [details] [review]
Fix for bug.
Comment 6 Chad Versace 2010-11-08 17:41:39 UTC
(In reply to comment #4)
> I don't think treating error expressions as some sort of odd constant error is
> right.  Shouldn't ir_expression::constant_expression_value just return NULL if
> the type is error?  That seems like a more correct fix (assuming that fixes
> it).

I just submitted a 3-line patch that does exactly that. And it does fix the bug, at least on i915 and swrast.

(Sorry for the double-post.)
Comment 7 Kenneth Graunke 2010-11-09 00:53:05 UTC
Ah man, I was way overthinking things.  That's definitely a sensible solution.

Confirmed fixed and pushed to master (with minor fix - 3-space indentation rather than 4).  Thanks Chad and Ian.

commit b62c1c4595551c4936323135224a5ea686ba972a
Author: Chad Versace <chad.versace@intel.com>
Date:   Mon Nov 8 17:30:13 2010 -0800

    glsl: Fix ir_expression::constant_expression_value()
    
    When the type of the ir_expression is error_type, return NULL.
    This fixes bug 31371.
Comment 8 Vinson Lee 2010-11-10 18:11:29 UTC
mesa: dc524adee2cfd0f115800cd4ec3f8384010f154e (master)

Verified fixed on swrast, softpipe, and llvmpipe.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.