[libcamera-devel] [PATCH v2 3/4] libcamera: base: backtrace: Use libunwind when available

[Kieran Bingham]

Quoting Laurent Pinchart (2021-10-03 23:36:05)
> libunwind is an alternative to glibc's backtrace() to extract a
> backtrace. Use it when available to extend backtrace support to more
> platforms.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> Changes since v1:
> 
> - Use __noinline__ attribute for backtraceTrace() and unwindTrace()

Is this guaranteed ? Or do we need to do some checks at runtime?

Perhaps we can deal with that if we find out we need it.

> ---
>  include/libcamera/base/backtrace.h |  3 ++
>  src/libcamera/base/backtrace.cpp   | 67 ++++++++++++++++++++++++++++--
>  src/libcamera/base/meson.build     |  6 +++
>  3 files changed, 73 insertions(+), 3 deletions(-)
> 
> diff --git a/include/libcamera/base/backtrace.h b/include/libcamera/base/backtrace.h
> index aefc76defa83..58ccc14c8f81 100644
> --- a/include/libcamera/base/backtrace.h
> +++ b/include/libcamera/base/backtrace.h
> @@ -26,6 +26,9 @@ public:
>  private:
>         LIBCAMERA_DISABLE_COPY(Backtrace)
>  
> +       bool backtraceTrace();
> +       bool unwindTrace();
> +
>         std::vector<void *> backtrace_;
>  };
>  
> diff --git a/src/libcamera/base/backtrace.cpp b/src/libcamera/base/backtrace.cpp
> index 79e4a31f3d21..0aafc6a366c5 100644
> --- a/src/libcamera/base/backtrace.cpp
> +++ b/src/libcamera/base/backtrace.cpp
> @@ -18,6 +18,15 @@
>  #include <unistd.h>
>  #endif
>  
> +#if HAVE_UNWIND
> +/*
> + * Disable support for remote unwinding to enable a more optimized

What is 'remote' unwinding? Not that it matters too much here.

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

> + * implementation.
> + */
> +#define UNW_LOCAL_ONLY
> +#include <libunwind.h>
> +#endif
> +
>  #include <sstream>
>  
>  #include <libcamera/base/span.h>
> @@ -146,6 +155,20 @@ std::string DwflParser::stackEntry(const void *ip)
>   * It can later be converted to a string with toString().
>   */
>  Backtrace::Backtrace()
> +{
> +       /* Try libunwind first and fall back to backtrace() if it fails. */
> +       if (unwindTrace())
> +               return;
> +
> +       backtraceTrace();
> +}
> +
> +/*
> + * Avoid inlining to make sure that the Backtrace constructor adds exactly two
> + * calls to the stack, which are later skipped in toString().
> + */
> +__attribute__((__noinline__))
> +bool Backtrace::backtraceTrace()
>  {
>  #if HAVE_BACKTRACE
>         backtrace_.resize(32);
> @@ -153,10 +176,45 @@ Backtrace::Backtrace()
>         int num_entries = backtrace(backtrace_.data(), backtrace_.size());
>         if (num_entries < 0) {
>                 backtrace_.clear();
> -               return;
> +               return false;
>         }
>  
>         backtrace_.resize(num_entries);
> +
> +       return true;
> +#else
> +       return false;
> +#endif
> +}
> +
> +__attribute__((__noinline__))
> +bool Backtrace::unwindTrace()
> +{
> +#if HAVE_UNWIND
> +       unw_context_t uc;
> +       int ret = unw_getcontext(&uc);
> +       if (ret)
> +               return false;
> +
> +       unw_cursor_t cursor;
> +       ret = unw_init_local(&cursor, &uc);
> +       if (ret)
> +               return false;
> +
> +       do {
> +               unw_word_t ip;
> +               ret = unw_get_reg(&cursor, UNW_REG_IP, &ip);
> +               if (ret) {
> +                       backtrace_.push_back(nullptr);
> +                       continue;
> +               }
> +
> +               backtrace_.push_back(reinterpret_cast<void *>(ip));
> +       } while (unw_step(&cursor) > 0);
> +
> +       return true;
> +#else
> +       return false;
>  #endif
>  }
>  
> @@ -181,8 +239,11 @@ Backtrace::Backtrace()
>   */
>  std::string Backtrace::toString(unsigned int skipLevels) const
>  {
> -       /* Skip the first entry, corresponding to the Backtrace construction. */
> -       skipLevels += 1;
> +       /*
> +        * Skip the first two entries, corresponding to the Backtrace
> +        * construction.
> +        */
> +       skipLevels += 2;
>  
>         if (backtrace_.size() <= skipLevels)
>                 return std::string();
> diff --git a/src/libcamera/base/meson.build b/src/libcamera/base/meson.build
> index 4c44b9f55011..05fed7acf561 100644
> --- a/src/libcamera/base/meson.build
> +++ b/src/libcamera/base/meson.build
> @@ -20,6 +20,7 @@ libcamera_base_sources = files([
>  ])
>  
>  libdw = cc.find_library('libdw', required : false)
> +libunwind = cc.find_library('libunwind', required : false)
>  
>  if cc.has_header_symbol('execinfo.h', 'backtrace')
>      config_h.set('HAVE_BACKTRACE', 1)
> @@ -29,10 +30,15 @@ if libdw.found()
>      config_h.set('HAVE_DW', 1)
>  endif
>  
> +if libunwind.found()
> +    config_h.set('HAVE_UNWIND', 1)
> +endif
> +
>  libcamera_base_deps = [
>      dependency('threads'),
>      libatomic,
>      libdw,
> +    libunwind,
>  ]
>  
>  # Internal components must use the libcamera_base_private dependency to enable
> -- 
> Regards,
> 
> Laurent Pinchart
>