Don't use eglGetProcAddress to retrieve core functions

According to the EGL spec, eglGetProcAddress should only be used to
retrieve extension functions. It also says that returning non-NULL
does not mean the extension is available so you could interpret this
as saying that the function is allowed to return garbage for core
functions. This seems to happen at least for the Android
implementation of EGL.

To workaround this the winsys's are now passed down a flag to say
whether the function is from the core API. This information is already
in the gl-prototypes headers as the minimum core GL version and as a
pair of flags to specify whether it is available in core GLES1 and
GLES2. If the function is in core the EGL winsys will now avoid using
eglGetProcAddress and always fallback to querying the library directly
with the GModule API.

The GLX winsys is left alone because glXGetProcAddress apparently
supports querying core API and extension functions.

The WGL winsys could ideally be changed because wglGetProcAddress
should also only be used for extension functions but the situation is
slightly different because WGL considers anything from GL > 1.1 to be
an extension so it would need a bit more information to determine
whether to query the function directly from the library.

The SDL winsys is also left alone because it's not as easy to portably
determine which GL library SDL has chosen to load in order to resolve
the symbols directly.

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

(cherry picked from commit 72089730ad06ccdd38a344279a893965ae68cec1)

  Since we aren't able to break API on the 1.12 branch
  cogl_get_proc_address is still supported but isn't easily able to
  determine whether the given name corresponds to a core symbol or
  not.  For now we just assume the symbol being queried isn't part
  of the core GL api and update the documentation accordingly.

14 files changed, 70 insertions, 17 deletions

diff --git a/cogl/cogl-feature-private.c b/cogl/cogl-feature-private.c
index e6f0bd3f..7883b3cb 100644
--- a/cogl/cogl-feature-private.c
+++ b/cogl/cogl-feature-private.c
@@ -45,6 +45,7 @@ _cogl_feature_check (CoglRenderer *renderer,
 {
   const char *suffix = NULL;
   int func_num;
+  CoglBool in_core;
 
   /* First check whether the functions should be directly provided by
      GL */
@@ -55,7 +56,10 @@ _cogl_feature_check (CoglRenderer *renderer,
        (data->gles_availability & COGL_EXT_IN_GLES)) ||
       (driver == COGL_DRIVER_GLES2 &&
        (data->gles_availability & COGL_EXT_IN_GLES2)))
-    suffix = "";
+    {
+      suffix = "";
+      in_core = TRUE;
+    }
   else
     {
       /* Otherwise try all of the extensions */
@@ -107,6 +111,8 @@ _cogl_feature_check (CoglRenderer *renderer,
               break;
             }
         }
+
+      in_core = FALSE;
     }
 
   /* If we couldn't find anything that provides the functions then
@@ -122,7 +128,9 @@ _cogl_feature_check (CoglRenderer *renderer,
 
       full_function_name = g_strconcat (data->functions[func_num].name,
                                         suffix, NULL);
-      func = _cogl_renderer_get_proc_address (renderer, full_function_name);
+      func = _cogl_renderer_get_proc_address (renderer,
+                                              full_function_name,
+                                              in_core);
       g_free (full_function_name);
 
       if (func == NULL)
diff --git a/cogl/cogl-renderer-private.h b/cogl/cogl-renderer-private.h
index 808102c5..72ea9d05 100644
--- a/cogl/cogl-renderer-private.h
+++ b/cogl/cogl-renderer-private.h
@@ -92,6 +92,7 @@ _cogl_renderer_remove_native_filter (CoglRenderer *renderer,
 
 void *
 _cogl_renderer_get_proc_address (CoglRenderer *renderer,
-                                 const char *name);
+                                 const char *name,
+                                 CoglBool in_core);
 
 #endif /* __COGL_RENDERER_PRIVATE_H */
diff --git a/cogl/cogl-renderer.c b/cogl/cogl-renderer.c
index fa84625f..82e45b68 100644
--- a/cogl/cogl-renderer.c
+++ b/cogl/cogl-renderer.c
@@ -509,11 +509,12 @@ cogl_renderer_get_winsys_id (CoglRenderer *renderer)
 
 void *
 _cogl_renderer_get_proc_address (CoglRenderer *renderer,
-                                 const char *name)
+                                 const char *name,
+                                 CoglBool in_core)
 {
   const CoglWinsysVtable *winsys = _cogl_renderer_get_winsys (renderer);
 
-  return winsys->renderer_get_proc_address (renderer, name);
+  return winsys->renderer_get_proc_address (renderer, name, in_core);
 }
 
 int
diff --git a/cogl/cogl.c b/cogl/cogl.c
index a1fa5a72..6ab38c5a 100644
--- a/cogl/cogl.c
+++ b/cogl/cogl.c
@@ -94,7 +94,7 @@ cogl_get_proc_address (const char* name)
 {
   _COGL_GET_CONTEXT (ctx, NULL);
 
-  return _cogl_renderer_get_proc_address (ctx->display->renderer, name);
+  return _cogl_renderer_get_proc_address (ctx->display->renderer, name, FALSE);
 }
 
 CoglBool
diff --git a/cogl/cogl1-context.h b/cogl/cogl1-context.h
index 49744ac4..f9cf9460 100644
--- a/cogl/cogl1-context.h
+++ b/cogl/cogl1-context.h
@@ -88,6 +88,10 @@ cogl_features_available (CoglFeatureFlags features);
  * as a wrapper around glXGetProcAddress() or whatever is the
  * appropriate function for the current backend.
  *
+ * <note>This function should not be used to query core opengl API
+ * symbols since eglGetProcAddress for example doesn't allow this and
+ * and may return a junk pointer if you do.</note>
+ *
  * Return value: a pointer to the requested function or %NULL if the
  *   function is not available.
  */
diff --git a/cogl/driver/gl/cogl-gl.c b/cogl/driver/gl/cogl-gl.c
index bc1e5ec7..3976d5a6 100644
--- a/cogl/driver/gl/cogl-gl.c
+++ b/cogl/driver/gl/cogl-gl.c
@@ -321,7 +321,8 @@ _cogl_driver_update_features (CoglContext *ctx,
      can expect */
   ctx->glGetString =
     (void *) _cogl_renderer_get_proc_address (ctx->display->renderer,
-                                              "glGetString");
+                                              "glGetString",
+                                              TRUE);
 
   if (!check_gl_version (ctx, error))
     return FALSE;
diff --git a/cogl/driver/gles/cogl-gles.c b/cogl/driver/gles/cogl-gles.c
index b570feab..9e46d12d 100644
--- a/cogl/driver/gles/cogl-gles.c
+++ b/cogl/driver/gles/cogl-gles.c
@@ -171,7 +171,8 @@ _cogl_driver_update_features (CoglContext *context,
      can expect */
   context->glGetString =
     (void *) _cogl_renderer_get_proc_address (context->display->renderer,
-                                              "glGetString");
+                                              "glGetString",
+                                              TRUE);
 
   COGL_NOTE (WINSYS,
              "Checking features\n"
diff --git a/cogl/winsys/cogl-winsys-egl.c b/cogl/winsys/cogl-winsys-egl.c
index 289bc808..800ebce3 100644
--- a/cogl/winsys/cogl-winsys-egl.c
+++ b/cogl/winsys/cogl-winsys-egl.c
@@ -118,11 +118,13 @@ get_error_string (void)
 
 static CoglFuncPtr
 _cogl_winsys_renderer_get_proc_address (CoglRenderer *renderer,
-                                        const char *name)
+                                        const char *name,
+                                        CoglBool in_core)
 {
-  void *ptr;
+  void *ptr = NULL;
 
-  ptr = eglGetProcAddress (name);
+  if (!in_core)
+    ptr = eglGetProcAddress (name);
 
   /* eglGetProcAddress doesn't support fetching core API so we need to
      get that separately with GModule */
diff --git a/cogl/winsys/cogl-winsys-glx.c b/cogl/winsys/cogl-winsys-glx.c
index 66501066..c733ec40 100644
--- a/cogl/winsys/cogl-winsys-glx.c
+++ b/cogl/winsys/cogl-winsys-glx.c
@@ -129,10 +129,15 @@ static const CoglFeatureData winsys_feature_data[] =
 
 static CoglFuncPtr
 _cogl_winsys_renderer_get_proc_address (CoglRenderer *renderer,
-                                        const char *name)
+                                        const char *name,
+                                        CoglBool in_core)
 {
   CoglGLXRenderer *glx_renderer = renderer->winsys;
 
+  /* The GLX_ARB_get_proc_address extension documents that this should
+   * work for core functions too so we don't need to do anything
+   * special with in_core */
+
   return glx_renderer->glXGetProcAddress ((const GLubyte *) name);
 }
 
diff --git a/cogl/winsys/cogl-winsys-private.h b/cogl/winsys/cogl-winsys-private.h
index cd9ca2e6..a39cbd61 100644
--- a/cogl/winsys/cogl-winsys-private.h
+++ b/cogl/winsys/cogl-winsys-private.h
@@ -70,7 +70,8 @@ typedef struct _CoglWinsysVtable
 
   CoglFuncPtr
   (*renderer_get_proc_address) (CoglRenderer *renderer,
-                                const char *name);
+                                const char *name,
+                                CoglBool in_core);
 
   CoglBool
   (*renderer_connect) (CoglRenderer *renderer, GError **error);
diff --git a/cogl/winsys/cogl-winsys-sdl.c b/cogl/winsys/cogl-winsys-sdl.c
index 58c177a4..39843fb6 100644
--- a/cogl/winsys/cogl-winsys-sdl.c
+++ b/cogl/winsys/cogl-winsys-sdl.c
@@ -54,6 +54,16 @@ static CoglFuncPtr
 _cogl_winsys_renderer_get_proc_address (CoglRenderer *renderer,
                                         const char *name)
 {
+  /* XXX: It's not totally clear whether it's safe to call this for
+   * core functions. From the code it looks like the implementations
+   * will fall back to using some form of dlsym if the winsys
+   * GetProcAddress function returns NULL. Presumably this will work
+   * in most cases apart from EGL platforms that return invalid
+   * pointers for core functions. It's awkward for this code to get a
+   * handle to the GL module that SDL has chosen to load so just
+   * calling SDL_GL_GetProcAddress is probably the best we can do
+   * here. */
+
 #ifdef COGL_HAS_SDL_GLES_SUPPORT
   if (renderer->driver != COGL_DRIVER_GL)
     return SDL_GLES_GetProcAddress (name);
diff --git a/cogl/winsys/cogl-winsys-sdl2.c b/cogl/winsys/cogl-winsys-sdl2.c
index 44843ade..f9b94e97 100644
--- a/cogl/winsys/cogl-winsys-sdl2.c
+++ b/cogl/winsys/cogl-winsys-sdl2.c
@@ -61,8 +61,18 @@ typedef struct _CoglOnscreenSdl2
 
 static CoglFuncPtr
 _cogl_winsys_renderer_get_proc_address (CoglRenderer *renderer,
-                                        const char *name)
+                                        const char *name,
+                                        CoglBool in_core)
 {
+  /* XXX: It's not totally clear whether it's safe to call this for
+   * core functions. From the code it looks like the implementations
+   * will fall back to using some form of dlsym if the winsys
+   * GetProcAddress function returns NULL. Presumably this will work
+   * in most cases apart from EGL platforms that return invalid
+   * pointers for core functions. It's awkward for this code to get a
+   * handle to the GL module that SDL has chosen to load so just
+   * calling SDL_GL_GetProcAddress is probably the best we can do
+   * here. */
   return SDL_GL_GetProcAddress (name);
 }
 
diff --git a/cogl/winsys/cogl-winsys-stub.c b/cogl/winsys/cogl-winsys-stub.c
index 02ebb8ba..26486227 100644
--- a/cogl/winsys/cogl-winsys-stub.c
+++ b/cogl/winsys/cogl-winsys-stub.c
@@ -47,7 +47,8 @@ static int _cogl_winsys_stub_dummy_ptr;
 
 static CoglFuncPtr
 _cogl_winsys_renderer_get_proc_address (CoglRenderer *renderer,
-                                        const char *name)
+                                        const char *name,
+                                        CoglBool in_core)
 {
   static GModule *module = NULL;
 
diff --git a/cogl/winsys/cogl-winsys-wgl.c b/cogl/winsys/cogl-winsys-wgl.c
index 78514042..61cd335c 100644
--- a/cogl/winsys/cogl-winsys-wgl.c
+++ b/cogl/winsys/cogl-winsys-wgl.c
@@ -127,14 +127,22 @@ static const CoglFeatureData winsys_feature_data[] =
 
 static CoglFuncPtr
 _cogl_winsys_renderer_get_proc_address (CoglRenderer *renderer,
-                                        const char *name)
+                                        const char *name,
+                                        CoglBool in_core)
 {
   CoglRendererWgl *wgl_renderer = renderer->winsys;
   void *proc = wglGetProcAddress ((LPCSTR) name);
 
   /* The documentation for wglGetProcAddress implies that it only
      returns pointers to extension functions so if it fails we'll try
-     resolving the symbol directly from the the GL library */
+     resolving the symbol directly from the the GL library. We could
+     completely avoid using wglGetProcAddress if in_core is TRUE but
+     on WGL any function that is in GL > 1.1 is considered an
+     extension and is not directly exported from opengl32.dll.
+     Therefore we currently just assume wglGetProcAddress will return
+     NULL for GL 1.1 functions and we can fallback to querying them
+     directly from the library */
+
   if (proc == NULL)
     {
       if (wgl_renderer->gl_module == NULL)