Don't use the 'NULL' GModule to resolve GL symbols

Previously, _cogl_get_proc_address had a fallback to resolve the
symbol using g_module_open(NULL) to get the symbol from anywhere in
the address space. The EGL backend ends up using this on some drivers
because eglGetProcAddress isn't meant to return a pointer for core
functions. This causes problems if something in the process is linking
against a different GL library, for example Cairo may be linking
against libGL itself. In this case it may end up resolving symbols
from the GL library even if GLES is being used.

This patch removes the fallback. The EGL version now has its own
fallback instead which passes the existing libgl_module from the
renderer to g_module_symbol so that it should only get symbols from
that library or its dependency chain. The GLX and WGL winsys only call
glXGetProcAddress and wglGetProcAddress. The stub winsys does however
continue using the global symbol lookup.

The internal _cogl_get_proc_address function has been renamed to
_cogl_renderer_get_proc_address because it needs a connected renderer
to work so it could be considered to be a renderer method. The pointer
to the renderer is passed down to the winsys backends so that it can
use the data attached to the renderer to get the module pointers.

https://bugzilla.gnome.org/show_bug.cgi?id=655412

Reviewed-by: Robert Bragg <robert@linux.intel.com>

cogl/cogl-feature-private.c

@@ -31,9 +31,10 @@
 #include "cogl-context-private.h"
 
 #include "cogl-feature-private.h"
+#include "cogl-renderer-private.h"
 
 gboolean
-_cogl_feature_check (const CoglWinsysVtable *winsys,
+_cogl_feature_check (CoglRenderer *renderer,
                      const char *driver_prefix,
                      const CoglFeatureData *data,
                      int gl_major,
@@ -122,7 +123,7 @@ _cogl_feature_check (const CoglWinsysVtable *winsys,
 
       full_function_name = g_strconcat (data->functions[func_num].name,
                                         suffix, NULL);
-      func = _cogl_get_proc_address (winsys, full_function_name);
+      func = _cogl_renderer_get_proc_address (renderer, full_function_name);
       g_free (full_function_name);
 
       if (func == NULL)
@@ -189,7 +190,7 @@ _cogl_feature_check_ext_functions (CoglContext *context,
   int i;
 
   for (i = 0; i < G_N_ELEMENTS (cogl_feature_ext_functions_data); i++)
-    _cogl_feature_check (_cogl_context_get_winsys (context),
+    _cogl_feature_check (context->display->renderer,
                          "GL", cogl_feature_ext_functions_data + i,
                          gl_major, gl_minor, context->driver,
                          gl_extensions,

cogl/cogl-feature-private.h

@@ -81,7 +81,7 @@ struct _CoglFeatureData
 };
 
 gboolean
-_cogl_feature_check (const CoglWinsysVtable *winsys,
+_cogl_feature_check (CoglRenderer *renderer,
                      const char *driver_prefix,
                      const CoglFeatureData *data,
                      int gl_major,

cogl/cogl-glx-renderer-private.h

@@ -94,6 +94,9 @@ typedef struct _CoglGLXRenderer
   XVisualInfo *
   (* glXGetVisualFromFBConfig) (Display *dpy, GLXFBConfig config);
 
+  void *
+  (* glXGetProcAddress) (const GLubyte *procName);
+
   /* Function pointers for GLX specific extensions */
 #define COGL_WINSYS_FEATURE_BEGIN(a, b, c, d, e, f)
 

cogl/cogl-renderer-private.h

@@ -79,4 +79,8 @@ _cogl_renderer_remove_native_filter (CoglRenderer *renderer,
                                      CoglNativeFilterFunc func,
                                      void *data);
 
+void *
+_cogl_renderer_get_proc_address (CoglRenderer *renderer,
+                                 const char *name);
+
 #endif /* __COGL_RENDERER_PRIVATE_H */

cogl/cogl-renderer.c

@@ -379,3 +379,12 @@ cogl_renderer_get_winsys_id (CoglRenderer *renderer)
 
   return renderer->winsys_vtable->id;
 }
+
+void *
+_cogl_renderer_get_proc_address (CoglRenderer *renderer,
+                                 const char *name)
+{
+  const CoglWinsysVtable *winsys = _cogl_renderer_get_winsys (renderer);
+
+  return winsys->renderer_get_proc_address (renderer, name);
+}

cogl/cogl.c

@@ -46,6 +46,7 @@
 #include "cogl-texture-driver.h"
 #include "cogl-attribute-private.h"
 #include "cogl-framebuffer-private.h"
+#include "cogl-renderer-private.h"
 
 #ifndef GL_PACK_INVERT_MESA
 #define GL_PACK_INVERT_MESA 0x8758
@@ -92,13 +93,9 @@ cogl_gl_error_to_string (GLenum error_code)
 CoglFuncPtr
 cogl_get_proc_address (const char* name)
 {
-  const CoglWinsysVtable *winsys;
-
   _COGL_GET_CONTEXT (ctx, NULL);
 
-  winsys = _cogl_context_get_winsys (ctx);
-
-  return _cogl_get_proc_address (winsys, name);
+  return _cogl_renderer_get_proc_address (ctx->display->renderer, name);
 }
 
 gboolean

cogl/driver/gl/cogl-gl.c

@@ -33,6 +33,7 @@
 #include "cogl-internal.h"
 #include "cogl-context-private.h"
 #include "cogl-feature-private.h"
+#include "cogl-renderer-private.h"
 
 static gboolean
 _cogl_get_gl_version (int *major_out, int *minor_out)
@@ -137,7 +138,7 @@ _cogl_gl_update_features (CoglContext *context,
      function because we need to use it to determine what functions we
      can expect */
   context->glGetString =
-    (void *) _cogl_get_proc_address (_cogl_context_get_winsys (context),
+    (void *) _cogl_renderer_get_proc_address (context->display->renderer,
                                               "glGetString");
 
   if (!check_gl_version (context, error))

cogl/driver/gles/cogl-gles.c

@@ -31,6 +31,7 @@
 #include "cogl-internal.h"
 #include "cogl-context-private.h"
 #include "cogl-feature-private.h"
+#include "cogl-renderer-private.h"
 
 gboolean
 _cogl_gles_update_features (CoglContext *context,
@@ -45,7 +46,7 @@ _cogl_gles_update_features (CoglContext *context,
      function because we need to use it to determine what functions we
      can expect */
   context->glGetString =
-    (void *) _cogl_get_proc_address (_cogl_context_get_winsys (context),
+    (void *) _cogl_renderer_get_proc_address (context->display->renderer,
                                               "glGetString");
 
   COGL_NOTE (WINSYS,

cogl/winsys/cogl-winsys-egl.c

@@ -212,9 +212,19 @@ static const CoglFeatureData winsys_feature_data[] =
   };
 
 static CoglFuncPtr
-_cogl_winsys_get_proc_address (const char *name)
+_cogl_winsys_renderer_get_proc_address (CoglRenderer *renderer,
+                                        const char *name)
 {
-  return (CoglFuncPtr) eglGetProcAddress (name);
+  void *ptr;
+
+  ptr = eglGetProcAddress (name);
+
+  /* eglGetProcAddress doesn't support fetching core API so we need to
+     get that separately with GModule */
+  if (ptr == NULL)
+    g_module_symbol (renderer->libgl_module, name, &ptr);
+
+  return ptr;
 }
 
 #ifdef COGL_HAS_EGL_PLATFORM_ANDROID_SUPPORT
@@ -342,7 +352,6 @@ static void
 check_egl_extensions (CoglRenderer *renderer)
 {
   CoglRendererEGL *egl_renderer = renderer->winsys;
-  const CoglWinsysVtable *winsys = renderer->winsys_vtable;
   const char *egl_extensions;
   int i;
 
@@ -352,7 +361,7 @@ check_egl_extensions (CoglRenderer *renderer)
 
   egl_renderer->private_features = 0;
   for (i = 0; i < G_N_ELEMENTS (winsys_feature_data); i++)
-    if (_cogl_feature_check (winsys,
+    if (_cogl_feature_check (renderer,
                              "EGL", winsys_feature_data + i, 0, 0,
                              COGL_DRIVER_GL, /* the driver isn't used */
                              egl_extensions,
@@ -1657,7 +1666,7 @@ static CoglWinsysVtable _cogl_winsys_vtable =
   {
     .id = COGL_WINSYS_ID_EGL,
     .name = "EGL",
-    .get_proc_address = _cogl_winsys_get_proc_address,
+    .renderer_get_proc_address = _cogl_winsys_renderer_get_proc_address,
     .renderer_connect = _cogl_winsys_renderer_connect,
     .renderer_disconnect = _cogl_winsys_renderer_disconnect,
     .display_setup = _cogl_winsys_display_setup,

cogl/winsys/cogl-winsys-glx.c

@@ -56,8 +56,6 @@
 #include <GL/glx.h>
 #include <X11/Xlib.h>
 
-typedef CoglFuncPtr (*GLXGetProcAddressProc) (const GLubyte *procName);
-
 #ifdef HAVE_DRM
 #include <drm.h>
 #include <sys/ioctl.h>
@@ -138,44 +136,12 @@ static const CoglFeatureData winsys_feature_data[] =
   };
 
 static CoglFuncPtr
-_cogl_winsys_get_proc_address (const char *name)
+_cogl_winsys_renderer_get_proc_address (CoglRenderer *renderer,
+                                        const char *name)
 {
-  static GLXGetProcAddressProc get_proc_func = NULL;
-  static void *dlhand = NULL;
+  CoglGLXRenderer *glx_renderer = renderer->winsys;
 
-  if (get_proc_func == NULL && dlhand == NULL)
-    {
-      dlhand = dlopen (NULL, RTLD_LAZY);
-
-      if (!dlhand)
-        {
-          g_warning ("Failed to dlopen (NULL, RTDL_LAZY): %s", dlerror ());
-          return NULL;
-        }
-
-      dlerror ();
-
-      get_proc_func =
-        (GLXGetProcAddressProc) dlsym (dlhand, "glXGetProcAddress");
-
-      if (dlerror () != NULL)
-        {
-          get_proc_func =
-            (GLXGetProcAddressProc) dlsym (dlhand, "glXGetProcAddressARB");
-        }
-
-      if (dlerror () != NULL)
-        {
-          get_proc_func = NULL;
-          g_warning ("failed to bind GLXGetProcAddress "
-                     "or GLXGetProcAddressARB");
-        }
-    }
-
-  if (get_proc_func)
-    return get_proc_func ((GLubyte *) name);
-
-  return NULL;
+  return glx_renderer->glXGetProcAddress ((const GLubyte *) name);
 }
 
 static CoglOnscreen *
@@ -316,7 +282,11 @@ resolve_core_glx_functions (CoglRenderer *renderer,
       !g_module_symbol (glx_renderer->libgl_module, "glXDestroyWindow",
                         (void **) &glx_renderer->glXDestroyWindow) ||
       !g_module_symbol (glx_renderer->libgl_module, "glXQueryExtensionsString",
-                        (void **) &glx_renderer->glXQueryExtensionsString))
+                        (void **) &glx_renderer->glXQueryExtensionsString) ||
+      (!g_module_symbol (glx_renderer->libgl_module, "glXGetProcAddress",
+                         (void **) &glx_renderer->glXGetProcAddress) &&
+       !g_module_symbol (glx_renderer->libgl_module, "glXGetProcAddressARB",
+                         (void **) &glx_renderer->glXGetProcAddress)))
     {
       g_set_error (error, COGL_WINSYS_ERROR,
                    COGL_WINSYS_ERROR_INIT,
@@ -419,7 +389,7 @@ update_winsys_features (CoglContext *context, GError **error)
                   TRUE);
 
   for (i = 0; i < G_N_ELEMENTS (winsys_feature_data); i++)
-    if (_cogl_feature_check (_cogl_context_get_winsys (context),
+    if (_cogl_feature_check (context->display->renderer,
                              "GLX", winsys_feature_data + i, 0, 0,
                              COGL_DRIVER_GL, /* the driver isn't used */
                              glx_extensions,
@@ -1992,7 +1962,7 @@ static CoglWinsysVtable _cogl_winsys_vtable =
   {
     .id = COGL_WINSYS_ID_GLX,
     .name = "GLX",
-    .get_proc_address = _cogl_winsys_get_proc_address,
+    .renderer_get_proc_address = _cogl_winsys_renderer_get_proc_address,
     .renderer_connect = _cogl_winsys_renderer_connect,
     .renderer_disconnect = _cogl_winsys_renderer_disconnect,
     .display_setup = _cogl_winsys_display_setup,

cogl/winsys/cogl-winsys-private.h

@@ -63,7 +63,8 @@ typedef struct _CoglWinsysVtable
   /* Required functions */
 
   CoglFuncPtr
-  (*get_proc_address) (const char *name);
+  (*renderer_get_proc_address) (CoglRenderer *renderer,
+                                const char *name);
 
   gboolean
   (*renderer_connect) (CoglRenderer *renderer, GError **error);
@@ -158,8 +159,4 @@ typedef struct _CoglWinsysVtable
 gboolean
 _cogl_winsys_has_feature (CoglWinsysFeature feature);
 
-CoglFuncPtr
-_cogl_get_proc_address (const CoglWinsysVtable *winsys,
-                        const char *name);
-
 #endif /* __COGL_WINSYS_PRIVATE_H */

cogl/winsys/cogl-winsys-sdl.c

@@ -28,7 +28,8 @@
 #include "cogl.h"
 
 CoglFuncPtr
-_cogl_winsys_get_proc_address (const char *name)
+_cogl_winsys_get_proc_address (CoglRenderer *renderer,
+                               const char *name)
 {
   return NULL;
 }

cogl/winsys/cogl-winsys-stub.c

@@ -45,8 +45,24 @@ static int _cogl_winsys_stub_dummy_ptr;
  */
 
 static CoglFuncPtr
-_cogl_winsys_get_proc_address (const char *name)
+_cogl_winsys_renderer_get_proc_address (CoglRenderer *renderer,
+                                        const char *name)
 {
+  static GModule *module = NULL;
+
+  /* this should find the right function if the program is linked against a
+   * library providing it */
+  if (G_UNLIKELY (module == NULL))
+    module = g_module_open (NULL, 0);
+
+  if (module)
+    {
+      void *symbol;
+
+      if (g_module_symbol (module, name, &symbol))
+        return symbol;
+    }
+
   return NULL;
 }
 
@@ -134,7 +150,7 @@ static CoglWinsysVtable _cogl_winsys_vtable =
   {
     .id = COGL_WINSYS_ID_STUB,
     .name = "STUB",
-    .get_proc_address = _cogl_winsys_get_proc_address,
+    .renderer_get_proc_address = _cogl_winsys_renderer_get_proc_address,
     .renderer_connect = _cogl_winsys_renderer_connect,
     .renderer_disconnect = _cogl_winsys_renderer_disconnect,
     .display_setup = _cogl_winsys_display_setup,

cogl/winsys/cogl-winsys-wgl.c

@@ -122,7 +122,8 @@ static const CoglFeatureData winsys_feature_data[] =
   };
 
 static CoglFuncPtr
-_cogl_winsys_get_proc_address (const char *name)
+_cogl_winsys_renderer_get_proc_address (CoglRenderer *renderer,
+                                        const char *name)
 {
   return (CoglFuncPtr) wglGetProcAddress ((LPCSTR) name);
 }
@@ -569,7 +570,7 @@ update_winsys_features (CoglContext *context, GError **error)
       COGL_NOTE (WINSYS, "  WGL Extensions: %s", wgl_extensions);
 
       for (i = 0; i < G_N_ELEMENTS (winsys_feature_data); i++)
-        if (_cogl_feature_check (_cogl_context_get_winsys (context),
+        if (_cogl_feature_check (context->display->renderer,
                                  "WGL", winsys_feature_data + i, 0, 0,
                                  COGL_DRIVER_GL,
                                  wgl_extensions,
@@ -833,7 +834,7 @@ _cogl_winsys_wgl_get_vtable (void)
 
       vtable.id = COGL_WINSYS_ID_EGL;
       vtable.name = "WGL";
-      vtable.get_proc_address = _cogl_winsys_get_proc_address;
+      vtable.renderer_get_proc_address = _cogl_winsys_renderer_get_proc_address;
       vtable.renderer_connect = _cogl_winsys_renderer_connect;
       vtable.renderer_disconnect = _cogl_winsys_renderer_disconnect;
       vtable.display_setup = _cogl_winsys_display_setup;

cogl/winsys/cogl-winsys.c

@@ -45,32 +45,3 @@ _cogl_winsys_has_feature (CoglWinsysFeature feature)
 
   return COGL_FLAGS_GET (ctx->winsys_features, feature);
 }
-
-/* XXX: we would call this _cogl_winsys_get_proc_address but that
- * currently collides with the per winsys implementation names we use.
- * */
-CoglFuncPtr
-_cogl_get_proc_address (const CoglWinsysVtable *winsys,
-                        const char *name)
-{
-  void *address = winsys->get_proc_address (name);
-  static GModule *module = NULL;
-
-  if (address)
-    return address;
-
-  /* this should find the right function if the program is linked against a
-   * library providing it */
-  if (G_UNLIKELY (module == NULL))
-    module = g_module_open (NULL, 0);
-
-  if (module)
-    {
-      gpointer symbol;
-
-      if (g_module_symbol (module, name, &symbol))
-        return symbol;
-    }
-
-  return NULL;
-}