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

[Laurent Pinchart]

Hi Nicolas,

On Mon, Sep 27, 2021 at 10:04:51AM -0400, Nicolas Dufresne wrote:
> Le vendredi 24 septembre 2021 à 13:23 +0300, Laurent Pinchart a écrit :
> > 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>
> > ---
> >  include/libcamera/base/backtrace.h |  3 ++
> >  src/libcamera/base/backtrace.cpp   | 61 ++++++++++++++++++++++++++++--
> >  src/libcamera/base/meson.build     |  6 +++
> >  3 files changed, 67 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 011f2e428d5d..40fa60d0f9bf 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
> > + * implementation.
> > + */
> > +#define UNW_LOCAL_ONLY
> > +#include <libunwind.h>
> > +#endif
> > +
> >  #include <sstream>
> >  
> >  #include <libcamera/base/span.h>
> > @@ -146,6 +155,15 @@ 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();
> > +}
> > +
> > +bool Backtrace::backtraceTrace()
> >  {
> >  #if HAVE_BACKTRACE
> >  	backtrace_.resize(32);
> > @@ -153,10 +171,44 @@ 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
> > +}
> > +
> > +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 +233,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;
> 
> Is this value specific to unwind ? Or perhaps you simply had it wrong in the
> original ? Perhaps you could fix the original or comment ?

It was 1 before because backtrace() was called directly in
Backtrace::Backtrace(), while now there are two helper functions
(backtraceTrace() and unwindTrace()) called by the constructor.

> >  
> >  	if (backtrace_.size() <= skipLevels)
> >  		return std::string();
> > diff --git a/src/libcamera/base/meson.build b/src/libcamera/base/meson.build
> > index 1fa894cf1896..8d14a65648fc 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