Bug 99229

Summary: [G33] thousands of tests crash
Product: Mesa Reporter: Mark Janes <mark.a.janes>
Component: Drivers/DRI/i915Assignee: Timothy Arceri <t_arceri>
Status: RESOLVED FIXED QA Contact: Default DRI bug account <dri-devel>
Severity: normal    
Priority: medium    
Version: git   
Hardware: Other   
OS: All   
Whiteboard:
i915 platform: i915 features:

Description Mark Janes 2016-12-30 09:43:53 UTC 
Introduced in series ending with:
194537ebe44cfcd6d72a98d0c2118f10a9e58deb
Author:     Timothy Arceri <timothy.arceri@collabora.com>
mesa/glsl/i965: remove Driver.NewShader()

After removing brw_shader in the previous commit this is no longer
needed.

V2: remove use in src/compiler/glsl/test_optpass.cpp

Reviewed-by: Eric Anholt <eric@anholt.net>


You can run piglit, deqp, or the gles cts to see the crashes.  No output is generated.

I'm bisecting to get a more precise commit.
Comment 1 Kenneth Graunke 2016-12-30 10:50:44 UTC 
It's

commit 41dd6c35396434be53581b59c4b477dd95e8b774
Author: Timothy Arceri <timothy.arceri@collabora.com>
Date:   Thu Dec 29 08:56:43 2016 +1100

    mesa/glsl: move subroutine metadata to gl_program


Something is getting confused and thinking there are subroutines (which is absurd, this is i915, we don't have GL 4.0 features)
Comment 2 Kenneth Graunke 2016-12-30 11:16:44 UTC 
Tim, this is crashing because ir_to_mesa.cpp's get_mesa_program() is doing:

   prog->arb.NumInstructions = num_instructions;

which happens to alias prog->sh.NumSubroutineUniformRemapTable in the union, so _mesa_program_init_subroutine_defaults() thinks it has a remap table to work with.

This is for a GLSL vertex shader.  ir_to_mesa gets called for GLSL programs too on i915, so the fact that it's setting ARB fields seems kinda bogus.  Then again, maybe the subroutine code isn't checking that it's a GLSL program, either?

I'll let you figure out the best solution.
Comment 3 Kenneth Graunke 2016-12-30 11:29:02 UTC 
Commit c3df65c123c6392b0b116900395a89fd3dbb9b85

    st/mesa/r200/i915/i965: move ARB program fields into a union

looks pretty bogus to me - those "ARB" fields are totally used for GLSL programs on i915 fragment programs, and swrast/prog_execute.  They can't be in a union.

Some drivers may not need those fields but the ones that do need them regardless of whether a program is GLSL, ARB, or fixed function.
Comment 4 Timothy Arceri 2016-12-30 20:48:03 UTC 
I've sent a fix: 

https://patchwork.freedesktop.org/patch/129903/
Comment 5 Timothy Arceri 2016-12-30 22:00:59 UTC 
Fix pushed:

commit 9d99dc4bc1fda9906e8dc576d6116fbdb05f67ac
Author: Timothy Arceri <timothy.arceri@collabora.com>
Date:   Sat Dec 31 07:45:35 2016 +1100

    mesa: make union in gl_program a struct and add FIXME
    
    i915 is mixing the use of these fields, for now change this to a
    struct and add a FIXME.
    
    Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99229

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.