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>

15 files changed, 80 insertions, 99 deletions

diff --git a/cogl/cogl-feature-private.c b/cogl/cogl-feature-private.c
index 053bd026..44539da7 100644
--- a/cogl/cogl-feature-private.c
+++ b/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,
diff --git a/cogl/cogl-feature-private.h b/cogl/cogl-feature-private.h
index 1cf3eed5..c2cec7c4 100644
--- a/cogl/cogl-feature-private.h
+++ b/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,
diff --git a/cogl/cogl-glx-renderer-private.h b/cogl/cogl-glx-renderer-private.h
index 10162351..2d36a5f8 100644
--- a/cogl/cogl-glx-renderer-private.h
+++ b/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)
 
diff --git a/cogl/cogl-renderer-private.h b/cogl/cogl-renderer-private.h
index dc279bac..f85a8437 100644
--- a/cogl/cogl-renderer-private.h
+++ b/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 */
diff --git a/cogl/cogl-renderer.c b/cogl/cogl-renderer.c
index be73adfe..5a4f570e 100644
--- a/cogl/cogl-renderer.c
+++ b/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);
+}
diff --git a/cogl/cogl.c b/cogl/cogl.c
index 15b91489..e3eb3c31 100644
--- a/cogl/cogl.c
+++ b/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
diff --git a/cogl/driver/gl/cogl-gl.c b/cogl/driver/gl/cogl-gl.c
index 3ed4b04b..589d3da1 100644
--- a/cogl/driver/gl/cogl-gl.c
+++ b/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,8 +138,8 @@ _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),
-                                     "glGetString");
+    (void *) _cogl_renderer_get_proc_address (context->display->renderer,
+                                              "glGetString");
 
   if (!check_gl_version (context, error))
     return FALSE;
diff --git a/cogl/driver/gles/cogl-gles.c b/cogl/driver/gles/cogl-gles.c
index 8418a7ab..1dbe433c 100644
--- a/cogl/driver/gles/cogl-gles.c
+++ b/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,8 +46,8 @@ _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),
-                                     "glGetString");
+    (void *) _cogl_renderer_get_proc_address (context->display->renderer,
+                                              "glGetString");
 
   COGL_NOTE (WINSYS,
              "Checking features\n"
diff --git a/cogl/winsys/cogl-winsys-egl.c b/cogl/winsys/cogl-winsys-egl.c
index c0763337..14de1e37 100644
--- a/cogl/winsys/cogl-winsys-egl.c
+++ b/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,
diff --git a/cogl/winsys/cogl-winsys-glx.c b/cogl/winsys/cogl-winsys-glx.c
index 12d7a54a..a092bc7a 100644
--- a/cogl/winsys/cogl-winsys-glx.c
+++ b/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;
-
-  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);
+  CoglGLXRenderer *glx_renderer = renderer->winsys;
 
-  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,
diff --git a/cogl/winsys/cogl-winsys-private.h b/cogl/winsys/cogl-winsys-private.h
index 5822479d..88df22d4 100644
--- a/cogl/winsys/cogl-winsys-private.h
+++ b/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 */
diff --git a/cogl/winsys/cogl-winsys-sdl.c b/cogl/winsys/cogl-winsys-sdl.c
index 9c937e4b..bb8908b2 100644
--- a/cogl/winsys/cogl-winsys-sdl.c
+++ b/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;
 }
diff --git a/cogl/winsys/cogl-winsys-stub.c b/cogl/winsys/cogl-winsys-stub.c
index 565814af..81d80343 100644
--- a/cogl/winsys/cogl-winsys-stub.c
+++ b/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,
diff --git a/cogl/winsys/cogl-winsys-wgl.c b/cogl/winsys/cogl-winsys-wgl.c
index d07391de..fac0161a 100644
--- a/cogl/winsys/cogl-winsys-wgl.c
+++ b/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;
diff --git a/cogl/winsys/cogl-winsys.c b/cogl/winsys/cogl-winsys.c
index e75b906f..b5669db1 100644
--- a/cogl/winsys/cogl-winsys.c
+++ b/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;
-}