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

[Brian Paul]

Yeah, it would never make sense to pass NULL as the first argument to 
any of the _reference() functions.

Would putting an assert(ptr) at the start of pipe_surface_reference(), 
for example, silence the UBSAN warning?

-Brian


On 02/07/2017 02:45 PM, Roland Scheidegger wrote:
> 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
>>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>