[Mesa-dev] [PATCH 1/5] gallium/u_inlines: fix member access within null pointer

[Roland Scheidegger]

I'm not quite sure there's really a bug here?
As far as I can tell, these functions are architected specifically to
look like that - they do not actually dereference potential null
pointers, but only take the address in the end. This change seems to add
some overhead.


Roland


Am 07.02.2017 um 19:34 schrieb Bartosz Tomczyk:
> ---
>  src/gallium/auxiliary/util/u_inlines.h | 65 ++++++++++++++++++++--------------
>  1 file changed, 39 insertions(+), 26 deletions(-)
> 
> diff --git a/src/gallium/auxiliary/util/u_inlines.h b/src/gallium/auxiliary/util/u_inlines.h
> index b7b8313583..3bb3bcd6e0 100644
> --- a/src/gallium/auxiliary/util/u_inlines.h
> +++ b/src/gallium/auxiliary/util/u_inlines.h
> @@ -104,14 +104,17 @@ pipe_reference(struct pipe_reference *ptr, struct pipe_reference *reference)
>  }
>  
>  static inline void
> -pipe_surface_reference(struct pipe_surface **ptr, struct pipe_surface *surf)
> +pipe_surface_reference(struct pipe_surface **pptr, struct pipe_surface *surf)
>  {
> -   struct pipe_surface *old_surf = *ptr;
> +   struct pipe_surface *ptr = pptr ? *pptr : NULL;
> +   struct pipe_surface *old_surf = ptr;
>  
> -   if (pipe_reference_described(&(*ptr)->reference, &surf->reference, 
> +   if (pipe_reference_described(ptr ? &ptr->reference : NULL,
> +                                surf ? &surf->reference : NULL,
>                                  (debug_reference_descriptor)debug_describe_surface))
>        old_surf->context->surface_destroy(old_surf->context, old_surf);
> -   *ptr = surf;
> +
> +   if (pptr) *pptr = surf;
>  }
>  
>  /**
> @@ -121,37 +124,43 @@ pipe_surface_reference(struct pipe_surface **ptr, struct pipe_surface *surf)
>   * that's shared by multiple contexts.
>   */
>  static inline void
> -pipe_surface_release(struct pipe_context *pipe, struct pipe_surface **ptr)
> +pipe_surface_release(struct pipe_context *pipe, struct pipe_surface **pptr)
>  {
> -   if (pipe_reference_described(&(*ptr)->reference, NULL,
> +   struct pipe_surface *ptr = pptr ? *pptr : NULL;
> +   if (pipe_reference_described(ptr ? &ptr->reference : NULL, NULL,
>                                  (debug_reference_descriptor)debug_describe_surface))
> -      pipe->surface_destroy(pipe, *ptr);
> -   *ptr = NULL;
> +      pipe->surface_destroy(pipe, ptr);
> +   if (pptr) *pptr = NULL;
>  }
>  
>  
>  static inline void
> -pipe_resource_reference(struct pipe_resource **ptr, struct pipe_resource *tex)
> +pipe_resource_reference(struct pipe_resource **pptr, struct pipe_resource *tex)
>  {
> -   struct pipe_resource *old_tex = *ptr;
> +   struct pipe_resource *ptr = pptr ? *pptr : NULL;
> +   struct pipe_resource *old_tex = ptr;
>  
> -   if (pipe_reference_described(&(*ptr)->reference, &tex->reference, 
> +   if (pipe_reference_described(ptr ? &ptr->reference : NULL,
> +                                tex ? &tex->reference : NULL, 
>                                  (debug_reference_descriptor)debug_describe_resource)) {
>        pipe_resource_reference(&old_tex->next, NULL);
>        old_tex->screen->resource_destroy(old_tex->screen, old_tex);
>     }
> -   *ptr = tex;
> +
> +   if (pptr) *pptr = tex;
>  }
>  
>  static inline void
> -pipe_sampler_view_reference(struct pipe_sampler_view **ptr, struct pipe_sampler_view *view)
> +pipe_sampler_view_reference(struct pipe_sampler_view **pptr, struct pipe_sampler_view *view)
>  {
> -   struct pipe_sampler_view *old_view = *ptr;
> +   struct pipe_sampler_view *ptr = pptr ? *pptr : NULL;
> +   struct pipe_sampler_view *old_view = ptr;
>  
> -   if (pipe_reference_described(&(*ptr)->reference, &view->reference,
> +   if (pipe_reference_described(ptr ? &ptr->reference : NULL,
> +                                view ? &view->reference : NULL,
>                                  (debug_reference_descriptor)debug_describe_sampler_view))
>        old_view->context->sampler_view_destroy(old_view->context, old_view);
> -   *ptr = view;
> +   if (pptr) *pptr = view;
>  }
>  
>  /**
> @@ -162,29 +171,33 @@ pipe_sampler_view_reference(struct pipe_sampler_view **ptr, struct pipe_sampler_
>   */
>  static inline void
>  pipe_sampler_view_release(struct pipe_context *ctx,
> -                          struct pipe_sampler_view **ptr)
> +                          struct pipe_sampler_view **pptr)
>  {
> -   struct pipe_sampler_view *old_view = *ptr;
> -   if (*ptr && (*ptr)->context != ctx) {
> +   struct pipe_sampler_view *ptr = pptr ? *pptr : NULL;
> +   struct pipe_sampler_view *old_view = ptr;
> +
> +   if (ptr && ptr->context != ctx) {
>        debug_printf_once(("context mis-match in pipe_sampler_view_release()\n"));
>     }
> -   if (pipe_reference_described(&(*ptr)->reference, NULL,
> +   if (pipe_reference_described(ptr ? &ptr->reference : NULL, NULL,
>                      (debug_reference_descriptor)debug_describe_sampler_view)) {
>        ctx->sampler_view_destroy(ctx, old_view);
>     }
> -   *ptr = NULL;
> +   if(pptr) *pptr = NULL;
>  }
>  
>  static inline void
> -pipe_so_target_reference(struct pipe_stream_output_target **ptr,
> +pipe_so_target_reference(struct pipe_stream_output_target **pptr,
>                           struct pipe_stream_output_target *target)
>  {
> -   struct pipe_stream_output_target *old = *ptr;
> +   struct pipe_stream_output_target *ptr = pptr ? *pptr : NULL;
> +   struct pipe_stream_output_target *old = ptr;
>  
> -   if (pipe_reference_described(&(*ptr)->reference, &target->reference,
> -                     (debug_reference_descriptor)debug_describe_so_target))
> +   if (pipe_reference_described(ptr ? &ptr->reference : NULL,
> +                                target ? &target->reference : NULL,
> +                                (debug_reference_descriptor)debug_describe_so_target))
>        old->context->stream_output_target_destroy(old->context, old);
> -   *ptr = target;
> +   if (pptr) *pptr = target;
>  }
>  
>  static inline void
>