| Message ID | 20211003223606.20016-5-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Quoting Laurent Pinchart (2021-10-03 23:36:06) > 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 0aafc6a366c5..d93e5518670d 100644 > --- a/src/libcamera/base/backtrace.cpp > +++ b/src/libcamera/base/backtrace.cpp > @@ -202,6 +202,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) { > @@ -210,6 +216,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; > @@ -245,9 +274,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), ""); Oh I see. Took a bit of looking to realise that this is concatenating all of the std::strings in a vector into a single string, while skipping the number of levels. Neat, but a little bit obtuse to read at first. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > + } > + > #if HAVE_DW > DwflParser dwfl; > > -- > Regards, > > Laurent Pinchart >
Hi Kieran, On Tue, Oct 12, 2021 at 11:57:11AM +0100, Kieran Bingham wrote: > Quoting Laurent Pinchart (2021-10-03 23:36:06) > > 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 0aafc6a366c5..d93e5518670d 100644 > > --- a/src/libcamera/base/backtrace.cpp > > +++ b/src/libcamera/base/backtrace.cpp > > @@ -202,6 +202,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) { > > @@ -210,6 +216,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; > > @@ -245,9 +274,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), ""); > > Oh I see. Took a bit of looking to realise that this is concatenating > all of the std::strings in a vector into a single string, while skipping > the number of levels. Neat, but a little bit obtuse to read at first. The C++20 ranges library is supposed to enable more expressive syntaxes, such as return backtraceText_ | std::views::drop(skipLevels) | std::views::join(""); (won't work as-is, it doesn't compile and I don't want to spend time figuring out what's wrong :-)). The concept is interesting, but it seems that in real life scenari it's actually difficult to use. > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > + } > > + > > #if HAVE_DW > > DwflParser dwfl; > >
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 0aafc6a366c5..d93e5518670d 100644 --- a/src/libcamera/base/backtrace.cpp +++ b/src/libcamera/base/backtrace.cpp @@ -202,6 +202,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) { @@ -210,6 +216,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; @@ -245,9 +274,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;
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(-)