Bug 54301 - [Bisected ILK regression]Piglit glx_GLX_ARB_create_context_forward-compatible_flag_with_3.0 Segmentation fault
Summary: [Bisected ILK regression]Piglit glx_GLX_ARB_create_context_forward-compatible...
Status: VERIFIED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Drivers/DRI/i965 (show other bugs)
Version: git
Hardware: All Linux (All)
: high major
Assignee: Ian Romanick
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-08-31 05:32 UTC by lu hua
Modified: 2012-10-23 05:30 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
Proposed fix (1.06 KB, patch)
2012-08-31 22:03 UTC, Ian Romanick
Details | Splinter Review
Proposed fix, v2 (1.67 KB, patch)
2012-09-05 22:15 UTC, Ian Romanick
Details | Splinter Review
The rest of the fix (1.15 KB, patch)
2012-09-26 18:14 UTC, Ian Romanick
Details | Splinter Review

Description lu hua 2012-08-31 05:32:27 UTC
System Environment:
--------------------------
Arch:             x86_64
Platform:         Ironlake
Libdrm:  (master)libdrm-2.4.39-1-g7080bfdfd9b6c5f003daaef37ae9c329f2d46a6c
Mesa:    (master)83933606596a0197e6cd1740cb439eaf055e544d
Xserver:(master)xorg-server-1.12.99.905-12-g148583d62b84832369e9df39b3e527b99ca96761
Xf86_video_intel:(master)2.20.5-23-gdeaa1cac269be03f4ec44092f70349ff466d59de
Libva:    (staging)3c9f3d7dbd0e1b53cd47ac42984e7a563853f194
Libva_intel_driver:(staging)658ba48f5586dacd051a2d99504beedea9f7a792
Kernel:    (drm-intel-next-queued) 0e0428baf7c156bc2ba8a3fd311399c1afd88f54

Bug detailed description:
-------------------------
It happens on ironlake with mesa master branch. It doesn't happen on mesa 8.0 branch.
The last known good commit:67e9ae856355be532455c1cf1211d59b3a4c5992.
The last known bad commit:83933606596a0197e6cd1740cb439eaf055e544d.

(gdb) bt
#0  intelDestroyContext (driContextPriv=0x61f4c0) at intel_context.c:800
#1  0x00007ffff4ccd7c1 in intelCreateContext (api=<optimized out>, mesaVis=<optimized out>, driContextPriv=0x61f4c0, major_version=<optimized out>,
    minor_version=<optimized out>, flags=<optimized out>, error=0x7fffffffe00c, sharedContextPrivate=0x0) at intel_screen.c:704
#2  0x00007ffff4d47bd4 in dri2CreateContextAttribs (screen=0x6199d0, api=<optimized out>, config=0x61cc00, shared=<optimized out>, num_attribs=<optimized out>,
    attribs=<optimized out>, error=0x7fffffffe00c, data=0x610db0) at dri_util.c:287
#3  0x00007ffff7aa9601 in dri2_create_context_attribs (base=0x610cb0, config_base=0x61df30, shareList=<optimized out>, num_attribs=<optimized out>, attribs=<optimized out>,
    error=0x7fffffffe00c) at dri2_glx.c:299
#4  0x00007ffff7a7a466 in glXCreateContextAttribsARB (dpy=0x602010, config=0x61df30, share_context=0x0, direct=1, attrib_list=0x7fffffffe060) at create_context.c:78
#5  0x000000000040114d in try_flag (flag=0) at /GFX/Test/Piglit/piglit/tests/spec/glx_arb_create_context/valid-flag-forward-compatible.c:35
#6  0x00000000004011e9 in main (argc=1, argv=0x7fffffffe198) at /GFX/Test/Piglit/piglit/tests/spec/glx_arb_create_context/valid-flag-forward-compatible.c:63

Reproduce steps:
----------------------------
1. xinit
2. ./bin/glx-create-context-valid-flag-forward-compatible
Comment 1 lu hua 2012-08-31 05:44:00 UTC
Bisect shows:91473485fcf3e2cef465784ae5581787a2a8a3b3 is the first bad commit.
commit 91473485fcf3e2cef465784ae5581787a2a8a3b3
Author:     Ian Romanick <ian.d.romanick@intel.com>
AuthorDate: Tue Aug 7 12:46:22 2012 -0700
Commit:     Ian Romanick <ian.d.romanick@intel.com>
CommitDate: Wed Aug 29 15:09:37 2012 -0700

    intel: Clean up bits of cruft in intelCreateContext

    This and the previous three commits should probably be squashed together...

    Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
Comment 2 Ian Romanick 2012-08-31 22:03:13 UTC
Created attachment 66421 [details] [review]
Proposed fix

Can you try this patch?
Comment 3 lu hua 2012-09-03 05:30:15 UTC
(In reply to comment #2)
> Created attachment 66421 [details] [review] [review]
> Proposed fix
> 
> Can you try this patch?

Add this patch to the latest mesa master branch(commit a96119cc8c998).It still happens.
Comment 4 Ian Romanick 2012-09-05 22:15:54 UTC
Created attachment 66700 [details] [review]
Proposed fix, v2

Can you try this patch instead?
Comment 5 lu hua 2012-09-06 01:37:50 UTC
(In reply to comment #4)
> Created attachment 66700 [details] [review] [review]
> Proposed fix, v2
> 
> Can you try this patch instead?


Add this patch to commit d220e2de7fe7a75e70d68f2a, It still happens.
Comment 6 Ian Romanick 2012-09-13 16:08:42 UTC
(In reply to comment #5)
> (In reply to comment #4)
> > Created attachment 66700 [details] [review] [review] [review]
> > Proposed fix, v2
> > 
> > Can you try this patch instead?
> 
> 
> Add this patch to commit d220e2de7fe7a75e70d68f2a, It still happens.

Are you sure you applied the updated patch correctly?  I don't have access to an Ironlake machine right now, but I simulated it with the patch below.  This causes creation of any core-profile or forward-compatible context to fail.  Without attachment #66700 [details] [review] this causes an assertion failure at line 798 of intel_context.c (this would be the segfault you saw if I built without debug).  With attachment #66700 [details] [review] the test passes.


diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965
index 60b6454..00c9079 100644
--- a/src/mesa/drivers/dri/i965/brw_context.c
+++ b/src/mesa/drivers/dri/i965/brw_context.c
@@ -110,7 +110,7 @@ brwCreateContext(int api,
    case API_OPENGLES2:
       break;
    case API_OPENGL_CORE: {
-#ifdef TEXTURE_FLOAT_ENABLED
+#if 0 && defined TEXTURE_FLOAT_ENABLED
       const unsigned max_version =
          (screen->gen == 6 ||
           (screen->gen == 7 && screen->kernel_has_gen7_sol_reset))
Comment 7 Ian Romanick 2012-09-26 18:14:40 UTC
Created attachment 67741 [details] [review]
The rest of the fix

Okay... both patches are required to fix the problem.  I've tested it on my ILK system.  Can you verify it on yours and give me a Tested-by?
Comment 8 lu hua 2012-09-27 02:27:56 UTC
Add both patches 66700 and 66741 into the latest commit ff947c6d6583. This issue goes away.
Comment 9 Ian Romanick 2012-10-19 19:06:05 UTC
Fixed by the following series on master.  These changes have also been picked to the 9.0 branch.

commit e87c63f2889fcbeb5a8bbd91eda1333d7ed44bf2
Author: Ian Romanick <ian.d.romanick@intel.com>
Date:   Fri Sep 28 15:38:26 2012 -0700

    i965: brwInitVtbl needs to know the chipset generation
    
    Fixes major regressions since de958de.
    
    Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>

commit de958de71b1450952e021af4e729c87406353db6
Author: Ian Romanick <ian.d.romanick@intel.com>
Date:   Wed Sep 26 17:57:01 2012 -0700

    i915: Don't free the intel_context structure when intelCreateContext fails.
    
    intelDestroyContext will eventually be called, and it will clean things up.
    
    NOTE: This is a candidate for the 9.0 branch.
    
    Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
    Reviewed-by: Eric Anholt <eric@anholt.net>
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=53618

commit 87f26214d6bdeb439b30615ec53c293c5141cf11
Author: Ian Romanick <ian.d.romanick@intel.com>
Date:   Fri Aug 31 14:55:21 2012 -0700

    i965: Don't free the intel_context structure when intelCreateContext fails.
    
    intelDestroyContext will eventually be called, and it will clean things
    up.  The call to brwInitVtbl is moved earlier so that
    intelDestroyContext can call the device-specific destructor.  This also
    makes the code look more like the i915 code.
    
    NOTE: This is a candidate for the 9.0 branch.
    
    Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
    Reviewed-by: Eric Anholt <eric@anholt.net>
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=54301

commit 22897c74979aa02facdd5cd729db8dadf86924f5
Author: Ian Romanick <ian.d.romanick@intel.com>
Date:   Fri Sep 28 08:29:12 2012 -0700

    intel: Don't call intelDestroyContext if there is no context to destroy
    
    Some error paths in the device-specific context creation functions can exit
    before the deintel_context structure is allocated.
    
    NOTE: This is a candidate for the 9.0 branch.
    
    Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
    Reviewed-by: Eric Anholt <eric@anholt.net>
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=53618
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=54301

commit f93cb0bebb10e3e3e5df099be51021b211650356
Author: Ian Romanick <ian.d.romanick@intel.com>
Date:   Wed Sep 26 11:08:11 2012 -0700

    dri_util: Use calloc to allocate __DRIcontext
    
    The __DRIcontext contains some pointers, and some drivers check for them to 
    NULL in some failure paths.  Instead of sprinkling NULL assignments across t
    various drivers, just zero out the whole thing.
    
    NOTE: This is a candidate for the 9.0 branch.
    
    Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
    Reviewed-by: Eric Anholt <eric@anholt.net>
    Reviewed-and-tested-by: Kenneth Graunke <kenneth@whitecape.org>
    Tested-by: Lu Hua <huax.lu@intel.com>
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=53618
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=54301
Comment 10 lu hua 2012-10-23 05:30:59 UTC
Verified. Fixed on master branch commit:259fc154f1fdcabbc0a6c02c524962b063f9dee6.


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.