Bug 99229 - [G33] thousands of tests crash
Summary: [G33] thousands of tests crash
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Drivers/DRI/i915 (show other bugs)
Version: git
Hardware: Other All
: medium normal
Assignee: Timothy Arceri
QA Contact: Default DRI bug account
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-12-30 09:43 UTC by Mark Janes
Modified: 2017-01-06 01:05 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
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.