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

[Nicolas Dufresne]

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 ?

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