[Dri-devel] BIG potential binary compatibility problem found

Thu, 25 Mar 2004 18:10:59 -0800

Well boys and girls, it looks like I took just enough rope to shoot all of us in the foot. As part of the new interface work, I added a field to the __GLXContextRec to track the fbconfig used to create it. That field is then only used in driBindContext2 (lib/GL/dri/dri_util.c). The problem is that this field is OUTSIDE the __DRIContextRec portion of the __GLXContextRec. When I created the new __DRIContextRec (in include/GL/internal/dri_interface.h in the Mesa tree) added a mode field, and some other fields.

Since *_dri.so binaries compiled before dri_interface.h was used (it is currently only used in my private tree) expect mode to be at offset N, but a libGL.so compiled with dri_interface.h has it at offset N+12. Basically, by having the *_dri.so access anything outside the __DRIContextRec part it is impossible to add any fields to __DRIContextRec. Oops.

I did a bit of research, and this bug only exists in DRI CVS. The exposure is limited to our binary snapshots. Attached if a patch that fixes the problem. I am able to detect the bad libGL version in __driUtilCreateNewScreen. When version 20031201 is detected, the driver will refuse to load. That prevents weird crashes when a broken libGL is used with a fixed driver. I don't see a way to fix things to other way around. If a fixed libGL is used with a broken driver, it will just inexplicably crash.

There is a way to work around problems like this in the future. Each *_dri.so can export a new symbol, something like __driDesiredAPIVersion, that contains the highest version the driver wants to use. If this symbol already existed, libGL could do the following to reject broken drivers:

   const unsigned *desired_version =
      (const unsigned *) dlsym(handle, "__driDesiredAPIVersion");
   if ( *desired_version == 20031201 ) {
      ErrorMessageF("Driver uses broken API.  Upgrade.");
      Xfree(driver);
      dlclose(handle);
      return NULL;
   }

This also provides a mechanism to deprecate the "old" DRI API. The attached patch does not include any __driDesiredAPIVersion stuff.

Comments & opinions are welcome, as always.

Index: lib/GL/dri/dri_util.c
===================================================================
RCS file: /cvs/dri/xc/xc/lib/GL/dri/dri_util.c,v
retrieving revision 1.29
diff -u -d -r1.29 dri_util.c
--- a/lib/GL/dri/dri_util.c     3 Mar 2004 21:57:59 -0000       1.29
+++ b/lib/GL/dri/dri_util.c     26 Mar 2004 02:05:28 -0000
@@ -433,9 +433,9 @@
        return GL_FALSE;
     }
 #else
-    if ( driCompareGLXAPIVersion( 20031201 ) >= 0 ) {
+    if ( driCompareGLXAPIVersion( 20040317 ) >= 0 ) {
 #endif /* DRI_NEW_INTERFACE_ONLY */
-       modes = gc->mode;
+       modes = gc->driContext.mode;
 #ifndef DRI_NEW_INTERFACE_ONLY
     }
     else {
@@ -1108,6 +1108,14 @@
     __DRIscreenPrivate *psp;
 
 
+    if ( internal_api_version == 20031201 ) {
+       fprintf( stderr, "libGL error: libGL version 20031201 has critical "
+                "binary compatilibity bugs.\nlibGL error: You must upgrade "
+                "to use direct-rendering!\n" );
+       return NULL;
+    }
+
+
     window_exists = (PFNGLXWINDOWEXISTSPROC)
        glXGetProcAddress( (const GLubyte *) "__glXWindowExists" );
 
Index: lib/GL/glx/glxclient.h
===================================================================
RCS file: /cvs/dri/xc/xc/lib/GL/glx/glxclient.h,v
retrieving revision 1.48
diff -u -d -r1.48 glxclient.h
--- a/lib/GL/glx/glxclient.h    3 Mar 2004 21:58:00 -0000       1.48
+++ b/lib/GL/glx/glxclient.h    26 Mar 2004 02:05:28 -0000
@@ -316,6 +316,13 @@
      */
     Bool (*unbindContext2)(Display *dpy, int scrn, GLXDrawable draw,
                           GLXDrawable read, GLXContext gc);
+
+    /**
+     * Pointer to the mode used to create this context.
+     * 
+     * \since Internal API version 20040317.
+     */
+    const __GLcontextModes * mode;
 };
 
 /**
@@ -810,16 +817,6 @@
     * drivers should NEVER use this data or even care that it exists.
     */
    void * client_state_private;
-
-#ifdef GLX_DIRECT_RENDERING
-    /**
-     * Pointer to the mode used to create this context.  This field replaces
-     * the \c vid and \c fbconfigID fields.
-     * 
-     * \since Internal API version 20031201.
-     */
-    const __GLcontextModes * mode;
-#endif /* GLX_DIRECT_RENDERING */
 };
 
 #define __glXSetError(gc,code) \
Index: lib/GL/glx/glxcmds.c
===================================================================
RCS file: /cvs/dri/xc/xc/lib/GL/glx/glxcmds.c,v
retrieving revision 1.77
diff -u -d -r1.77 glxcmds.c
--- a/lib/GL/glx/glxcmds.c      3 Mar 2004 21:58:00 -0000       1.77
+++ b/lib/GL/glx/glxcmds.c      26 Mar 2004 02:05:28 -0000
@@ -583,7 +583,7 @@
                    gc->screen = mode->screen;
                    gc->vid = mode->visualID;
                    gc->fbconfigID = mode->fbconfigID;
-                   gc->mode = mode;
+                   gc->driContext.mode = mode;
                }
            }
        }
@@ -3298,8 +3298,11 @@
      *            deprecated GLX_NV_vertex_array_range & GLX_MESA_agp_offset
      *            interfaces.
      * 20031201 - Added support for the first round of DRI interface changes.
+     *            Do NOT test against this version!  It has binary
+     *            compatibility bugs, use 20040317 instead.
+     * 20040317 - Added the 'mode' field to __DRIcontextRec.
      */
-    return 20031201;
+    return 20040317;
 }

Reply via email to