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

[Naushir Patuck]

Hi Laurent,

On Wed, 13 Jul 2022 at 03:01, Laurent Pinchart <
laurent.pinchart at ideasonboard.com> wrote:

> 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


I actually meant https://github.com/dotnet/runtime/issues/5595

Not sure what went on with my clipboard there :)


>
>
> ?
>
> > > 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.
>

Yes, it works since undefined is treated as 0.  It's annoying to add
the #if as I would have hoped gcc would ignore "pragma clang"
statements, but it does not.


> #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.
>

Sounds good!



>
> > > +#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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20220713/5a3b6bc8/attachment.htm>