[Mesa-dev] [PATCH] llvmpipe: warn about illegal use of objects in different contexts

[Jose Fonseca]

On 19/01/16 23:49, sroland at vmware.com wrote:
> From: Roland Scheidegger <sroland at vmware.com>
>
> Doing that is clearly a bug. We can't quite assert as st/mesa may hit this,
> but increase at least visibility of it a bit.
> (For the non-refcounted objects it would be illegal too, but we can't detect
> that unless we'd store the context ourselves. Plus, those don't tend to cause
> random crashes at context or object destruction time... So just sampler views,
> surfaces and so targets for now.)
> ---
>   src/gallium/drivers/llvmpipe/lp_state_sampler.c |  9 +++++++++
>   src/gallium/drivers/llvmpipe/lp_state_so.c      |  9 +++++++++
>   src/gallium/drivers/llvmpipe/lp_state_surface.c | 15 ++++++++++++++-
>   3 files changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/src/gallium/drivers/llvmpipe/lp_state_sampler.c b/src/gallium/drivers/llvmpipe/lp_state_sampler.c
> index 69af38e..32bf9fd 100644
> --- a/src/gallium/drivers/llvmpipe/lp_state_sampler.c
> +++ b/src/gallium/drivers/llvmpipe/lp_state_sampler.c
> @@ -129,6 +129,15 @@ llvmpipe_set_sampler_views(struct pipe_context *pipe,
>          */
>         pipe_sampler_view_release(pipe,
>                                   &llvmpipe->sampler_views[shader][start + i]);
> +      /*
> +       * Warn if someone tries to set a view created in a different context
> +       * (which is why we need the hack above in the first place).
> +       * An assert would be better but st/mesa relies on it...
> +       */
> +      if (views[i] && views[i]->context != pipe) {
> +         debug_printf("Illegal setting of sampler_view %d created in another "
> +                      "context\n", i);
> +      }
>         pipe_sampler_view_reference(&llvmpipe->sampler_views[shader][start + i],
>                                     views[i]);
>      }
> diff --git a/src/gallium/drivers/llvmpipe/lp_state_so.c b/src/gallium/drivers/llvmpipe/lp_state_so.c
> index 2af04cd..b2afd6f 100644
> --- a/src/gallium/drivers/llvmpipe/lp_state_so.c
> +++ b/src/gallium/drivers/llvmpipe/lp_state_so.c
> @@ -70,6 +70,15 @@ llvmpipe_set_so_targets(struct pipe_context *pipe,
>      int i;
>      for (i = 0; i < num_targets; i++) {
>         const boolean append = (offsets[i] == (unsigned)-1);
> +      /*
> +       * Warn if the so target was created in another context.
> +       * XXX Not entirely sure if mesa/st may rely on this?
> +       * Otherwise should just assert.
> +       */
> +      if (targets[i] && targets[i]->context != pipe) {
> +         debug_printf("Illegal setting of so target with target %d created in "
> +                       "another context\n", i);
> +      }
>         pipe_so_target_reference((struct pipe_stream_output_target **)&llvmpipe->so_targets[i], targets[i]);
>         /* If we're not appending then lets set the internal
>            offset to what was requested */
> diff --git a/src/gallium/drivers/llvmpipe/lp_state_surface.c b/src/gallium/drivers/llvmpipe/lp_state_surface.c
> index c879ba9..b20b9c5 100644
> --- a/src/gallium/drivers/llvmpipe/lp_state_surface.c
> +++ b/src/gallium/drivers/llvmpipe/lp_state_surface.c
> @@ -52,6 +52,7 @@ llvmpipe_set_framebuffer_state(struct pipe_context *pipe,
>      struct llvmpipe_context *lp = llvmpipe_context(pipe);
>
>      boolean changed = !util_framebuffer_state_equal(&lp->framebuffer, fb);
> +   unsigned i;
>
>      assert(fb->width <= LP_MAX_WIDTH);
>      assert(fb->height <= LP_MAX_HEIGHT);
> @@ -66,10 +67,22 @@ llvmpipe_set_framebuffer_state(struct pipe_context *pipe,
>         const struct util_format_description *depth_desc =
>            util_format_description(depth_format);
>
> +      if (lp->framebuffer.zsbuf && lp->framebuffer.zsbuf->context != pipe) {
> +         debug_printf("Illegal setting of fb state with zsbuf created in "
> +                       "another context\n");
> +      }
> +      for (i = 0; i < fb->nr_cbufs; i++) {
> +         if (lp->framebuffer.cbufs[i] &&
> +             lp->framebuffer.cbufs[i]->context != pipe) {
> +            debug_printf("Illegal setting of fb state with cbuf %d created in "
> +                          "another context\n", i);
> +         }
> +      }
> +
>         util_copy_framebuffer_state(&lp->framebuffer, fb);
>
>         if (LP_PERF & PERF_NO_DEPTH) {
> -	 pipe_surface_reference(&lp->framebuffer.zsbuf, NULL);
> +         pipe_surface_reference(&lp->framebuffer.zsbuf, NULL);
>         }
>
>         /*
>

Reviewed-by: Jose Fonseca <jfonseca at vmware.com>