[Mesa-dev] [PATCH] mesa: Restore functionality to dispatch sanity test

[Ian Romanick]

On 04/29/2015 12:07 PM, Ian Romanick wrote:
> From: Ian Romanick <ian.d.romanick at intel.com>
> 
> Along with a couple secondary goals, the dispatch sanity test had two
> major, primary goals.
> 
> 1. Ensure that all functions part of an API version are set in the
>    dispatch table.
> 
> 2. Ensure that functions that cannot be part of an API version are not
>    set in the dispatch table.
> 
> Commit 4bdbb58 removed the tests ability to fulfill either of its

This commit also breaks binary compatibility between old libGL and new
DRI driver.

$ EGL_LOG_LEVEL=debug es2_info 
libEGL debug: Native platform type: x11 (autodetected)
libEGL debug: EGL search path is /opt/xorg-master-x86_64/lib64/egl
libEGL debug: added egl_dri2 to module array
libEGL debug: failed to open /home/idr/devel/graphics/Mesa/BUILD/10.4-64/lib64/i965_dri.so: /home/idr/devel/graphics/Mesa/BUILD/10.4-64/lib64/i965_dri.so: undefined symbol: _glapi_set_nop_handler

That's not ok. :(

Brian: Can you propose an alternate solution to your original problem
that doesn't break compatibility?  Otherwise, I'm going to have to just
revert 4bdbb58.

> primary goals by removing anything that used _mesa_generic_nop().  It
> seems like the problem on Windows could have been resolved by adding the
> NULL context pointer check from nop_handler to _mesa_generic_nop().
> There is, however, some debugging benefit to actually getting the
> (supposed) function name logged in the "unsupported function called"
> message.
> 
> The preceding commit added a function, _glapi_new_nop_table, that
> allocates a table of per-entry point no-op functions.  Restore the
> ability to actually validate the sanity of the dispatch table by using
> _glapi_new_nop_table.
> 
> Previous to this commit removing a function from one of the
> *_functions_possible lists would not cause the test to fail.  With this
> commit removing such a function will result in failure, as is expected.
> 
> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
> Cc: Brian Paul <brianp at vmware.com>
> ---
>  src/mesa/main/tests/dispatch_sanity.cpp | 49 +++++++++++++++++++++++++++++----
>  1 file changed, 43 insertions(+), 6 deletions(-)
> 
> diff --git a/src/mesa/main/tests/dispatch_sanity.cpp b/src/mesa/main/tests/dispatch_sanity.cpp
> index 946eabb..2a5afcd 100644
> --- a/src/mesa/main/tests/dispatch_sanity.cpp
> +++ b/src/mesa/main/tests/dispatch_sanity.cpp
> @@ -82,6 +82,7 @@ public:
>     struct dd_function_table driver_functions;
>     struct gl_context share_list;
>     struct gl_context ctx;
> +   _glapi_proc *nop_table;
>  };
>  
>  void
> @@ -93,6 +94,9 @@ DispatchSanity_test::SetUp()
>     memset(&ctx, 0, sizeof(ctx));
>  
>     _mesa_init_driver_functions(&driver_functions);
> +
> +   const unsigned size = _glapi_get_dispatch_table_size();
> +   nop_table = (_glapi_proc *) _glapi_new_nop_table(size);
>  }
>  
>  void
> @@ -122,11 +126,18 @@ offset_to_proc_name_safe(unsigned offset)
>   * _glapi_proc *table exist.
>   */
>  static void
> -validate_functions(struct gl_context *ctx, const struct function *function_table)
> +validate_functions(struct gl_context *ctx, const struct function *function_table,
> +                   const _glapi_proc *nop_table)
>  {
>     _glapi_proc *table = (_glapi_proc *) ctx->Exec;
>  
>     for (unsigned i = 0; function_table[i].name != NULL; i++) {
> +      /* The context version is >= the GL version where the function was
> +       * introduced. Therefore, the function cannot be set to the nop
> +       * function.
> +       */
> +      const bool cant_be_nop = ctx->Version >= function_table[i].Version;
> +
>        const int offset = (function_table[i].offset != -1)
>           ? function_table[i].offset
>           : _glapi_get_proc_offset(function_table[i].name);
> @@ -136,32 +147,58 @@ validate_functions(struct gl_context *ctx, const struct function *function_table
>        ASSERT_EQ(offset,
>                  _glapi_get_proc_offset(function_table[i].name))
>           << "Function: " << function_table[i].name;
> +      if (cant_be_nop) {
> +         EXPECT_NE(nop_table[offset], table[offset])
> +            << "Function: " << function_table[i].name
> +            << " at offset " << offset;
> +      }
> +
> +      table[offset] = nop_table[offset];
> +   }
> +}
> +
> +/* Scan through the table and ensure that there is nothing except
> + * nop functions (as set by validate_functions().
> + */
> +static void
> +validate_nops(struct gl_context *ctx, const _glapi_proc *nop_table)
> +{
> +   _glapi_proc *table = (_glapi_proc *) ctx->Exec;
> +
> +   const unsigned size = _glapi_get_dispatch_table_size();
> +   for (unsigned i = 0; i < size; i++) {
> +      EXPECT_EQ(nop_table[i], table[i])
> +         << "i = " << i << " (" << offset_to_proc_name_safe(i) << ")";
>     }
>  }
>  
>  TEST_F(DispatchSanity_test, GL31_CORE)
>  {
>     SetUpCtx(API_OPENGL_CORE, 31);
> -   validate_functions(&ctx, gl_core_functions_possible);
> +   validate_functions(&ctx, gl_core_functions_possible, nop_table);
> +   validate_nops(&ctx, nop_table);
>  }
>  
>  TEST_F(DispatchSanity_test, GLES11)
>  {
>     SetUpCtx(API_OPENGLES, 11);
> -   validate_functions(&ctx, gles11_functions_possible);
> +   validate_functions(&ctx, gles11_functions_possible, nop_table);
> +   validate_nops(&ctx, nop_table);
>  }
>  
>  TEST_F(DispatchSanity_test, GLES2)
>  {
>     SetUpCtx(API_OPENGLES2, 20);
> -   validate_functions(&ctx, gles2_functions_possible);
> +   validate_functions(&ctx, gles2_functions_possible, nop_table);
> +   validate_nops(&ctx, nop_table);
>  }
>  
>  TEST_F(DispatchSanity_test, GLES3)
>  {
>     SetUpCtx(API_OPENGLES2, 30);
> -   validate_functions(&ctx, gles2_functions_possible);
> -   validate_functions(&ctx, gles3_functions_possible);
> +   validate_functions(&ctx, gles2_functions_possible, nop_table);
> +   validate_functions(&ctx, gles3_functions_possible, nop_table);
> +   validate_nops(&ctx, nop_table);
>  }
>  
>  const struct function gl_core_functions_possible[] = {
>