From d259a87602516f6882138e6bc031d0647cb15140 Mon Sep 17 00:00:00 2001 From: Neil Roberts Date: Wed, 27 Jul 2011 12:30:02 +0100 Subject: [PATCH] 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 --- cogl/cogl-feature-private.c | 7 +++-- cogl/cogl-feature-private.h | 2 +- cogl/cogl-glx-renderer-private.h | 3 ++ cogl/cogl-renderer-private.h | 4 +++ cogl/cogl-renderer.c | 9 ++++++ cogl/cogl.c | 7 ++--- cogl/driver/gl/cogl-gl.c | 5 +-- cogl/driver/gles/cogl-gles.c | 5 +-- cogl/winsys/cogl-winsys-egl.c | 19 ++++++++--- cogl/winsys/cogl-winsys-glx.c | 52 +++++++------------------------ cogl/winsys/cogl-winsys-private.h | 7 ++--- cogl/winsys/cogl-winsys-sdl.c | 3 +- cogl/winsys/cogl-winsys-stub.c | 20 ++++++++++-- cogl/winsys/cogl-winsys-wgl.c | 7 +++-- cogl/winsys/cogl-winsys.c | 29 ----------------- 15 files changed, 80 insertions(+), 99 deletions(-) diff --git a/cogl/cogl-feature-private.c b/cogl/cogl-feature-private.c index 053bd0268..44539da71 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 1cf3eed56..c2cec7c40 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 101623518..2d36a5f88 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 dc279bac0..f85a84375 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 be73adfe6..5a4f570e4 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 15b914892..e3eb3c315 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 3ed4b04bb..589d3da13 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 8418a7ab5..1dbe433c4 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 c07633373..14de1e375 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 12d7a54a6..a092bc7a0 100644 --- a/cogl/winsys/cogl-winsys-glx.c +++ b/cogl/winsys/cogl-winsys-glx.c @@ -56,8 +56,6 @@ #include #include -typedef CoglFuncPtr (*GLXGetProcAddressProc) (const GLubyte *procName); - #ifdef HAVE_DRM #include #include @@ -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, diff --git a/cogl/winsys/cogl-winsys-private.h b/cogl/winsys/cogl-winsys-private.h index 5822479d6..88df22d48 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 9c937e4bc..bb8908b21 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 565814afe..81d80343f 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 d07391ded..fac0161a8 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 e75b906ff..b5669db14 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; -}