Bug 74143 - [SNB Regression]Piglit/glx_GLX_ARB_create_context_current_with_no_framebuffer segfault
Summary: [SNB Regression]Piglit/glx_GLX_ARB_create_context_current_with_no_framebuffer...
Status: VERIFIED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Drivers/DRI/i965 (show other bugs)
Version: unspecified
Hardware: All Linux (All)
: high major
Assignee: Kristian Høgsberg
QA Contact: Intel 3D Bugs Mailing List
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-01-28 08:52 UTC by lu hua
Modified: 2014-02-08 02:51 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
patch to fix the issue (4.93 KB, patch)
2014-02-03 13:09 UTC, Tapani Pälli
Details | Splinter Review
Fix for GLX makecurrent with None drawable/readable. (2.96 KB, patch)
2014-02-05 19:47 UTC, Kristian Høgsberg
Details | Splinter Review

Description lu hua 2014-01-28 08:52:58 UTC
System Environment:
--------------------------
Platform: Sandybridge
Libdrm:		(master)libdrm-2.4.52-2-gcbb31f2d6e674513c0f66484c414475baba09153
Mesa:		(master)e5e4120723f7aa927faadca96910d4e8cbd99f40
Xserver:	(master)xorg-server-1.15.0-558-gc1ce807d9f18f215332d7eeb844e8c640f71c53c
Xf86_video_intel:(master)2.99.907-54-g294180b3bf78f2c0ae2f1197e1c0819d99009356
Cairo:		(master)2a7f133639bed86a29dd9693cb78c0aa42eaff30
Libva:		(staging)79ddde6bab54618046f136471c020c08ff4fde50
Libva_intel_driver:(staging)6736a39fce442a8e5abb2a2e99adf56e3965d35f
Kernel:	(drm-intel-nightly) 164a4cb4c1431a0689f85507868356fae24da638

Bug detailed description:
-----------------------------
It segfault on Sandybridge with mesa master branch. It works well on Mesa 10.0 branch.

Test on latest commit, It fails.

Test it on 1032c33cb93f1e8839be0f743b81492c2ca87e39 with nightly build tar file, It works well. I rebuild this commit, It still fails.

Also test on latest Mesa commit with earlier /opt/X11R7.unstable/lib/dri/i965_dri.so, It works well.

output:
Segmentation fault (core dumped)

Reproduce steps:
----------------------------
1. xinit
2. ./bin/glx-create-context-current-no-framebuffer -fbo -auto
Comment 1 Tapani Pälli 2014-02-03 10:17:53 UTC
bisected to following commit, it looks like commit breaks the case as there is no more checking for the BUFFER_BACK_LEFT renderbuffer which does not exist.

--- 8< ---
commit 11baad35088dfd4bdabc1710df650dbfb413e7a3
Author: Kristian Høgsberg <krh@bitplanet.net>
Date:   Tue Jan 21 12:17:03 2014 -0800

    intel: Fix initial MakeCurrent for single-buffer drawables
Comment 2 Tapani Pälli 2014-02-03 13:09:58 UTC
Created attachment 93282 [details] [review]
patch to fix the issue
Comment 3 Tapani Pälli 2014-02-05 11:15:45 UTC
Kristian, could you review the changes?
Comment 4 Kristian Høgsberg 2014-02-05 19:47:36 UTC
Created attachment 93487 [details] [review]
Fix for GLX makecurrent with None drawable/readable.

That's not the right fix and this was never working right except by accident.  The attached patch is what we want.  I'll send it to mesa-dev for review.
Comment 5 Kristian Høgsberg 2014-02-06 05:15:14 UTC
Adding irc discussion with Ian here:

12:15 < idr> krh: In the None drawable case, both read and draw must be None.  
             Is that checked somewhere?
...
12:29 < krh> idr: seems like something we should do in MakeContextCurrent, but 
             I don't see it there
12:30 < krh> idr: if only one of them is NULL, the dri driver will crash
12:30 < idr> I'm also not sure how we'll enforce the "...and the OpenGL version 
             supported by <ctx> is 3.0 or greater." requirement. :(
12:30 < idr> Now or with your patch?
12:31 < krh> without my patch, the driver would never see a NULL draw/readable
12:32 < krh> so it wouldn't hit that case

I think further checking and validation of the extension makes sense, but it's orthogonal to fixing this regression.  Once change in behavior is that without the patch in comment 4, passing a None and a non-None drawable to glxMakeContext Current I get

X Error of failed request:  BadDrawable (invalid Pixmap or Window parameter)
  Major opcode of failed request:  152 (DRI2)
  Minor opcode of failed request:  3 (DRI2CreateDrawable)
  Resource id in failed request:  0x0
  Serial number of failed request:  35
  Current serial number in output stream:  37

with the patch we segfault instead.

So more work is needed to better check input, but that's not a new problem.  Unless Ian objects, I'll commit this tomorrow.
Comment 6 Kristian Høgsberg 2014-02-06 22:24:57 UTC
Committed to master:

commit f658150639c36eda351590e757247c56507f494f
Author: Kristian Høgsberg <krh@bitplanet.net>
Date:   Wed Feb 5 11:43:58 2014 -0800

    glx: Pass NULL DRI drawables into the DRI driver for None GLX drawables
    
    GLX_ARB_create_context allows making a GLX context current with None
    drawable and readables, but this was never implemented correctly in GLX.
    We would create a __DRIdrawable for the None GLX drawable and pass that
    to the DRI driver and that would somehow work.  Now it's somehow broken.
    
    The way this should have worked is that we pass a NULL DRI drawable
    to the DRI driver when the GLX user calls glXMakeContextCurrent()
    with None for drawable and readables.
    
    https://bugs.freedesktop.org/show_bug.cgi?id=74143
    Signed-off-by: Kristian Høgsberg <krh@bitplanet.net>
Comment 7 lu hua 2014-02-08 02:51:59 UTC
Verified.Fixed.


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.