| Message ID | 20220712130138.25084-1-naush@raspberrypi.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Naush, On Tue, Jul 12, 2022 at 02:01:38PM +0100, Naushir Patuck via libcamera-devel wrote: > Compiling backtrace.cpp produces the follwing error with the ARM clang-11 (and > later) compiler: Let's add "on arm32 systems" > > -------------------- > ../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 > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> Great, it fixes the build indeed! Tested-by: Jacopo Mondi <jacopo@jmondi.org> Thanks j > --- > 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__ > +#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) > -- > 2.25.1 >
Hi Jacopo, Thank you for your feedback. On Tue, 12 Jul 2022 at 15:24, Jacopo Mondi <jacopo@jmondi.org> wrote: > Hi Naush, > > On Tue, Jul 12, 2022 at 02:01:38PM +0100, Naushir Patuck via > libcamera-devel wrote: > > Compiling backtrace.cpp produces the follwing error with the ARM > clang-11 (and > > later) compiler: > > Let's add "on arm32 systems" > Sure, no problem. Presumably this can be done while applying? Naush > > > > > -------------------- > > ../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 > > > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > > Great, it fixes the build indeed! > > Tested-by: Jacopo Mondi <jacopo@jmondi.org> > > Thanks > j > > > --- > > 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__ > > +#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) > > -- > > 2.25.1 > > >
Hi Naush, On Tue, Jul 12, 2022 at 03:38:53PM +0100, Naushir Patuck wrote: > Hi Jacopo, > > Thank you for your feedback. > > On Tue, 12 Jul 2022 at 15:24, Jacopo Mondi <jacopo@jmondi.org> wrote: > > > Hi Naush, > > > > On Tue, Jul 12, 2022 at 02:01:38PM +0100, Naushir Patuck via > > libcamera-devel wrote: > > > Compiling backtrace.cpp produces the follwing error with the ARM > > clang-11 (and > > > later) compiler: > > > > Let's add "on arm32 systems" > > > > Sure, no problem. Presumably this can be done while applying? > Sure it does! Let's wait for one more review and merge this fast as it fixes a compilation error. Thanks j > Naush > > > > > > > > > > > -------------------- > > > ../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 > > > > > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > > > > Great, it fixes the build indeed! > > > > Tested-by: Jacopo Mondi <jacopo@jmondi.org> > > > > Thanks > > j > > > > > --- > > > 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__ > > > +#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) > > > -- > > > 2.25.1 > > > > >
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 > > Signed-off-by: Naushir Patuck <naush@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. 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. */ 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. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > +#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) > -- > 2.25.1 >
Hi Naush, On Tue, Jul 12, 2022 at 03:38:53PM +0100, Naushir Patuck via libcamera-devel wrote: > On Tue, 12 Jul 2022 at 15:24, Jacopo Mondi wrote: > > On Tue, Jul 12, 2022 at 02:01:38PM +0100, Naushir Patuck via libcamera-devel wrote: > > > Compiling backtrace.cpp produces the follwing error with the ARM clang-11 (and > > > later) compiler: > > > > Let's add "on arm32 systems" > > Sure, no problem. Presumably this can be done while applying? Absolutely. I'll s/ARM/ARM32/ here and in the subject line, and will also fix the "follwing" typo. > > > > > > -------------------- > > > ../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 > > > And I'll add Bug: https://bugs.libcamera.org/show_bug.cgi?id=136 > > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > > > > Great, it fixes the build indeed! > > > > Tested-by: Jacopo Mondi <jacopo@jmondi.org> > > > > > --- > > > 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__ > > > +#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)
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@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@ideasonboard.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@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)
Hi Laurent, On Wed, 13 Jul 2022 at 03:01, Laurent Pinchart < laurent.pinchart@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@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@ideasonboard.com> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@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 >
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__ +#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)
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 Signed-off-by: Naushir Patuck <naush@raspberrypi.com> --- src/libcamera/base/backtrace.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+)