[Mesa-dev] [Mesa-stable] [PATCH] mesa: do not use _glapi_new_nop_table() for DRI builds

[Brian Paul]

On 05/26/2015 12:38 PM, Ian Romanick wrote:
> From: Brian Paul <brianp at vmware.com>
>
> Commit 4bdbb588a9d38 introduced new _glapi_new_nop_table() and
> _glapi_set_nop_handler() functions in the glapi dispatcher (which
> live in libGL.so).  The calls to those functions from context.c
> would be undefined (i.e. an ABI break) if the libGL used at runtime
> was older.
>
> For the time being, use the old single generic_nop() function for
> non-Windows builds to avoid this problem.  At some point in the future
> it should be safe to remove this work-around.  See comments for more
> details.
>
> v2: Incorporate feedback from Emil.  Use _WIN32 instead of
> GLX_DIRECT_RENDERING to control behavior, move comments.
>
> v3 (idr): Encapsulate the knowledge about how to build the nop table in
> a new _mesa_new_nop_table function.  This makes it easier for
> dispatch_sanity to keep working.
>
> Cc: 10.6 <mesa-stable at lists.freedesktop.org>
> Reviewed-and-tested-by: Ian Romanick <ian.d.romanick at intel.com> [v2]
> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com> [v3]
> ---
>   src/mesa/main/api_exec.h                |  3 ++
>   src/mesa/main/context.c                 | 64 +++++++++++++++++++++++++++++++--
>   src/mesa/main/tests/dispatch_sanity.cpp |  2 +-
>   3 files changed, 65 insertions(+), 4 deletions(-)
>
> diff --git a/src/mesa/main/api_exec.h b/src/mesa/main/api_exec.h
> index 12249fe..655cb32 100644
> --- a/src/mesa/main/api_exec.h
> +++ b/src/mesa/main/api_exec.h
> @@ -38,6 +38,9 @@ _mesa_initialize_exec_table(struct gl_context *ctx);
>   extern void
>   _mesa_initialize_dispatch_tables(struct gl_context *ctx);
>
> +extern struct _glapi_table *
> +_mesa_new_nop_table(unsigned numEntries);
> +
>   #ifdef __cplusplus
>   } // extern "C"
>   #endif
> diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
> index 544cc14..e4faf3d 100644
> --- a/src/mesa/main/context.c
> +++ b/src/mesa/main/context.c
> @@ -883,6 +883,19 @@ update_default_objects(struct gl_context *ctx)
>   }
>
>
> +/* XXX this is temporary and should be removed at some point in the
> + * future when there's a reasonable expectation that the libGL library
> + * contains the _glapi_new_nop_table() and _glapi_set_nop_handler()
> + * functions which were added in Mesa 10.6.
> + */
> +#if !defined(_WIN32)
> +/* Avoid libGL / driver ABI break */
> +#define USE_GLAPI_NOP_FEATURES 0
> +#else
> +#define USE_GLAPI_NOP_FEATURES 1
> +#endif
> +
> +
>   /**
>    * This function is called by the glapi no-op functions.  For each OpenGL
>    * function/entrypoint there's a simple no-op function.  These "no-op"
> @@ -898,6 +911,7 @@ update_default_objects(struct gl_context *ctx)
>    *
>    * \param name  the name of the OpenGL function
>    */
> +#if USE_GLAPI_NOP_FEATURES
>   static void
>   nop_handler(const char *name)
>   {
> @@ -914,6 +928,7 @@ nop_handler(const char *name)
>      }
>   #endif
>   }
> +#endif
>
>
>   /**
> @@ -923,11 +938,51 @@ nop_handler(const char *name)
>   static void GLAPIENTRY
>   nop_glFlush(void)
>   {
> -   /* don't record an error like we do in _mesa_generic_nop() */
> +   /* don't record an error like we do in nop_handler() */
>   }
>   #endif
>
>
> +#if !USE_GLAPI_NOP_FEATURES
> +static int
> +generic_nop(void)
> +{
> +   GET_CURRENT_CONTEXT(ctx);
> +   _mesa_error(ctx, GL_INVALID_OPERATION,
> +               "unsupported function called "
> +               "(unsupported extension or deprecated function?)");
> +   return 0;
> +}
> +#endif
> +
> +
> +/**
> + * Create a new API dispatch table in which all entries point to the
> + * generic_nop() function.  This will not work on Windows because of
> + * the __stdcall convention which requires the callee to clean up the
> + * call stack.  That's impossible with one generic no-op function.
> + */
> +struct _glapi_table *
> +_mesa_new_nop_table(unsigned numEntries)
> +{
> +   struct _glapi_table *table;
> +
> +#if !USE_GLAPI_NOP_FEATURES
> +   table = malloc(numEntries * sizeof(_glapi_proc));
> +   if (table) {
> +      _glapi_proc *entry = (_glapi_proc *) table;
> +      unsigned i;
> +      for (i = 0; i < numEntries; i++) {
> +         entry[i] = (_glapi_proc) generic_nop;
> +      }
> +   }
> +#else
> +   table = _glapi_new_nop_table(numEntries);
> +#endif
> +   return table;
> +}
> +
> +
>   /**
>    * Allocate and initialize a new dispatch table.  The table will be
>    * populated with pointers to "no-op" functions.  In turn, the no-op
> @@ -941,8 +996,9 @@ alloc_dispatch_table(void)
>       * Mesa we do this to accommodate different versions of libGL and various
>       * DRI drivers.
>       */
> -   GLint numEntries = MAX2(_glapi_get_dispatch_table_size(), _gloffset_COUNT);
> -   struct _glapi_table *table = _glapi_new_nop_table(numEntries);
> +   int numEntries = MAX2(_glapi_get_dispatch_table_size(), _gloffset_COUNT);
> +
> +   struct _glapi_table *table = _mesa_new_nop_table(numEntries);
>
>   #if defined(_WIN32)
>      if (table) {
> @@ -966,7 +1022,9 @@ alloc_dispatch_table(void)
>      }
>   #endif
>
> +#if USE_GLAPI_NOP_FEATURES
>      _glapi_set_nop_handler(nop_handler);
> +#endif
>
>      return table;
>   }
> diff --git a/src/mesa/main/tests/dispatch_sanity.cpp b/src/mesa/main/tests/dispatch_sanity.cpp
> index d38b68d..c6f3c39 100644
> --- a/src/mesa/main/tests/dispatch_sanity.cpp
> +++ b/src/mesa/main/tests/dispatch_sanity.cpp
> @@ -96,7 +96,7 @@ DispatchSanity_test::SetUp()
>      _mesa_init_driver_functions(&driver_functions);
>
>      const unsigned size = _glapi_get_dispatch_table_size();
> -   nop_table = (_glapi_proc *) _glapi_new_nop_table(size);
> +   nop_table = (_glapi_proc *) _mesa_new_nop_table(size);
>   }
>
>   void
>

This doesn't apply to ToT, but it looks like a good solution to me.

-Brian