[libcamera-devel] [PATCH/RFC] libcamera: bound_method: Please gcc undefined behaviour sanitizer

[Kieran Bingham]

Hi Laurent,

$SUBJECT might need improving ;-)

On 12/04/2021 23:59, Laurent Pinchart wrote:
> Enabling the gcc undefined behaviour sanitizer (with the meson configure
> -Db_sanitize=undefined option) causes many tests to fail, with errors
> such as the following (for test/object-invoke):
> 
> ../../include/libcamera/bound_method.h:198:27: runtime error: member access within address 0x55fcd7bfbd38 which does not point to an object of type 'BoundMethodBase'
> 0x55fcd7bfbd38: note: object has invalid vptr
>  fc 55 00 00  2a 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  31 00 00 00 00 00 00 00  4b c6 72 88
>               ^~~~~~~~~~~~~~~~~~~~~~~
>               invalid vptr
> ../../include/libcamera/bound_method.h:198:41: runtime error: member call on null pointer of type 'struct InvokedObject'
> ../../include/libcamera/bound_method.h:198:41: runtime error: member access within null pointer of type 'struct InvokedObject'
> Segmentation fault
> 
> The root cause isn't clear, but this change fixes the issue. It may be a
> bug in gcc.

Given that this is just a small semantic change to separate the casting
of the object, and the calling, I don't think there's any harm in
applying this as is, given that even if we fix GCC we'd have to wait for
it to filter down?

However I have no real understanding of the underlying issues either I'm
afraid, so I don't think I can help ...

Is it easy to reduce it down to a reproducible issue?

Acked-by: Kieran Bingham <kieran.bingham at ideasonboard.com>



> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  include/libcamera/bound_method.h | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> If anyone is interested in trying the gcc undefined behaviour sanitizer,
> this patch is needed. Bonus points if you can spot why it helps.
> 
> diff --git a/include/libcamera/bound_method.h b/include/libcamera/bound_method.h
> index f216e3b56826..de17fdee3612 100644
> --- a/include/libcamera/bound_method.h
> +++ b/include/libcamera/bound_method.h
> @@ -163,7 +163,8 @@ public:
>  
>  	R invoke(Args... args) override
>  	{
> -		return (static_cast<T *>(this->obj_)->*func_)(args...);
> +		T *obj = static_cast<T *>(this->obj_);
> +		return (obj->*func_)(args...);
>  	}
>  
>  private:
> @@ -195,7 +196,8 @@ public:
>  
>  	void invoke(Args... args) override
>  	{
> -		(static_cast<T *>(this->obj_)->*func_)(args...);
> +		T *obj = static_cast<T *>(this->obj_);
> +		(obj->*func_)(args...);
>  	}
>  
>  private:
> 

-- 
Regards
--
Kieran