Bug 30623 - Conquest crash (Assertion `lhs_components == this->rhs->type->vector_elements' failed.)
Summary: Conquest crash (Assertion `lhs_components == this->rhs->type->vector_elements...
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Mesa core (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Ian Romanick
QA Contact:
URL: http://download.conquest-game.com/Con...
Whiteboard:
Keywords: regression
Depends on:
Blocks:
 
Reported: 2010-10-05 05:26 UTC by Pavel Ondračka
Modified: 2010-10-26 00:54 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
Always print assignment expressions for debugging (448 bytes, patch)
2010-10-12 11:15 UTC, Ian Romanick
Details | Splinter Review
new backtrace (5.66 KB, text/plain)
2010-10-12 13:30 UTC, Pavel Ondračka
Details

Description Pavel Ondračka 2010-10-05 05:26:04 UTC
Game Conquest http://www.conquest-game.com/ crash at start, tested with r300g and llvmpipe, mesa: bf21b7006c63c3dc47045c22d4f372dfe6c7ce67.

Conquest.bin: ir.cpp:150: ir_assignment::ir_assignment(ir_dereference*, ir_rvalue*, ir_rvalue*, unsigned int): Assertion `lhs_components == this->rhs->type->vector_elements' failed.

Program received signal SIGABRT, Aborted.
0x00110416 in __kernel_vsyscall ()

#0  0x00110416 in __kernel_vsyscall ()
#1  0x009a9d11 in raise () from /lib/libc.so.6
#2  0x009ab5ea in abort () from /lib/libc.so.6
#3  0x009a2d98 in __assert_fail () from /lib/libc.so.6
#4  0x0115f81a in ir_assignment::ir_assignment (this=0xb04ea38, lhs=0xb04e950, 
    rhs=0xb04e9a0, condition=0x0, write_mask=7) at ir.cpp:150
#5  0x011a44ed in emit_inline_vector_constructor (type=0x1aa2bdc, 
    instructions=0xb04d808, parameters=0xbfffe3e0, ctx=0xaffc720)
    at ast_function.cpp:566
#6  0x011a72e0 in ast_function_expression::hir (this=0xb028660, 
    instructions=0xb04d808, state=0xaffc720) at ast_function.cpp:1171
#7  0x01142877 in ast_expression::hir (this=0xb028c50, instructions=0xb04d808, 
    state=0xaffc720) at ast_to_hir.cpp:690
#8  0x0113d486 in ast_expression_statement::hir (this=0xb028cc0, 
    instructions=0xb04d808, state=0xaffc720) at ast_to_hir.cpp:1455
#9  0x0113d8cb in ast_compound_statement::hir (this=0xb028e68, 
    instructions=0xb04d808, state=0xaffc720) at ast_to_hir.cpp:1471
#10 0x0113da4f in ast_function_definition::hir (this=0xb028ec8, 
    instructions=0xb028d10, state=0xaffc720) at ast_to_hir.cpp:2406
#11 0x0113e9d3 in _mesa_ast_to_hir (instructions=0xb028d10, state=0xaffc720)
    at ast_to_hir.cpp:85
#12 0x0113648c in _mesa_glsl_compile_shader (ctx=0x81adad8, shader=0xaff57c8)
    at program/ir_to_mesa.cpp:2551
#13 0x010e8159 in compile_shader (shaderObj=18) at main/shaderapi.c:807
#14 _mesa_CompileShaderARB (shaderObj=18) at main/shaderapi.c:1095
#15 0x0055a6b3 in Proxy::Shader::build() ()
   from /home/Paulie/Stažené/Conquest/Binaries/libProxyVideo.so
#16 0x0055c58d in Proxy::Shader::load() ()
   from /home/Paulie/Stažené/Conquest/Binaries/libProxyVideo.so
#17 0x0055cf17 in Proxy::Shader::Shader(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) ()
   from /home/Paulie/Stažené/Conquest/Binaries/libProxyVideo.so
#18 0x001253eb in Proxy::ResourceManager<Proxy::Shader>::setFromDirectory(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) ()
   from /home/Paulie/Stažené/Conquest/Binaries/libConquestClientGame.so
#19 0x0011eb97 in Conquest::ClientGame::initClientData() ()
   from /home/Paulie/Stažené/Conquest/Binaries/libConquestClientGame.so
#20 0x0011ffec in Conquest::ClientGame::initClient() ()
   from /home/Paulie/Stažené/Conquest/Binaries/libConquestClientGame.so
#21 0x00120b70 in Conquest::ClientGame::init() ()
   from /home/Paulie/Stažené/Conquest/Binaries/libConquestClientGame.so
#22 0x001371f0 in Conquest::Game::run() ()
   from /home/Paulie/Stažené/Conquest/Binaries/libConquestGame.so
#23 0x0059081f in Proxy::Application::main(int, char**) ()
   from /home/Paulie/Stažené/Conquest/Binaries/libProxySystem.so
#24 0x08049a0c in main ()
Comment 1 Ian Romanick 2010-10-08 15:09:40 UTC
Does this also occur with the 7.9 release or branch?

At the crash, can you:

 - Go up to stack frame #5 (in emit_inline_vector_constructor) and print lhs->type->name and rhs->type->name?

 - Go up to stack frame #8 (in ast_expression_statement::hir) and invoke this->print()?
Comment 2 Pavel Ondračka 2010-10-10 01:22:55 UTC
(In reply to comment #1)
> Does this also occur with the 7.9 release or branch?
> 
Yes

> At the crash, can you:
> 
>  - Go up to stack frame #5 (in emit_inline_vector_constructor) and print
> lhs->type->name and rhs->type->name?
> 
>  - Go up to stack frame #8 (in ast_expression_statement::hir) and invoke
> this->print()?

I need some help with this, run, bt and q are all commands I've ever used with gdb. I'm able to go up to stack frame #5 but when I do 'print lhs->type->name' I get 'Cannot access memory at address 0x10'. Maybe I'm using wrong command? I also noticed in bt full something like this in stack frame 5 (and others) 'lhs = <value optimized out>' I recompiled with -O0 but it didn't help. And how to "invoke" this->print()?
Comment 3 Pavel Ondračka 2010-10-11 00:02:33 UTC
I did some more testing and it seems this is a regression. I haven't found this before since the game was released just a week ago and a didn't have time for longer testing. 

commit b39e6f33b60ef9bbaf81f320aaca6a440d8a6a8f
Author: Eric Anholt <eric@anholt.net>
Date:   Wed Sep 22 11:47:03 2010 -0700

    glsl: Rework assignments with write_masks to have LHS chan count match RHS.
    
    It turns out that most people new to this IR are surprised when an
    assignment to (say) 3 components on the LHS takes 4 components on the
    RHS.  It also makes for quite strange IR output:
    
    (assign (constant bool (1)) (x) (var_ref color) (swiz x (var_ref v) ))
    (assign (constant bool (1)) (y) (var_ref color) (swiz yy (var_ref v) ))
    (assign (constant bool (1)) (z) (var_ref color) (swiz zzz (var_ref v) ))
    
    But even worse, even we get it wrong, as shown by this line of our
    current step(float, vec4):
    
    (assign (constant bool (1)) (w)
    	(var_ref t)
    	(expression float b2f (expression bool >=
    		    (swiz w (var_ref x))(var_ref edge))))
    
    where we try to assign a float to the writemasked-out x channel and
    don't supply anything for the actual w channel we're writing.  Drivers
    right now just get lucky since ir_to_mesa spams the float value across
    all the source channels of a vec4.
    
    Instead, the RHS will now have a number of components equal to the
    number of components actually being written.  Hopefully this confuses
    everyone less, and it also makes codegen for a scalar target simpler.
    
    Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
    Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
Comment 4 Ian Romanick 2010-10-12 11:14:19 UTC
(In reply to comment #2)
> (In reply to comment #1)
> > Does this also occur with the 7.9 release or branch?
> > 
> Yes
> 
> > At the crash, can you:
> > 
> >  - Go up to stack frame #5 (in emit_inline_vector_constructor) and print
> > lhs->type->name and rhs->type->name?
> > 
> >  - Go up to stack frame #8 (in ast_expression_statement::hir) and invoke
> > this->print()?
> 
> I need some help with this, run, bt and q are all commands I've ever used with
> gdb. I'm able to go up to stack frame #5 but when I do 'print lhs->type->name'
> I get 'Cannot access memory at address 0x10'. Maybe I'm using wrong command? I
> also noticed in bt full something like this in stack frame 5 (and others) 'lhs
> = <value optimized out>' I recompiled with -O0 but it didn't help.

Compiling with -O0 should fix that.  You will need to 'make clean' before changing the compiler flags.  If you're using the autoconf build, you should probably also add --enable-debug to the configure or autogen command line.

> And how to "invoke" this->print()?

Usually you would just do 'call this->print()', but that doesn't seem to work for me on this code right now.  If that fails, I have a patch that will force it to always print an assignment expression.
Comment 5 Ian Romanick 2010-10-12 11:15:05 UTC
Created attachment 39388 [details] [review]
Always print assignment expressions for debugging
Comment 6 Pavel Ondračka 2010-10-12 11:34:58 UTC
I can't test this with latest mesa, there is different crash now (is this some new bug?), I'll go back and provide the needed info with some earlier version of mesa.
 
#0  0x00110416 in __kernel_vsyscall ()
#1  0x00927501 in raise () from /lib/libc.so.6
#2  0x00928f6e in abort () from /lib/libc.so.6
#3  0x0091fe18 in __assert_fail () from /lib/libc.so.6
#4  0x010325e7 in swizzle_for_size (size=<value optimized out>)
    at program/ir_to_mesa.cpp:314
#5  0x01032d88 in ir_to_mesa_visitor::visit (this=0xbfffe574, ir=0x8a03320)
    at program/ir_to_mesa.cpp:1313
#6  0x01056838 in ir_dereference_record::accept (this=0x8a03320, v=0xbfffe574)
    at ir.h:1360
#7  0x01032768 in ir_to_mesa_visitor::visit (this=0xbfffe574, ir=0x89f71e8)
    at program/ir_to_mesa.cpp:1153
#8  0x01056718 in ir_swizzle::accept (this=0x89f71e8, v=0xbfffe574)
    at ir.h:1207
#9  0x0103514b in ir_to_mesa_visitor::visit (this=0xbfffe574, ir=0x875ae80)
    at program/ir_to_mesa.cpp:1349
#10 0x010565e8 in ir_assignment::accept (this=0x875ae80, v=0xbfffe574)
    at ir.h:604
#11 0x01032e1e in ir_to_mesa_visitor::visit (this=0xbfffe574, ir=0x8a59798)
    at program/ir_to_mesa.cpp:779
#12 0x01056568 in ir_function::accept (this=0x8a59798, v=0xbfffe574)
    at ir.h:437
#13 0x010608ab in visit_exec_list (list=0x87bb630, visitor=0xbfffe574)
    at ir.cpp:1200
#14 0x01038a21 in get_mesa_program (ctx=0x8100310, shader_program=0x85ee0d0, 
    shader=0x89fb730) at program/ir_to_mesa.cpp:2309
#15 0x010395f9 in _mesa_ir_link_shader (ctx=0x8100310, prog=0x85ee0d0)
    at program/ir_to_mesa.cpp:2517
#16 0x01039ca9 in _mesa_glsl_link_shader (ctx=0x8100310, prog=0x85ee0d0)
    at program/ir_to_mesa.cpp:2659
#17 0x00fd3c6a in link_program (ctx=0x8100310, program=1)
    at main/shaderapi.c:833
#18 0x00fd4d55 in _mesa_LinkProgramARB (programObj=1) at main/shaderapi.c:1346
#19 0x00cc60e9 in glLinkProgramARB () from /usr/lib/libGL.so.1
#20 0x0058e8aa in Proxy::Shader::build() ()
   from /home/Paulie/Stažené/Conquest/Binaries/libProxyVideo.so
#21 0x0059085d in Proxy::Shader::load() ()
   from /home/Paulie/Stažené/Conquest/Binaries/libProxyVideo.so
#22 0x005911e7 in Proxy::Shader::Shader(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) ()
   from /home/Paulie/Stažené/Conquest/Binaries/libProxyVideo.so
#23 0x0012515b in Proxy::ResourceManager<Proxy::Shader>::setFromDirectory(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) ()
   from /home/Paulie/Stažené/Conquest/Binaries/libConquestClientGame.so
#24 0x0011f573 in Conquest::ClientGame::initClient() ()
   from /home/Paulie/Stažené/Conquest/Binaries/libConquestClientGame.so
#25 0x001212d0 in Conquest::ClientGame::init() ()
   from /home/Paulie/Stažené/Conquest/Binaries/libConquestClientGame.so
#26 0x00137240 in Conquest::Game::run() ()
   from /home/Paulie/Stažené/Conquest/Binaries/libConquestGame.so
#27 0x005c481f in Proxy::Application::main(int, char**) ()
   from /home/Paulie/Stažené/Conquest/Binaries/libProxySystem.so
#28 0x08049a0c in main ()
Comment 7 Pavel Ondračka 2010-10-12 12:15:52 UTC
I did revert to older mesa and I was quite surprised it worked...
I asked game developers about recent changes, I hope this is helpful, I'll look now into the new crash.

From #conquest-game Quake-Net
<void`> let me check
<void`> yeah we fixed that
<void`> the intel cards doesnt work properly with a certain assignment syntax
<Paulie889> i'm asking because there is surely a bug in mesa because the game was working with proprietary drivers... 
<void`> (at least some of the series)
<void`> yeah quite possible
<void`> if you want to forward it
<void`> vec4 bla = vec4(vec3(1.0, 1.0, 1.0), alpha);
<void`> does not work
<void`> while
<void`> vec3 bla = vec4(1.0, 1.0, 1.0, alpha)
<void`> does
<Paulie889> ah, can i send it to the bug report?
<void`> sure
<void`> though
<void`> rather write gl_FragColor instead of vec3 bla
<void`> maybe its only for this output value
<void`> alpha is a float
<void`> but im pretty sure by just giving the above lines theyll figure it out
<Paulie889> ok, thank you very much
Comment 8 Pavel Ondračka 2010-10-12 13:30:59 UTC
Created attachment 39394 [details]
new backtrace

OK, so now everything is working correctly in the game. The original problem was solved in game, new crash was solved with 9fea9e5e2115bcb52435648d2ef753638733d7d9. I was even able to disable optimizations... (CFLAGS="-O0" wasn't enough, CXXFLAGS="-O0" was also needed.) I'm attaching new backtrace acquired with patch applied and the rest of needed info.

(gdb) print lhs->type->name
$1 = 0x80bb088 "vec4"

(gdb) print rhs->type->name
$2 = 0x80bafc8 "float"
Comment 9 Ian Romanick 2010-10-12 18:57:41 UTC
(In reply to comment #6)
> I can't test this with latest mesa, there is different crash now (is this some
> new bug?), I'll go back and provide the needed info with some earlier version
> of mesa.
> 
> #0  0x00110416 in __kernel_vsyscall ()
> #1  0x00927501 in raise () from /lib/libc.so.6
> #2  0x00928f6e in abort () from /lib/libc.so.6
> #3  0x0091fe18 in __assert_fail () from /lib/libc.so.6
> #4  0x010325e7 in swizzle_for_size (size=<value optimized out>)
>     at program/ir_to_mesa.cpp:314

This should have been fixed earlier today by:

commit 9fea9e5e2115bcb52435648d2ef753638733d7d9
Author: Ian Romanick <ian.d.romanick@intel.com>
Date:   Tue Oct 12 12:50:29 2010 -0700

    glsl: Fix incorrect assertion
    
    This assertion was added in commit f1c1ee11, but it did not notice
    that the array is accessed with 'size-1' instead of 'size'.  As a
    result, the assertion was off by one.  This caused failures in at
    least glsl-orangebook-ch06-bump.
Comment 10 Ian Romanick 2010-10-12 19:23:32 UTC
(In reply to comment #7)
> I did revert to older mesa and I was quite surprised it worked...
> I asked game developers about recent changes, I hope this is helpful, I'll look
> now into the new crash.
> 
> From #conquest-game Quake-Net
> <void`> let me check
> <void`> yeah we fixed that
> <void`> the intel cards doesnt work properly with a certain assignment syntax
> <Paulie889> i'm asking because there is surely a bug in mesa because the game
> was working with proprietary drivers... 
> <void`> (at least some of the series)
> <void`> yeah quite possible
> <void`> if you want to forward it
> <void`> vec4 bla = vec4(vec3(1.0, 1.0, 1.0), alpha);
> <void`> does not work
> <void`> while
> <void`> vec3 bla = vec4(1.0, 1.0, 1.0, alpha)

This is invalid GLSL syntax.  It works on NVIDIA because it's valid Cg syntax, but it fails *everywhere* else.  You can't assign a vec4 to a vec3 because they're different types.  The GLSL 1.20 spec explicitly forbids this on page 36 (page 42 of the PDF):

    "The expression and lvalue must have the same type, or the expression
    must have a type in the table in Section 4.1.10 "Implicit Conversions"
    that converts to the type of lvalue, in which case an implicit
    conversion will be done on the expression before the assignment is done."

This shouldn't come as any surprise because it's basically the same rule that C/C++ use.

However, it shouldn't cause the compiler to crash.  I've added a piglit test assignment-type-mismatch.vert that reproduces this.
Comment 11 Ian Romanick 2010-10-14 12:43:26 UTC
After re-reading comment #7, I've added a positive test constructor-28.vert that reproduces this bug.
Comment 12 Ian Romanick 2010-10-25 13:28:26 UTC
Can you verify that this bug is fixed by the following commit?

commit ba2382f50d7815947e17fe993b39feb573638d12
Author: Kenneth Graunke <kenneth@whitecape.org>
Date:   Mon Oct 25 12:44:55 2010 -0700

    glsl: Fix constant component count in vector constructor emitting.

    Fixes freedesktop.org bug #31101 as well as piglit test cases
    assignment-type-mismatch.vert and constructor-28.vert.
Comment 13 Pavel Ondračka 2010-10-26 00:54:22 UTC
Fixed indeed.


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.