[libcamera-devel,v1,4/4] libcamera: base: backtrace: Fallback to libunwind for symbolic names
diff mbox series

Message ID 20210924102323.26787-5-laurent.pinchart@ideasonboard.com
State Accepted
Headers show
Series
  • libcamera: Improve backtraces
Related show

Commit Message

Laurent Pinchart Sept. 24, 2021, 10:23 a.m. UTC 
libunwind has an API to provide symbolic names for functions. It's less
optimal than using backtrace_symbols() or libdw, as it doesn't allow
deferring the symbolic names lookup, but it can be usefull as a fallback
if no other option is available.

A sample backtrace when falling back to libunwind looks like

libcamera::VimcCameraData::init()+0xbd
libcamera::PipelineHandlerVimc::match(libcamera::DeviceEnumerator*)+0x3e0
libcamera::CameraManager::Private::createPipelineHandlers()+0x1a7
libcamera::CameraManager::Private::init()+0x98
libcamera::CameraManager::Private::run()+0x9f
libcamera::Thread::startThread()+0xee
decltype(*(std::__1::forward<libcamera::Thread*>(fp0)).*fp()) std::__1::__invoke<void (libcamera::Thread::*)(), libcamera::Thread*, void>(void (libcamera::Thread::*&&)(), libcamera::Thread*&&)+0x77
void std::__1::__thread_execute<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (libcamera::Thread::*)(), libcamera::Thread*, 2ul>(std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (libcamera::Thread::*)(), libcamera::Thread*>&, std::__1::__tuple_indices<2ul>)+0x3e
void* std::__1::__thread_proxy<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (libcamera::Thread::*)(), libcamera::Thread*> >(void*)+0x62
start_thread+0xde
???

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/libcamera/base/backtrace.h |  1 +
 src/libcamera/base/backtrace.cpp   | 37 +++++++++++++++++++++++++++++-
 2 files changed, 37 insertions(+), 1 deletion(-)

Comments

Nicolas Dufresne Sept. 27, 2021, 2:06 p.m. UTC | #1 
Le vendredi 24 septembre 2021 à 13:23 +0300, Laurent Pinchart a écrit :
> libunwind has an API to provide symbolic names for functions. It's less
> optimal than using backtrace_symbols() or libdw, as it doesn't allow
> deferring the symbolic names lookup, but it can be usefull as a fallback
> if no other option is available.
> 
> A sample backtrace when falling back to libunwind looks like
> 
> libcamera::VimcCameraData::init()+0xbd
> libcamera::PipelineHandlerVimc::match(libcamera::DeviceEnumerator*)+0x3e0
> libcamera::CameraManager::Private::createPipelineHandlers()+0x1a7
> libcamera::CameraManager::Private::init()+0x98
> libcamera::CameraManager::Private::run()+0x9f
> libcamera::Thread::startThread()+0xee
> decltype(*(std::__1::forward<libcamera::Thread*>(fp0)).*fp()) std::__1::__invoke<void (libcamera::Thread::*)(), libcamera::Thread*, void>(void (libcamera::Thread::*&&)(), libcamera::Thread*&&)+0x77
> void std::__1::__thread_execute<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (libcamera::Thread::*)(), libcamera::Thread*, 2ul>(std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (libcamera::Thread::*)(), libcamera::Thread*>&, std::__1::__tuple_indices<2ul>)+0x3e
> void* std::__1::__thread_proxy<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (libcamera::Thread::*)(), libcamera::Thread*> >(void*)+0x62
> start_thread+0xde
> ???
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  include/libcamera/base/backtrace.h |  1 +
>  src/libcamera/base/backtrace.cpp   | 37 +++++++++++++++++++++++++++++-
>  2 files changed, 37 insertions(+), 1 deletion(-)
> 
> diff --git a/include/libcamera/base/backtrace.h b/include/libcamera/base/backtrace.h
> index 58ccc14c8f81..bb77c73b67e3 100644
> --- a/include/libcamera/base/backtrace.h
> +++ b/include/libcamera/base/backtrace.h
> @@ -30,6 +30,7 @@ private:
>  	bool unwindTrace();
>  
>  	std::vector<void *> backtrace_;
> +	std::vector<std::string> backtraceText_;
>  };
>  
>  } /* namespace libcamera */
> diff --git a/src/libcamera/base/backtrace.cpp b/src/libcamera/base/backtrace.cpp
> index 40fa60d0f9bf..bbb0cc0e17da 100644
> --- a/src/libcamera/base/backtrace.cpp
> +++ b/src/libcamera/base/backtrace.cpp
> @@ -196,6 +196,12 @@ bool Backtrace::unwindTrace()
>  		return false;
>  
>  	do {
> +#if HAVE_BACKTRACE || HAVE_DW
> +		/*
> +		 * If backtrace() or libdw is available, they will be used in
> +		 * toString() to provide symbol information for the stack
> +		 * frames using the IP register value.
> +		 */
>  		unw_word_t ip;
>  		ret = unw_get_reg(&cursor, UNW_REG_IP, &ip);
>  		if (ret) {
> @@ -204,6 +210,29 @@ bool Backtrace::unwindTrace()
>  		}
>  
>  		backtrace_.push_back(reinterpret_cast<void *>(ip));
> +#else
> +		/*
> +		 * Otherwise, use libunwind to get the symbol information. As
> +		 * the libunwind API uses cursors, we can't store the IP values
> +		 * and delay symbol lookup to toString().
> +		 */
> +		char symbol[256];
> +		unw_word_t offset = 0;
> +		ret = unw_get_proc_name(&cursor, symbol, sizeof(symbol), &offset);
> +		if (ret) {
> +			backtraceText_.emplace_back("???\n");
> +			continue;
> +		}
> +
> +		std::ostringstream entry;
> +
> +		char *name = abi::__cxa_demangle(symbol, nullptr, nullptr, nullptr);
> +		entry << (name ? name : symbol);
> +		free(name);

Perhaps a smart pointer.

> +
> +		entry << "+0x" << std::hex << offset << "\n";
> +		backtraceText_.emplace_back(entry.str());
> +#endif
>  	} while (unw_step(&cursor) > 0);
>  
>  	return true;
> @@ -239,9 +268,15 @@ std::string Backtrace::toString(unsigned int skipLevels) const
>  	 */
>  	skipLevels += 2;
>  
> -	if (backtrace_.size() <= skipLevels)
> +	if (backtrace_.size() <= skipLevels &&
> +	    backtraceText_.size() <= skipLevels)
>  		return std::string();
>  
> +	if (!backtraceText_.empty()) {
> +		Span<const std::string> trace{ backtraceText_ };
> +		return utils::join(trace.subspan(skipLevels), "");
> +	}
> +
>  #if HAVE_DW
>  	DwflParser dwfl;
>
Laurent Pinchart Sept. 28, 2021, 2:36 a.m. UTC | #2 
Hi Nicolas,

On Mon, Sep 27, 2021 at 10:06:40AM -0400, Nicolas Dufresne wrote:
> Le vendredi 24 septembre 2021 à 13:23 +0300, Laurent Pinchart a écrit :
> > libunwind has an API to provide symbolic names for functions. It's less
> > optimal than using backtrace_symbols() or libdw, as it doesn't allow
> > deferring the symbolic names lookup, but it can be usefull as a fallback
> > if no other option is available.
> > 
> > A sample backtrace when falling back to libunwind looks like
> > 
> > libcamera::VimcCameraData::init()+0xbd
> > libcamera::PipelineHandlerVimc::match(libcamera::DeviceEnumerator*)+0x3e0
> > libcamera::CameraManager::Private::createPipelineHandlers()+0x1a7
> > libcamera::CameraManager::Private::init()+0x98
> > libcamera::CameraManager::Private::run()+0x9f
> > libcamera::Thread::startThread()+0xee
> > decltype(*(std::__1::forward<libcamera::Thread*>(fp0)).*fp()) std::__1::__invoke<void (libcamera::Thread::*)(), libcamera::Thread*, void>(void (libcamera::Thread::*&&)(), libcamera::Thread*&&)+0x77
> > void std::__1::__thread_execute<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (libcamera::Thread::*)(), libcamera::Thread*, 2ul>(std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (libcamera::Thread::*)(), libcamera::Thread*>&, std::__1::__tuple_indices<2ul>)+0x3e
> > void* std::__1::__thread_proxy<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (libcamera::Thread::*)(), libcamera::Thread*> >(void*)+0x62
> > start_thread+0xde
> > ???
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  include/libcamera/base/backtrace.h |  1 +
> >  src/libcamera/base/backtrace.cpp   | 37 +++++++++++++++++++++++++++++-
> >  2 files changed, 37 insertions(+), 1 deletion(-)
> > 
> > diff --git a/include/libcamera/base/backtrace.h b/include/libcamera/base/backtrace.h
> > index 58ccc14c8f81..bb77c73b67e3 100644
> > --- a/include/libcamera/base/backtrace.h
> > +++ b/include/libcamera/base/backtrace.h
> > @@ -30,6 +30,7 @@ private:
> >  	bool unwindTrace();
> >  
> >  	std::vector<void *> backtrace_;
> > +	std::vector<std::string> backtraceText_;
> >  };
> >  
> >  } /* namespace libcamera */
> > diff --git a/src/libcamera/base/backtrace.cpp b/src/libcamera/base/backtrace.cpp
> > index 40fa60d0f9bf..bbb0cc0e17da 100644
> > --- a/src/libcamera/base/backtrace.cpp
> > +++ b/src/libcamera/base/backtrace.cpp
> > @@ -196,6 +196,12 @@ bool Backtrace::unwindTrace()
> >  		return false;
> >  
> >  	do {
> > +#if HAVE_BACKTRACE || HAVE_DW
> > +		/*
> > +		 * If backtrace() or libdw is available, they will be used in
> > +		 * toString() to provide symbol information for the stack
> > +		 * frames using the IP register value.
> > +		 */
> >  		unw_word_t ip;
> >  		ret = unw_get_reg(&cursor, UNW_REG_IP, &ip);
> >  		if (ret) {
> > @@ -204,6 +210,29 @@ bool Backtrace::unwindTrace()
> >  		}
> >  
> >  		backtrace_.push_back(reinterpret_cast<void *>(ip));
> > +#else
> > +		/*
> > +		 * Otherwise, use libunwind to get the symbol information. As
> > +		 * the libunwind API uses cursors, we can't store the IP values
> > +		 * and delay symbol lookup to toString().
> > +		 */
> > +		char symbol[256];
> > +		unw_word_t offset = 0;
> > +		ret = unw_get_proc_name(&cursor, symbol, sizeof(symbol), &offset);
> > +		if (ret) {
> > +			backtraceText_.emplace_back("???\n");
> > +			continue;
> > +		}
> > +
> > +		std::ostringstream entry;
> > +
> > +		char *name = abi::__cxa_demangle(symbol, nullptr, nullptr, nullptr);
> > +		entry << (name ? name : symbol);
> > +		free(name);
> 
> Perhaps a smart pointer.

As mentioned in the reply to 2/4, I don't mind. Let's discuss it there,
and I'll update this location accordingly.

> > +
> > +		entry << "+0x" << std::hex << offset << "\n";
> > +		backtraceText_.emplace_back(entry.str());
> > +#endif
> >  	} while (unw_step(&cursor) > 0);
> >  
> >  	return true;
> > @@ -239,9 +268,15 @@ std::string Backtrace::toString(unsigned int skipLevels) const
> >  	 */
> >  	skipLevels += 2;
> >  
> > -	if (backtrace_.size() <= skipLevels)
> > +	if (backtrace_.size() <= skipLevels &&
> > +	    backtraceText_.size() <= skipLevels)
> >  		return std::string();
> >  
> > +	if (!backtraceText_.empty()) {
> > +		Span<const std::string> trace{ backtraceText_ };
> > +		return utils::join(trace.subspan(skipLevels), "");
> > +	}
> > +
> >  #if HAVE_DW
> >  	DwflParser dwfl;
> >

Patch
diff mbox series 

diff --git a/include/libcamera/base/backtrace.h b/include/libcamera/base/backtrace.h
index 58ccc14c8f81..bb77c73b67e3 100644
--- a/include/libcamera/base/backtrace.h
+++ b/include/libcamera/base/backtrace.h
@@ -30,6 +30,7 @@  private:
 	bool unwindTrace();
 
 	std::vector<void *> backtrace_;
+	std::vector<std::string> backtraceText_;
 };
 
 } /* namespace libcamera */
diff --git a/src/libcamera/base/backtrace.cpp b/src/libcamera/base/backtrace.cpp
index 40fa60d0f9bf..bbb0cc0e17da 100644
--- a/src/libcamera/base/backtrace.cpp
+++ b/src/libcamera/base/backtrace.cpp
@@ -196,6 +196,12 @@  bool Backtrace::unwindTrace()
 		return false;
 
 	do {
+#if HAVE_BACKTRACE || HAVE_DW
+		/*
+		 * If backtrace() or libdw is available, they will be used in
+		 * toString() to provide symbol information for the stack
+		 * frames using the IP register value.
+		 */
 		unw_word_t ip;
 		ret = unw_get_reg(&cursor, UNW_REG_IP, &ip);
 		if (ret) {
@@ -204,6 +210,29 @@  bool Backtrace::unwindTrace()
 		}
 
 		backtrace_.push_back(reinterpret_cast<void *>(ip));
+#else
+		/*
+		 * Otherwise, use libunwind to get the symbol information. As
+		 * the libunwind API uses cursors, we can't store the IP values
+		 * and delay symbol lookup to toString().
+		 */
+		char symbol[256];
+		unw_word_t offset = 0;
+		ret = unw_get_proc_name(&cursor, symbol, sizeof(symbol), &offset);
+		if (ret) {
+			backtraceText_.emplace_back("???\n");
+			continue;
+		}
+
+		std::ostringstream entry;
+
+		char *name = abi::__cxa_demangle(symbol, nullptr, nullptr, nullptr);
+		entry << (name ? name : symbol);
+		free(name);
+
+		entry << "+0x" << std::hex << offset << "\n";
+		backtraceText_.emplace_back(entry.str());
+#endif
 	} while (unw_step(&cursor) > 0);
 
 	return true;
@@ -239,9 +268,15 @@  std::string Backtrace::toString(unsigned int skipLevels) const
 	 */
 	skipLevels += 2;
 
-	if (backtrace_.size() <= skipLevels)
+	if (backtrace_.size() <= skipLevels &&
+	    backtraceText_.size() <= skipLevels)
 		return std::string();
 
+	if (!backtraceText_.empty()) {
+		Span<const std::string> trace{ backtraceText_ };
+		return utils::join(trace.subspan(skipLevels), "");
+	}
+
 #if HAVE_DW
 	DwflParser dwfl;