Bug 43896

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.

(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."

(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."

(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

(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.

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.