[libcamera-devel] [PATCH] libcamera: base: Remove ARM clang-11 compile error

[Laurent Pinchart]

On Tue, Jul 12, 2022 at 10:49:07PM +0100, Kieran Bingham via libcamera-devel wrote:
> Quoting Naushir Patuck via libcamera-devel (2022-07-12 14:01:38)
> > Compiling backtrace.cpp produces the follwing error with the ARM clang-11 (and
> > later) compiler:
> > 
> > --------------------
> > ../src/libcamera/base/backtrace.cpp:195:12: error: use of SP or PC in the list is deprecated [-Werror,-Winline-asm]
> >         int ret = unw_getcontext(&uc);
> >                   ^
> > /usr/include/arm-linux-gnueabihf/libunwind-common.h:114:29: note: expanded from macro 'unw_getcontext'
> >                                         ^
> > /usr/include/arm-linux-gnueabihf/libunwind-arm.h:270:5: note: expanded from macro 'unw_tdep_getcontext'
> >     "stmia %[base], {r0-r15}"                                           \
> >     ^
> > <inline asm>:1:2: note: instantiated into assembly here
> >         stmia r0, {r0-r15}
> > --------------------
> > 
> > Suppress this compilation error with a clang specific pragma around the
> > offending statements.
> > 
> > Further information about this error can be found here:
> > https://github.com/dotnet/runtime/issues/5

Naush, did you mean

https://github.com/dotnet/runtime/issues/38652

or maybe

https://github.com/dotnet/runtime/pull/38971

?

> > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> > ---
> >  src/libcamera/base/backtrace.cpp | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/src/libcamera/base/backtrace.cpp b/src/libcamera/base/backtrace.cpp
> > index 483492c390c3..b8ae2f6dc331 100644
> > --- a/src/libcamera/base/backtrace.cpp
> > +++ b/src/libcamera/base/backtrace.cpp
> > @@ -191,11 +191,21 @@ __attribute__((__noinline__))
> >  bool Backtrace::unwindTrace()
> >  {
> >  #if HAVE_UNWIND
> > +
> > +#if __clang__
> 
> Has this been tested on !__clang__ ? Shouldn't it be #ifdef __clang__?
> 
> Otherwise when __clang__ isn't defined ... I expect this will fail?
> 
> Ok - so testing this - it seems like at least with GCC it's fine. I
> guess it treats undefined as '0'. So that's ok.

#ifdef would still be better I think, so let's do that.

> I'd probably also put a single line comment above saying /why/ we're
> disabling this here.
> 
> 
> Even something simple like:
> 
>  /* clang-11 fails here on arm32 builds. */

/*
 * unw_getcontext() for ARM32 is an inline assembly function using the stmia
 * instruction to store SP and PC. This is considered by clang-11 as deprecated,
 * and generates a warning.
 */

> But as Jacopo said - this fixes a compile breakage so lets get it resolved.
> I'll see if I can add a 32 bit compile to my matrix.

I have one, but it's using gcc. If you manage to write a meson
cross-file for cross-compilation with clang, I'm interested :-)

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

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

I'll post a v2 with all the minor changes that have been discussed
during review, just to make sure I didn't make any stupid mistake.

> > +#pragma clang diagnostic push
> > +#pragma clang diagnostic ignored "-Winline-asm"
> > +#endif
> > +
> >         unw_context_t uc;
> >         int ret = unw_getcontext(&uc);
> >         if (ret)
> >                 return false;
> >  
> > +#if __clang__
> > +#pragma clang diagnostic pop
> > +#endif
> > +
> >         unw_cursor_t cursor;
> >         ret = unw_init_local(&cursor, &uc);
> >         if (ret)

-- 
Regards,

Laurent Pinchart