On 03/10/2015 05:20 PM, Matt Turner wrote:
> Creating/recreating the strings in eglQueryString() is extra work and
> isn't thread-safe, as exhibited by shader-db's run.c using libepoxy.
>
> Multiple threads in run.c call eglReleaseThread() around the same time.
> libepoxy calls eglQueryString() to determine whether eglReleaseThread()
> exists, and our EGL implementation passes a pointer to the version
> string to libepoxy while simultaneously overwriting the string, leading
> to a failure in libepoxy.
>
> Moreover, the EGL spec says (emphasis mine):
>
> "eglQueryString returns a pointer to a *static*, zero-terminated string"
>
> This patch moves some auxiliary functions from eglmisc.c to eglapi.c so
> that they may be used to create the extension, API, and version strings
> once during eglInitialize(). The auxiliary functions are renamed from
> _eglUpdate* to _eglCreate*, and some checks made unnecessary by calling
> the functions from eglInitialize() are removed.
>
> It was suggested to me to simply make the generation of the version
> string behave like that of the extension and API strings -- to do a
> check like
>
> if (exts[0])
> return;
>
> before creating the string, but that is not thread-safe either.
It is safe because eglQueryString calls _eglQueryString (via
drv->API.QueryString) inside a _eglLockDisplay region.
The method you use here presumes that every implementation will use the
_eglQueryString fallback for drv->API.QueryString. Right now it does,
and I suspect that they always will. A little git archaeology leads me
to believe that this has always been the case.
Perhaps we should just remove API.QueryString altogether? Perhaps Brian
knows of a reason not to...
> ---
> src/egl/main/eglapi.c | 112 ++++++++++++++++++++++++++++++++++++++++++++
> src/egl/main/eglmisc.c | 125
> -------------------------------------------------
> 2 files changed, 112 insertions(+), 125 deletions(-)
>
> diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
> index 2258830..cb4475e 100644
> --- a/src/egl/main/eglapi.c
> +++ b/src/egl/main/eglapi.c
> @@ -100,6 +100,7 @@
> #include "eglmode.h"
> #include "eglimage.h"
> #include "eglsync.h"
> +#include "eglstring.h"
>
>
> /**
> @@ -341,6 +342,111 @@ eglGetPlatformDisplayEXT(EGLenum platform, void
> *native_display,
> }
>
> /**
> + * Copy the extension into the string and update the string pointer.
> + */
> +static EGLint
> +_eglAppendExtension(char **str, const char *ext)
> +{
> + char *s = *str;
> + size_t len = strlen(ext);
> +
> + if (s) {
> + memcpy(s, ext, len);
> + s[len++] = ' ';
> + s[len] = '\0';
> +
> + *str += len;
> + }
> + else {
> + len++;
> + }
> +
> + return (EGLint) len;
> +}
> +
> +/**
> + * Examine the individual extension enable/disable flags and recompute
> + * the driver's Extensions string.
> + */
> +static void
> +_eglCreateExtensionsString(_EGLDisplay *dpy)
> +{
> +#define _EGL_CHECK_EXTENSION(ext) \
> + do { \
> + if (dpy->Extensions.ext) { \
> + _eglAppendExtension(&exts, "EGL_" #ext); \
> + assert(exts <= dpy->ExtensionsString + _EGL_MAX_EXTENSIONS_LEN); \
> + } \
> + } while (0)
> +
> + char *exts = dpy->ExtensionsString;
> +
> + _EGL_CHECK_EXTENSION(MESA_screen_surface);
> + _EGL_CHECK_EXTENSION(MESA_copy_context);
> + _EGL_CHECK_EXTENSION(MESA_drm_display);
> + _EGL_CHECK_EXTENSION(MESA_drm_image);
> + _EGL_CHECK_EXTENSION(MESA_configless_context);
> +
> + _EGL_CHECK_EXTENSION(WL_bind_wayland_display);
> + _EGL_CHECK_EXTENSION(WL_create_wayland_buffer_from_image);
> +
> + _EGL_CHECK_EXTENSION(KHR_image_base);
> + _EGL_CHECK_EXTENSION(KHR_image_pixmap);
> + if (dpy->Extensions.KHR_image_base && dpy->Extensions.KHR_image_pixmap)
> + _eglAppendExtension(&exts, "EGL_KHR_image");
> +
> + _EGL_CHECK_EXTENSION(KHR_vg_parent_image);
> + _EGL_CHECK_EXTENSION(KHR_get_all_proc_addresses);
> + _EGL_CHECK_EXTENSION(KHR_gl_texture_2D_image);
> + _EGL_CHECK_EXTENSION(KHR_gl_texture_cubemap_image);
> + _EGL_CHECK_EXTENSION(KHR_gl_texture_3D_image);
> + _EGL_CHECK_EXTENSION(KHR_gl_renderbuffer_image);
> +
> + _EGL_CHECK_EXTENSION(KHR_reusable_sync);
> + _EGL_CHECK_EXTENSION(KHR_fence_sync);
> +
> + _EGL_CHECK_EXTENSION(KHR_surfaceless_context);
> + _EGL_CHECK_EXTENSION(KHR_create_context);
> +
> + _EGL_CHECK_EXTENSION(NOK_swap_region);
> + _EGL_CHECK_EXTENSION(NOK_texture_from_pixmap);
> +
> + _EGL_CHECK_EXTENSION(ANDROID_image_native_buffer);
> +
> + _EGL_CHECK_EXTENSION(CHROMIUM_sync_control);
> +
> + _EGL_CHECK_EXTENSION(EXT_create_context_robustness);
> + _EGL_CHECK_EXTENSION(EXT_buffer_age);
> + _EGL_CHECK_EXTENSION(EXT_swap_buffers_with_damage);
> + _EGL_CHECK_EXTENSION(EXT_image_dma_buf_import);
> +
> + _EGL_CHECK_EXTENSION(NV_post_sub_buffer);
> +#undef _EGL_CHECK_EXTENSION
> +}
> +
> +static void
> +_eglCreateAPIsString(_EGLDisplay *dpy)
> +{
> + if (dpy->ClientAPIs & EGL_OPENGL_BIT)
> + strcat(dpy->ClientAPIsString, "OpenGL ");
> +
> + if (dpy->ClientAPIs & EGL_OPENGL_ES_BIT)
> + strcat(dpy->ClientAPIsString, "OpenGL_ES ");
> +
> + if (dpy->ClientAPIs & EGL_OPENGL_ES2_BIT)
> + strcat(dpy->ClientAPIsString, "OpenGL_ES2 ");
> +
> + if (dpy->ClientAPIs & EGL_OPENGL_ES3_BIT_KHR)
> + strcat(dpy->ClientAPIsString, "OpenGL_ES3 ");
> +
> + if (dpy->ClientAPIs & EGL_OPENVG_BIT)
> + strcat(dpy->ClientAPIsString, "OpenVG ");
> +
> + assert(strlen(dpy->ClientAPIsString) < sizeof(dpy->ClientAPIsString));
> +}
> +
> +
> +/**
> * This is typically the second EGL function that an application calls.
> * Here we load/initialize the actual hardware driver.
> */
> @@ -375,6 +481,12 @@ eglInitialize(EGLDisplay dpy, EGLint *major, EGLint
> *minor)
> * EGL_KHR_get_all_proc_addresses also.
> */
> disp->Extensions.KHR_get_all_proc_addresses = EGL_TRUE;
> +
> + _eglCreateExtensionsString(disp);
> + _eglCreateAPIsString(disp);
> + _eglsnprintf(disp->VersionString, sizeof(disp->VersionString),
> + "%d.%d (%s)", disp->VersionMajor, disp->VersionMinor,
> + disp->Driver->Name);
> }
>
> /* Update applications version of major and minor if not NULL */
> diff --git a/src/egl/main/eglmisc.c b/src/egl/main/eglmisc.c
> index 2f49809..3ca3524 100644
> --- a/src/egl/main/eglmisc.c
> +++ b/src/egl/main/eglmisc.c
> @@ -33,129 +33,9 @@
> */
>
>
> -#include <assert.h>
> -#include <string.h>
> #include "eglcurrent.h"
> #include "eglmisc.h"
> #include "egldisplay.h"
> -#include "egldriver.h"
> -#include "eglstring.h"
> -
> -
> -/**
> - * Copy the extension into the string and update the string pointer.
> - */
> -static EGLint
> -_eglAppendExtension(char **str, const char *ext)
> -{
> - char *s = *str;
> - size_t len = strlen(ext);
> -
> - if (s) {
> - memcpy(s, ext, len);
> - s[len++] = ' ';
> - s[len] = '\0';
> -
> - *str += len;
> - }
> - else {
> - len++;
> - }
> -
> - return (EGLint) len;
> -}
> -
> -
> -/**
> - * Examine the individual extension enable/disable flags and recompute
> - * the driver's Extensions string.
> - */
> -static void
> -_eglUpdateExtensionsString(_EGLDisplay *dpy)
> -{
> -#define _EGL_CHECK_EXTENSION(ext) \
> - do { \
> - if (dpy->Extensions.ext) { \
> - _eglAppendExtension(&exts, "EGL_" #ext); \
> - assert(exts <= dpy->ExtensionsString + _EGL_MAX_EXTENSIONS_LEN); \
> - } \
> - } while (0)
> -
> - char *exts = dpy->ExtensionsString;
> -
> - if (exts[0])
> - return;
> -
> - _EGL_CHECK_EXTENSION(MESA_screen_surface);
> - _EGL_CHECK_EXTENSION(MESA_copy_context);
> - _EGL_CHECK_EXTENSION(MESA_drm_display);
> - _EGL_CHECK_EXTENSION(MESA_drm_image);
> - _EGL_CHECK_EXTENSION(MESA_configless_context);
> -
> - _EGL_CHECK_EXTENSION(WL_bind_wayland_display);
> - _EGL_CHECK_EXTENSION(WL_create_wayland_buffer_from_image);
> -
> - _EGL_CHECK_EXTENSION(KHR_image_base);
> - _EGL_CHECK_EXTENSION(KHR_image_pixmap);
> - if (dpy->Extensions.KHR_image_base && dpy->Extensions.KHR_image_pixmap)
> - _eglAppendExtension(&exts, "EGL_KHR_image");
> -
> - _EGL_CHECK_EXTENSION(KHR_vg_parent_image);
> - _EGL_CHECK_EXTENSION(KHR_get_all_proc_addresses);
> - _EGL_CHECK_EXTENSION(KHR_gl_texture_2D_image);
> - _EGL_CHECK_EXTENSION(KHR_gl_texture_cubemap_image);
> - _EGL_CHECK_EXTENSION(KHR_gl_texture_3D_image);
> - _EGL_CHECK_EXTENSION(KHR_gl_renderbuffer_image);
> -
> - _EGL_CHECK_EXTENSION(KHR_reusable_sync);
> - _EGL_CHECK_EXTENSION(KHR_fence_sync);
> -
> - _EGL_CHECK_EXTENSION(KHR_surfaceless_context);
> - _EGL_CHECK_EXTENSION(KHR_create_context);
> -
> - _EGL_CHECK_EXTENSION(NOK_swap_region);
> - _EGL_CHECK_EXTENSION(NOK_texture_from_pixmap);
> -
> - _EGL_CHECK_EXTENSION(ANDROID_image_native_buffer);
> -
> - _EGL_CHECK_EXTENSION(CHROMIUM_sync_control);
> -
> - _EGL_CHECK_EXTENSION(EXT_create_context_robustness);
> - _EGL_CHECK_EXTENSION(EXT_buffer_age);
> - _EGL_CHECK_EXTENSION(EXT_swap_buffers_with_damage);
> - _EGL_CHECK_EXTENSION(EXT_image_dma_buf_import);
> -
> - _EGL_CHECK_EXTENSION(NV_post_sub_buffer);
> -#undef _EGL_CHECK_EXTENSION
> -}
> -
> -
> -static void
> -_eglUpdateAPIsString(_EGLDisplay *dpy)
> -{
> - char *apis = dpy->ClientAPIsString;
> -
> - if (apis[0] || !dpy->ClientAPIs)
> - return;
> -
> - if (dpy->ClientAPIs & EGL_OPENGL_BIT)
> - strcat(apis, "OpenGL ");
> -
> - if (dpy->ClientAPIs & EGL_OPENGL_ES_BIT)
> - strcat(apis, "OpenGL_ES ");
> -
> - if (dpy->ClientAPIs & EGL_OPENGL_ES2_BIT)
> - strcat(apis, "OpenGL_ES2 ");
> -
> - if (dpy->ClientAPIs & EGL_OPENGL_ES3_BIT_KHR)
> - strcat(apis, "OpenGL_ES3 ");
> -
> - if (dpy->ClientAPIs & EGL_OPENVG_BIT)
> - strcat(apis, "OpenVG ");
> -
> - assert(strlen(apis) < sizeof(dpy->ClientAPIsString));
> -}
> -
>
> const char *
> _eglQueryString(_EGLDriver *drv, _EGLDisplay *dpy, EGLint name)
> @@ -166,15 +46,10 @@ _eglQueryString(_EGLDriver *drv, _EGLDisplay *dpy,
> EGLint name)
> case EGL_VENDOR:
> return _EGL_VENDOR_STRING;
> case EGL_VERSION:
> - _eglsnprintf(dpy->VersionString, sizeof(dpy->VersionString),
> - "%d.%d (%s)", dpy->VersionMajor, dpy->VersionMinor,
> - dpy->Driver->Name);
> return dpy->VersionString;
> case EGL_EXTENSIONS:
> - _eglUpdateExtensionsString(dpy);
> return dpy->ExtensionsString;
> case EGL_CLIENT_APIS:
> - _eglUpdateAPIsString(dpy);
> return dpy->ClientAPIsString;
> default:
> _eglError(EGL_BAD_PARAMETER, "eglQueryString");
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
