Bug 43896

Summary: Mesa assembly breaks Super Meat Boy, Shank
Product: Mesa Reporter: Sven Arvidsson <sa>
Component: Mesa coreAssignee: mesa-dev
Status: RESOLVED NOTOURBUG QA Contact:
Severity: normal    
Priority: medium CC: imamdxl8805, marti, mnemonico, sidicas2
Version: 7.11   
Hardware: Other   
OS: All   
URL: http://www.humblebundle.com/
Whiteboard:
i915 platform: i915 features:
Attachments: [PATCH] glapi: fix _glapi_get_proc_addresss

Description Sven Arvidsson 2011-12-16 11:04:00 UTC
The games Super Meat Boy and Shank from The Humble Indie Bundle does not work correctly when the assembler optimizations in Mesa is used.

Super Meat Boy will either give an error "MojoShader compile failed" or segfault. Shank runs, but does not render correctly or, using git master, will segfault on start.

All of these problems go away if Mesa is compiled with the --disable-asm flag. 

Both the error message from the game and the backtraces from the segfaults point to the games themselves so I'm not sure if this is Mesa exposing a bug in the games or the other way around. I did however notice that Fedora explicitly makes use of the flag to turn the assembly off with this comment "i do not have words for how much the assembly dispatch code infuriates me".

A bug report for the games is filed here:
https://bugzilla.icculus.org/show_bug.cgi?id=5315


System environment:
-- system architecture: 32-bit
-- Linux distribution: Debian unstable
-- GPU: REDWOOD
-- Model: XFX Radeon HD 5670 1GB
-- Display connector: DVI
-- xf86-video-ati: 6.14.3
-- xserver: 1.11.2.902
-- mesa: 
-- drm: 2.4.29
-- kernel: 3.1.5
Comment 1 Vadim Girlin 2011-12-17 03:42:43 UTC
Created attachment 54524 [details] [review]
[PATCH] glapi: fix _glapi_get_proc_addresss

I'm not sure if the patch is completely correct, though there are no piglit regressions.

There is a bug in the 32-bit mesa build, when asm is enabled - glXGetProcAddress returns non-NULL for any string starting with "gl".

In this case the app calls glXGetProcAddress with the incorrect function name, e.g. "glCreateShaderObject", mesa incorrectly returns non-NULL, the app calls it.
Comment 2 Henri Verbeet 2011-12-17 05:16:28 UTC
(In reply to comment #1)
> There is a bug in the 32-bit mesa build, when asm is enabled -
> glXGetProcAddress returns non-NULL for any string starting with "gl".
> 
> In this case the app calls glXGetProcAddress with the incorrect function name,
> e.g. "glCreateShaderObject", mesa incorrectly returns non-NULL, the app calls
> it.
Sounds like a bug in the application to me. From the GLX 1.4 spec (though that's certainly not new in 1.4):

"A non-NULL return value for glXGetProcAddress does not guarantee that an extension function is actually supported at runtime. The client must also query glGetString() or glXQueryExtensionsString to determine if an extension is supported by a particular context."
Comment 3 Vadim Girlin 2011-12-17 05:40:34 UTC
(In reply to comment #2)
> Sounds like a bug in the application to me. From the GLX 1.4 spec (though
> that's certainly not new in 1.4):
> 
> "A non-NULL return value for glXGetProcAddress does not guarantee that an
> extension function is actually supported at runtime. The client must also query
> glGetString() or glXQueryExtensionsString to determine if an extension is
> supported by a particular context."

I'm not sure what behaviour is correct. I relied on the the description of glXGetProcAddress: http://www.opengl.org/sdk/docs/man/xhtml/glXGetProcAddress.xml

"A NULL pointer is returned if function requested is not suported in the implementation being queried."
Comment 4 Vadim Girlin 2011-12-17 06:36:09 UTC
(In reply to comment #2)
> (In reply to comment #1)
> > There is a bug in the 32-bit mesa build, when asm is enabled -
> > glXGetProcAddress returns non-NULL for any string starting with "gl".
> > 
> > In this case the app calls glXGetProcAddress with the incorrect function name,
> > e.g. "glCreateShaderObject", mesa incorrectly returns non-NULL, the app calls
> > it.
> Sounds like a bug in the application to me. From the GLX 1.4 spec (though
> that's certainly not new in 1.4):
> 
> "A non-NULL return value for glXGetProcAddress does not guarantee that an
> extension function is actually supported at runtime. The client must also query
> glGetString() or glXQueryExtensionsString to determine if an extension is
> supported by a particular context."

AFAICS the problem is that extension is supported (ARB_shader_objects), but the app tries glxGetProcAddress with the wrong name first (glCreateShaderObject), and if it returns NULL, then the app tries the same name with the "ARB" suffix and gets the correct pointer. When glXGetProcAddress returns non-NULL for the wrong name, the app just uses the returned pointer, which leads to nowhere. You can view the code here (see loadsym function) :

http://hg.icculus.org/icculus/mojoshader/file/12e1db42bf75/mojoshader_opengl.c
Comment 5 Brian Paul 2011-12-19 07:00:39 UTC
(In reply to comment #3)
> (In reply to comment #2)
> > Sounds like a bug in the application to me. From the GLX 1.4 spec (though
> > that's certainly not new in 1.4):
> > 
> > "A non-NULL return value for glXGetProcAddress does not guarantee that an
> > extension function is actually supported at runtime. The client must also query
> > glGetString() or glXQueryExtensionsString to determine if an extension is
> > supported by a particular context."
> 
> I'm not sure what behaviour is correct. I relied on the the description of
> glXGetProcAddress:
> http://www.opengl.org/sdk/docs/man/xhtml/glXGetProcAddress.xml
> 
> "A NULL pointer is returned if function requested is not suported in the
> implementation being queried."

There's no conflict between the quotes above.  Getting a non-NULL pointer is no indication whatsoever about whether the extension is supported or not supported.

The reason is this:  suppose the very first call in main() is glXGetProcAddress("glGenTransformFeedbacks").  At this point, the GL library can't determine whether the extension is supported because it hasn't even opened an X display connection.  Depending on which display/GPU is used, the extension may or may not be supported.  So if libGL implements the function (or can generate a dispatch stub for it) it must return the pointer for it right away.

Later on, the app has to check GL_EXTENSIONS to see if GL_ARB_transform_feedback2 is really supported.  If the extension is not supported and the function is called anyway, things might blow up.

This is an application bug.
Comment 6 Ryan C. Gordon 2011-12-20 19:25:49 UTC
Brian is correct, this is my bug in MojoShader; that code is goofy, and I'll fix it soon (and push out an updated Super Meat Boy).

My apologies for this generating traffic on your bug tracker.

--ryan.

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.