| Message ID | 20210924102323.26787-3-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Le vendredi 24 septembre 2021 à 13:23 +0300, Laurent Pinchart a écrit : > libdw provides access to debugging information. This allows creating > better stack trace entries, with file names and line numbers, but also > with demangled symbols as the symbol name is available and can be passed > to abi::__cxa_demangle(). > > With libdw, the backtrace previously generated by backtrace_symbols() > > src/cam/../libcamera/libcamera.so(_ZN9libcamera14VimcCameraData4initEv+0xbd) [0x7f7dbb73222d] > src/cam/../libcamera/libcamera.so(_ZN9libcamera19PipelineHandlerVimc5matchEPNS_16DeviceEnumeratorE+0x3e0) [0x7f7dbb731c40] > src/cam/../libcamera/libcamera.so(_ZN9libcamera13CameraManager7Private22createPipelineHandlersEv+0x1a7) [0x7f7dbb5ea027] > src/cam/../libcamera/libcamera.so(_ZN9libcamera13CameraManager7Private4initEv+0x98) [0x7f7dbb5e9dc8] > src/cam/../libcamera/libcamera.so(_ZN9libcamera13CameraManager7Private3runEv+0x9f) [0x7f7dbb5e9c5f] > src/cam/../libcamera/base/libcamera-base.so(_ZN9libcamera6Thread11startThreadEv+0xee) [0x7f7dbb3e95be] > src/cam/../libcamera/base/libcamera-base.so(+0x4f9d7) [0x7f7dbb3ec9d7] > src/cam/../libcamera/base/libcamera-base.so(+0x4f90e) [0x7f7dbb3ec90e] > src/cam/../libcamera/base/libcamera-base.so(+0x4f2c2) [0x7f7dbb3ec2c2] > /lib64/libpthread.so.0(+0x7e8e) [0x7f7dbab65e8e] > /lib64/libc.so.6(clone+0x3f) [0x7f7dbb10b26f] > > becomes > > libcamera::VimcCameraData::init()+0xbd (src/libcamera/libcamera.so [0x00007f9de605b22d]) > libcamera::PipelineHandlerVimc::match(libcamera::DeviceEnumerator*)+0x3e0 (src/libcamera/libcamera.so [0x00007f9de605ac40]) > libcamera::CameraManager::Private::createPipelineHandlers()+0x1a7 (src/libcamera/libcamera.so [0x00007f9de5f13027]) > libcamera::CameraManager::Private::init()+0x98 (src/libcamera/libcamera.so [0x00007f9de5f12dc8]) > libcamera::CameraManager::Private::run()+0x9f (src/libcamera/libcamera.so [0x00007f9de5f12c5f]) > libcamera::Thread::startThread()+0xee (src/libcamera/base/libcamera-base.so [0x00007f9de5d125be]) > decltype(*(std::__1::forward<libcamera::Thread*>(fp0)).*fp()) std::__1::__invoke<void (libcamera::Thread::*)(), libcamera::Thread*, void>(void (libcamera::Thread::*&&)(), libcamera::Thread*&&)+0x77 (src/libcamera/base/libcamera-base.so [0x00007f9de5d159d7]) > 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 (src/libcamera/base/libcamera-base.so [0x00007f9de5d1590e]) > 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 (src/libcamera/base/libcamera-base.so [0x00007f9de5d152c2]) > start_thread+0xde (/var/tmp/portage/sys-libs/glibc-2.33-r1/work/glibc-2.33/nptl/pthread_create.c:482) > __clone+0x3f (../sysdeps/unix/sysv/linux/x86_64/clone.S:97) > > The stack entries related to libcamera are missing source file name and > line information, which will be investigated separately, but this is > still an improvement. > > Use libdw when available, falling back to backtrace_symbols() otherwise, > or if libdw fails for any reason. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/libcamera/base/backtrace.cpp | 120 +++++++++++++++++++++++++++++++ > src/libcamera/base/meson.build | 7 ++ > 2 files changed, 127 insertions(+) > > diff --git a/src/libcamera/base/backtrace.cpp b/src/libcamera/base/backtrace.cpp > index 913f7ba71b03..011f2e428d5d 100644 > --- a/src/libcamera/base/backtrace.cpp > +++ b/src/libcamera/base/backtrace.cpp > @@ -12,9 +12,16 @@ > #include <stdlib.h> > #endif > > +#ifdef HAVE_DW > +#include <cxxabi.h> > +#include <elfutils/libdwfl.h> > +#include <unistd.h> > +#endif > + > #include <sstream> > > #include <libcamera/base/span.h> > +#include <libcamera/base/utils.h> > > /** > * \file backtrace.h > @@ -23,6 +30,101 @@ > > namespace libcamera { > > +namespace { > + > +#if HAVE_DW > +class DwflParser > +{ > +public: > + DwflParser(); > + ~DwflParser(); > + > + bool isValid() const { return valid_; } > + std::string stackEntry(const void *ip); > + > +private: > + Dwfl_Callbacks callbacks_; > + Dwfl *dwfl_; > + bool valid_; > +}; > + > +DwflParser::DwflParser() > + : callbacks_({}), dwfl_(nullptr), valid_(false) > +{ > + callbacks_.find_elf = dwfl_linux_proc_find_elf; > + callbacks_.find_debuginfo = dwfl_standard_find_debuginfo; > + > + dwfl_ = dwfl_begin(&callbacks_); > + if (!dwfl_) > + return; > + > + int ret = dwfl_linux_proc_report(dwfl_, getpid()); > + if (ret) > + return; > + > + ret = dwfl_report_end(dwfl_, nullptr, nullptr); > + if (ret) > + return; > + > + valid_ = true; > +} > + > +DwflParser::~DwflParser() > +{ > + if (dwfl_) > + dwfl_end(dwfl_); > +} > + > +std::string DwflParser::stackEntry(const void *ip) > +{ > + Dwarf_Addr addr = reinterpret_cast<Dwarf_Addr>(ip); > + > + Dwfl_Module *module = dwfl_addrmodule(dwfl_, addr); > + if (!module) > + return std::string(); > + > + std::ostringstream entry; > + > + GElf_Off offset; > + GElf_Sym sym; > + const char *symbol = dwfl_module_addrinfo(module, addr, &offset, &sym, > + nullptr, nullptr, nullptr); > + if (symbol) { > + char *name = abi::__cxa_demangle(symbol, nullptr, nullptr, nullptr); A suggestion, and yes, you have to choose between memory safety and ugly code. std::unique_ptr<char[], decltype(free)*>{ abi::__cxa_demangle(symbol, nullptr, nullptr, nullptr), free}; This is needed as by default C++ will assume that delete[] is to be used rather then free. > + entry << (name ? name : symbol) << "+0x" << std::hex << offset > + << std::dec; > + free(name); And this error prone free is not longer needed, since you have a smart pointer. > + } else { > + entry << "??? [" << utils::hex(addr) << "]"; > + } > + > + entry << " ("; > + > + Dwfl_Line *line = dwfl_module_getsrc(module, addr); > + if (line) { > + const char *filename; > + int lineNumber = 0; > + > + filename = dwfl_lineinfo(line, &addr, &lineNumber, nullptr, > + nullptr, nullptr); > + > + entry << (filename ? filename : "???") << ":" << lineNumber; > + } else { > + const char *filename = nullptr; > + > + dwfl_module_info(module, nullptr, nullptr, nullptr, nullptr, > + nullptr, &filename, nullptr); > + > + entry << (filename ? filename : "???") << " [" << utils::hex(addr) << "]"; > + } > + > + entry << ")"; > + return entry.str(); > +} > +#endif /* HAVE_DW */ > + > +} /* namespace */ > + > /** > * \class Backtrace > * \brief Representation of a call stack backtrace > @@ -85,6 +187,24 @@ std::string Backtrace::toString(unsigned int skipLevels) const > if (backtrace_.size() <= skipLevels) > return std::string(); > > +#if HAVE_DW > + DwflParser dwfl; > + > + if (dwfl.isValid()) { > + std::ostringstream msg; > + > + Span<void *const> trace{ backtrace_ }; > + for (const void *ip : trace.subspan(skipLevels)) { > + if (ip) > + msg << dwfl.stackEntry(ip) << std::endl; > + else > + msg << "???" << std::endl; > + } > + > + return msg.str(); > + } > +#endif > + > #if HAVE_BACKTRACE > Span<void *const> trace{ backtrace_ }; > trace = trace.subspan(skipLevels); > diff --git a/src/libcamera/base/meson.build b/src/libcamera/base/meson.build > index 85af01a19365..1fa894cf1896 100644 > --- a/src/libcamera/base/meson.build > +++ b/src/libcamera/base/meson.build > @@ -19,13 +19,20 @@ libcamera_base_sources = files([ > 'utils.cpp', > ]) > > +libdw = cc.find_library('libdw', required : false) > + > if cc.has_header_symbol('execinfo.h', 'backtrace') > config_h.set('HAVE_BACKTRACE', 1) > endif > > +if libdw.found() > + config_h.set('HAVE_DW', 1) > +endif > + > libcamera_base_deps = [ > dependency('threads'), > libatomic, > + libdw, > ] > > # Internal components must use the libcamera_base_private dependency to enable
Hi Nicolas, On Mon, Sep 27, 2021 at 10:01:15AM -0400, Nicolas Dufresne wrote: > Le vendredi 24 septembre 2021 à 13:23 +0300, Laurent Pinchart a écrit : > > libdw provides access to debugging information. This allows creating > > better stack trace entries, with file names and line numbers, but also > > with demangled symbols as the symbol name is available and can be passed > > to abi::__cxa_demangle(). > > > > With libdw, the backtrace previously generated by backtrace_symbols() > > > > src/cam/../libcamera/libcamera.so(_ZN9libcamera14VimcCameraData4initEv+0xbd) [0x7f7dbb73222d] > > src/cam/../libcamera/libcamera.so(_ZN9libcamera19PipelineHandlerVimc5matchEPNS_16DeviceEnumeratorE+0x3e0) [0x7f7dbb731c40] > > src/cam/../libcamera/libcamera.so(_ZN9libcamera13CameraManager7Private22createPipelineHandlersEv+0x1a7) [0x7f7dbb5ea027] > > src/cam/../libcamera/libcamera.so(_ZN9libcamera13CameraManager7Private4initEv+0x98) [0x7f7dbb5e9dc8] > > src/cam/../libcamera/libcamera.so(_ZN9libcamera13CameraManager7Private3runEv+0x9f) [0x7f7dbb5e9c5f] > > src/cam/../libcamera/base/libcamera-base.so(_ZN9libcamera6Thread11startThreadEv+0xee) [0x7f7dbb3e95be] > > src/cam/../libcamera/base/libcamera-base.so(+0x4f9d7) [0x7f7dbb3ec9d7] > > src/cam/../libcamera/base/libcamera-base.so(+0x4f90e) [0x7f7dbb3ec90e] > > src/cam/../libcamera/base/libcamera-base.so(+0x4f2c2) [0x7f7dbb3ec2c2] > > /lib64/libpthread.so.0(+0x7e8e) [0x7f7dbab65e8e] > > /lib64/libc.so.6(clone+0x3f) [0x7f7dbb10b26f] > > > > becomes > > > > libcamera::VimcCameraData::init()+0xbd (src/libcamera/libcamera.so [0x00007f9de605b22d]) > > libcamera::PipelineHandlerVimc::match(libcamera::DeviceEnumerator*)+0x3e0 (src/libcamera/libcamera.so [0x00007f9de605ac40]) > > libcamera::CameraManager::Private::createPipelineHandlers()+0x1a7 (src/libcamera/libcamera.so [0x00007f9de5f13027]) > > libcamera::CameraManager::Private::init()+0x98 (src/libcamera/libcamera.so [0x00007f9de5f12dc8]) > > libcamera::CameraManager::Private::run()+0x9f (src/libcamera/libcamera.so [0x00007f9de5f12c5f]) > > libcamera::Thread::startThread()+0xee (src/libcamera/base/libcamera-base.so [0x00007f9de5d125be]) > > decltype(*(std::__1::forward<libcamera::Thread*>(fp0)).*fp()) std::__1::__invoke<void (libcamera::Thread::*)(), libcamera::Thread*, void>(void (libcamera::Thread::*&&)(), libcamera::Thread*&&)+0x77 (src/libcamera/base/libcamera-base.so [0x00007f9de5d159d7]) > > 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 (src/libcamera/base/libcamera-base.so [0x00007f9de5d1590e]) > > 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 (src/libcamera/base/libcamera-base.so [0x00007f9de5d152c2]) > > start_thread+0xde (/var/tmp/portage/sys-libs/glibc-2.33-r1/work/glibc-2.33/nptl/pthread_create.c:482) > > __clone+0x3f (../sysdeps/unix/sysv/linux/x86_64/clone.S:97) > > > > The stack entries related to libcamera are missing source file name and > > line information, which will be investigated separately, but this is > > still an improvement. > > > > Use libdw when available, falling back to backtrace_symbols() otherwise, > > or if libdw fails for any reason. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > src/libcamera/base/backtrace.cpp | 120 +++++++++++++++++++++++++++++++ > > src/libcamera/base/meson.build | 7 ++ > > 2 files changed, 127 insertions(+) > > > > diff --git a/src/libcamera/base/backtrace.cpp b/src/libcamera/base/backtrace.cpp > > index 913f7ba71b03..011f2e428d5d 100644 > > --- a/src/libcamera/base/backtrace.cpp > > +++ b/src/libcamera/base/backtrace.cpp > > @@ -12,9 +12,16 @@ > > #include <stdlib.h> > > #endif > > > > +#ifdef HAVE_DW > > +#include <cxxabi.h> > > +#include <elfutils/libdwfl.h> > > +#include <unistd.h> > > +#endif > > + > > #include <sstream> > > > > #include <libcamera/base/span.h> > > +#include <libcamera/base/utils.h> > > > > /** > > * \file backtrace.h > > @@ -23,6 +30,101 @@ > > > > namespace libcamera { > > > > +namespace { > > + > > +#if HAVE_DW > > +class DwflParser > > +{ > > +public: > > + DwflParser(); > > + ~DwflParser(); > > + > > + bool isValid() const { return valid_; } > > + std::string stackEntry(const void *ip); > > + > > +private: > > + Dwfl_Callbacks callbacks_; > > + Dwfl *dwfl_; > > + bool valid_; > > +}; > > + > > +DwflParser::DwflParser() > > + : callbacks_({}), dwfl_(nullptr), valid_(false) > > +{ > > + callbacks_.find_elf = dwfl_linux_proc_find_elf; > > + callbacks_.find_debuginfo = dwfl_standard_find_debuginfo; > > + > > + dwfl_ = dwfl_begin(&callbacks_); > > + if (!dwfl_) > > + return; > > + > > + int ret = dwfl_linux_proc_report(dwfl_, getpid()); > > + if (ret) > > + return; > > + > > + ret = dwfl_report_end(dwfl_, nullptr, nullptr); > > + if (ret) > > + return; > > + > > + valid_ = true; > > +} > > + > > +DwflParser::~DwflParser() > > +{ > > + if (dwfl_) > > + dwfl_end(dwfl_); > > +} > > + > > +std::string DwflParser::stackEntry(const void *ip) > > +{ > > + Dwarf_Addr addr = reinterpret_cast<Dwarf_Addr>(ip); > > + > > + Dwfl_Module *module = dwfl_addrmodule(dwfl_, addr); > > + if (!module) > > + return std::string(); > > + > > + std::ostringstream entry; > > + > > + GElf_Off offset; > > + GElf_Sym sym; > > + const char *symbol = dwfl_module_addrinfo(module, addr, &offset, &sym, > > + nullptr, nullptr, nullptr); > > + if (symbol) { > > + char *name = abi::__cxa_demangle(symbol, nullptr, nullptr, nullptr); > > A suggestion, and yes, you have to choose between memory safety and ugly code. > > std::unique_ptr<char[], decltype(free)*>{ > abi::__cxa_demangle(symbol, nullptr, nullptr, nullptr), > free}; > > This is needed as by default C++ will assume that delete[] is to be used rather > then free. > > > + entry << (name ? name : symbol) << "+0x" << std::hex << offset > > + << std::dec; > > + free(name); > > And this error prone free is not longer needed, since you have a smart pointer. I've actually written something similar to the above initially, but then considered that the free was close enough to the allocation, and the code unlikely enough to change in dangerous ways, that it wasn't worth it. I wouldn't mind changing it if a unique_ptr is preferred. > > + } else { > > + entry << "??? [" << utils::hex(addr) << "]"; > > + } > > + > > + entry << " ("; > > + > > + Dwfl_Line *line = dwfl_module_getsrc(module, addr); > > + if (line) { > > + const char *filename; > > + int lineNumber = 0; > > + > > + filename = dwfl_lineinfo(line, &addr, &lineNumber, nullptr, > > + nullptr, nullptr); > > + > > + entry << (filename ? filename : "???") << ":" << lineNumber; > > + } else { > > + const char *filename = nullptr; > > + > > + dwfl_module_info(module, nullptr, nullptr, nullptr, nullptr, > > + nullptr, &filename, nullptr); > > + > > + entry << (filename ? filename : "???") << " [" << utils::hex(addr) << "]"; > > + } > > + > > + entry << ")"; > > + return entry.str(); > > +} > > +#endif /* HAVE_DW */ > > + > > +} /* namespace */ > > + > > /** > > * \class Backtrace > > * \brief Representation of a call stack backtrace > > @@ -85,6 +187,24 @@ std::string Backtrace::toString(unsigned int skipLevels) const > > if (backtrace_.size() <= skipLevels) > > return std::string(); > > > > +#if HAVE_DW > > + DwflParser dwfl; > > + > > + if (dwfl.isValid()) { > > + std::ostringstream msg; > > + > > + Span<void *const> trace{ backtrace_ }; > > + for (const void *ip : trace.subspan(skipLevels)) { > > + if (ip) > > + msg << dwfl.stackEntry(ip) << std::endl; > > + else > > + msg << "???" << std::endl; > > + } > > + > > + return msg.str(); > > + } > > +#endif > > + > > #if HAVE_BACKTRACE > > Span<void *const> trace{ backtrace_ }; > > trace = trace.subspan(skipLevels); > > diff --git a/src/libcamera/base/meson.build b/src/libcamera/base/meson.build > > index 85af01a19365..1fa894cf1896 100644 > > --- a/src/libcamera/base/meson.build > > +++ b/src/libcamera/base/meson.build > > @@ -19,13 +19,20 @@ libcamera_base_sources = files([ > > 'utils.cpp', > > ]) > > > > +libdw = cc.find_library('libdw', required : false) > > + > > if cc.has_header_symbol('execinfo.h', 'backtrace') > > config_h.set('HAVE_BACKTRACE', 1) > > endif > > > > +if libdw.found() > > + config_h.set('HAVE_DW', 1) > > +endif > > + > > libcamera_base_deps = [ > > dependency('threads'), > > libatomic, > > + libdw, > > ] > > > > # Internal components must use the libcamera_base_private dependency to enable
Le mardi 28 septembre 2021 à 05:33 +0300, Laurent Pinchart a écrit : > Hi Nicolas, > > On Mon, Sep 27, 2021 at 10:01:15AM -0400, Nicolas Dufresne wrote: > > Le vendredi 24 septembre 2021 à 13:23 +0300, Laurent Pinchart a écrit : > > > libdw provides access to debugging information. This allows creating > > > better stack trace entries, with file names and line numbers, but also > > > with demangled symbols as the symbol name is available and can be passed > > > to abi::__cxa_demangle(). > > > > > > With libdw, the backtrace previously generated by backtrace_symbols() > > > > > > src/cam/../libcamera/libcamera.so(_ZN9libcamera14VimcCameraData4initEv+0xbd) [0x7f7dbb73222d] > > > src/cam/../libcamera/libcamera.so(_ZN9libcamera19PipelineHandlerVimc5matchEPNS_16DeviceEnumeratorE+0x3e0) [0x7f7dbb731c40] > > > src/cam/../libcamera/libcamera.so(_ZN9libcamera13CameraManager7Private22createPipelineHandlersEv+0x1a7) [0x7f7dbb5ea027] > > > src/cam/../libcamera/libcamera.so(_ZN9libcamera13CameraManager7Private4initEv+0x98) [0x7f7dbb5e9dc8] > > > src/cam/../libcamera/libcamera.so(_ZN9libcamera13CameraManager7Private3runEv+0x9f) [0x7f7dbb5e9c5f] > > > src/cam/../libcamera/base/libcamera-base.so(_ZN9libcamera6Thread11startThreadEv+0xee) [0x7f7dbb3e95be] > > > src/cam/../libcamera/base/libcamera-base.so(+0x4f9d7) [0x7f7dbb3ec9d7] > > > src/cam/../libcamera/base/libcamera-base.so(+0x4f90e) [0x7f7dbb3ec90e] > > > src/cam/../libcamera/base/libcamera-base.so(+0x4f2c2) [0x7f7dbb3ec2c2] > > > /lib64/libpthread.so.0(+0x7e8e) [0x7f7dbab65e8e] > > > /lib64/libc.so.6(clone+0x3f) [0x7f7dbb10b26f] > > > > > > becomes > > > > > > libcamera::VimcCameraData::init()+0xbd (src/libcamera/libcamera.so [0x00007f9de605b22d]) > > > libcamera::PipelineHandlerVimc::match(libcamera::DeviceEnumerator*)+0x3e0 (src/libcamera/libcamera.so [0x00007f9de605ac40]) > > > libcamera::CameraManager::Private::createPipelineHandlers()+0x1a7 (src/libcamera/libcamera.so [0x00007f9de5f13027]) > > > libcamera::CameraManager::Private::init()+0x98 (src/libcamera/libcamera.so [0x00007f9de5f12dc8]) > > > libcamera::CameraManager::Private::run()+0x9f (src/libcamera/libcamera.so [0x00007f9de5f12c5f]) > > > libcamera::Thread::startThread()+0xee (src/libcamera/base/libcamera-base.so [0x00007f9de5d125be]) > > > decltype(*(std::__1::forward<libcamera::Thread*>(fp0)).*fp()) std::__1::__invoke<void (libcamera::Thread::*)(), libcamera::Thread*, void>(void (libcamera::Thread::*&&)(), libcamera::Thread*&&)+0x77 (src/libcamera/base/libcamera-base.so [0x00007f9de5d159d7]) > > > 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 (src/libcamera/base/libcamera-base.so [0x00007f9de5d1590e]) > > > 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 (src/libcamera/base/libcamera-base.so [0x00007f9de5d152c2]) > > > start_thread+0xde (/var/tmp/portage/sys-libs/glibc-2.33-r1/work/glibc-2.33/nptl/pthread_create.c:482) > > > __clone+0x3f (../sysdeps/unix/sysv/linux/x86_64/clone.S:97) > > > > > > The stack entries related to libcamera are missing source file name and > > > line information, which will be investigated separately, but this is > > > still an improvement. > > > > > > Use libdw when available, falling back to backtrace_symbols() otherwise, > > > or if libdw fails for any reason. > > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > --- > > > src/libcamera/base/backtrace.cpp | 120 +++++++++++++++++++++++++++++++ > > > src/libcamera/base/meson.build | 7 ++ > > > 2 files changed, 127 insertions(+) > > > > > > diff --git a/src/libcamera/base/backtrace.cpp b/src/libcamera/base/backtrace.cpp > > > index 913f7ba71b03..011f2e428d5d 100644 > > > --- a/src/libcamera/base/backtrace.cpp > > > +++ b/src/libcamera/base/backtrace.cpp > > > @@ -12,9 +12,16 @@ > > > #include <stdlib.h> > > > #endif > > > > > > +#ifdef HAVE_DW > > > +#include <cxxabi.h> > > > +#include <elfutils/libdwfl.h> > > > +#include <unistd.h> > > > +#endif > > > + > > > #include <sstream> > > > > > > #include <libcamera/base/span.h> > > > +#include <libcamera/base/utils.h> > > > > > > /** > > > * \file backtrace.h > > > @@ -23,6 +30,101 @@ > > > > > > namespace libcamera { > > > > > > +namespace { > > > + > > > +#if HAVE_DW > > > +class DwflParser > > > +{ > > > +public: > > > + DwflParser(); > > > + ~DwflParser(); > > > + > > > + bool isValid() const { return valid_; } > > > + std::string stackEntry(const void *ip); > > > + > > > +private: > > > + Dwfl_Callbacks callbacks_; > > > + Dwfl *dwfl_; > > > + bool valid_; > > > +}; > > > + > > > +DwflParser::DwflParser() > > > + : callbacks_({}), dwfl_(nullptr), valid_(false) > > > +{ > > > + callbacks_.find_elf = dwfl_linux_proc_find_elf; > > > + callbacks_.find_debuginfo = dwfl_standard_find_debuginfo; > > > + > > > + dwfl_ = dwfl_begin(&callbacks_); > > > + if (!dwfl_) > > > + return; > > > + > > > + int ret = dwfl_linux_proc_report(dwfl_, getpid()); > > > + if (ret) > > > + return; > > > + > > > + ret = dwfl_report_end(dwfl_, nullptr, nullptr); > > > + if (ret) > > > + return; > > > + > > > + valid_ = true; > > > +} > > > + > > > +DwflParser::~DwflParser() > > > +{ > > > + if (dwfl_) > > > + dwfl_end(dwfl_); > > > +} > > > + > > > +std::string DwflParser::stackEntry(const void *ip) > > > +{ > > > + Dwarf_Addr addr = reinterpret_cast<Dwarf_Addr>(ip); > > > + > > > + Dwfl_Module *module = dwfl_addrmodule(dwfl_, addr); > > > + if (!module) > > > + return std::string(); > > > + > > > + std::ostringstream entry; > > > + > > > + GElf_Off offset; > > > + GElf_Sym sym; > > > + const char *symbol = dwfl_module_addrinfo(module, addr, &offset, &sym, > > > + nullptr, nullptr, nullptr); > > > + if (symbol) { > > > + char *name = abi::__cxa_demangle(symbol, nullptr, nullptr, nullptr); > > > > A suggestion, and yes, you have to choose between memory safety and ugly code. > > > > std::unique_ptr<char[], decltype(free)*>{ > > abi::__cxa_demangle(symbol, nullptr, nullptr, nullptr), > > free}; > > > > This is needed as by default C++ will assume that delete[] is to be used rather > > then free. > > > > > + entry << (name ? name : symbol) << "+0x" << std::hex << offset > > > + << std::dec; > > > + free(name); > > > > And this error prone free is not longer needed, since you have a smart pointer. > > I've actually written something similar to the above initially, but then > considered that the free was close enough to the allocation, and the > code unlikely enough to change in dangerous ways, that it wasn't worth > it. I wouldn't mind changing it if a unique_ptr is preferred. I agree. I think in the long term, we'll just add a helper that makes it less ugly, and we will be more keen to use it. > > > > + } else { > > > + entry << "??? [" << utils::hex(addr) << "]"; > > > + } > > > + > > > + entry << " ("; > > > + > > > + Dwfl_Line *line = dwfl_module_getsrc(module, addr); > > > + if (line) { > > > + const char *filename; > > > + int lineNumber = 0; > > > + > > > + filename = dwfl_lineinfo(line, &addr, &lineNumber, nullptr, > > > + nullptr, nullptr); > > > + > > > + entry << (filename ? filename : "???") << ":" << lineNumber; > > > + } else { > > > + const char *filename = nullptr; > > > + > > > + dwfl_module_info(module, nullptr, nullptr, nullptr, nullptr, > > > + nullptr, &filename, nullptr); > > > + > > > + entry << (filename ? filename : "???") << " [" << utils::hex(addr) << "]"; > > > + } > > > + > > > + entry << ")"; > > > + return entry.str(); > > > +} > > > +#endif /* HAVE_DW */ > > > + > > > +} /* namespace */ > > > + > > > /** > > > * \class Backtrace > > > * \brief Representation of a call stack backtrace > > > @@ -85,6 +187,24 @@ std::string Backtrace::toString(unsigned int skipLevels) const > > > if (backtrace_.size() <= skipLevels) > > > return std::string(); > > > > > > +#if HAVE_DW > > > + DwflParser dwfl; > > > + > > > + if (dwfl.isValid()) { > > > + std::ostringstream msg; > > > + > > > + Span<void *const> trace{ backtrace_ }; > > > + for (const void *ip : trace.subspan(skipLevels)) { > > > + if (ip) > > > + msg << dwfl.stackEntry(ip) << std::endl; > > > + else > > > + msg << "???" << std::endl; > > > + } > > > + > > > + return msg.str(); > > > + } > > > +#endif > > > + > > > #if HAVE_BACKTRACE > > > Span<void *const> trace{ backtrace_ }; > > > trace = trace.subspan(skipLevels); > > > diff --git a/src/libcamera/base/meson.build b/src/libcamera/base/meson.build > > > index 85af01a19365..1fa894cf1896 100644 > > > --- a/src/libcamera/base/meson.build > > > +++ b/src/libcamera/base/meson.build > > > @@ -19,13 +19,20 @@ libcamera_base_sources = files([ > > > 'utils.cpp', > > > ]) > > > > > > +libdw = cc.find_library('libdw', required : false) > > > + > > > if cc.has_header_symbol('execinfo.h', 'backtrace') > > > config_h.set('HAVE_BACKTRACE', 1) > > > endif > > > > > > +if libdw.found() > > > + config_h.set('HAVE_DW', 1) > > > +endif > > > + > > > libcamera_base_deps = [ > > > dependency('threads'), > > > libatomic, > > > + libdw, > > > ] > > > > > > # Internal components must use the libcamera_base_private dependency to enable >
diff --git a/src/libcamera/base/backtrace.cpp b/src/libcamera/base/backtrace.cpp index 913f7ba71b03..011f2e428d5d 100644 --- a/src/libcamera/base/backtrace.cpp +++ b/src/libcamera/base/backtrace.cpp @@ -12,9 +12,16 @@ #include <stdlib.h> #endif +#ifdef HAVE_DW +#include <cxxabi.h> +#include <elfutils/libdwfl.h> +#include <unistd.h> +#endif + #include <sstream> #include <libcamera/base/span.h> +#include <libcamera/base/utils.h> /** * \file backtrace.h @@ -23,6 +30,101 @@ namespace libcamera { +namespace { + +#if HAVE_DW +class DwflParser +{ +public: + DwflParser(); + ~DwflParser(); + + bool isValid() const { return valid_; } + std::string stackEntry(const void *ip); + +private: + Dwfl_Callbacks callbacks_; + Dwfl *dwfl_; + bool valid_; +}; + +DwflParser::DwflParser() + : callbacks_({}), dwfl_(nullptr), valid_(false) +{ + callbacks_.find_elf = dwfl_linux_proc_find_elf; + callbacks_.find_debuginfo = dwfl_standard_find_debuginfo; + + dwfl_ = dwfl_begin(&callbacks_); + if (!dwfl_) + return; + + int ret = dwfl_linux_proc_report(dwfl_, getpid()); + if (ret) + return; + + ret = dwfl_report_end(dwfl_, nullptr, nullptr); + if (ret) + return; + + valid_ = true; +} + +DwflParser::~DwflParser() +{ + if (dwfl_) + dwfl_end(dwfl_); +} + +std::string DwflParser::stackEntry(const void *ip) +{ + Dwarf_Addr addr = reinterpret_cast<Dwarf_Addr>(ip); + + Dwfl_Module *module = dwfl_addrmodule(dwfl_, addr); + if (!module) + return std::string(); + + std::ostringstream entry; + + GElf_Off offset; + GElf_Sym sym; + const char *symbol = dwfl_module_addrinfo(module, addr, &offset, &sym, + nullptr, nullptr, nullptr); + if (symbol) { + char *name = abi::__cxa_demangle(symbol, nullptr, nullptr, nullptr); + entry << (name ? name : symbol) << "+0x" << std::hex << offset + << std::dec; + free(name); + } else { + entry << "??? [" << utils::hex(addr) << "]"; + } + + entry << " ("; + + Dwfl_Line *line = dwfl_module_getsrc(module, addr); + if (line) { + const char *filename; + int lineNumber = 0; + + filename = dwfl_lineinfo(line, &addr, &lineNumber, nullptr, + nullptr, nullptr); + + entry << (filename ? filename : "???") << ":" << lineNumber; + } else { + const char *filename = nullptr; + + dwfl_module_info(module, nullptr, nullptr, nullptr, nullptr, + nullptr, &filename, nullptr); + + entry << (filename ? filename : "???") << " [" << utils::hex(addr) << "]"; + } + + entry << ")"; + return entry.str(); +} +#endif /* HAVE_DW */ + +} /* namespace */ + /** * \class Backtrace * \brief Representation of a call stack backtrace @@ -85,6 +187,24 @@ std::string Backtrace::toString(unsigned int skipLevels) const if (backtrace_.size() <= skipLevels) return std::string(); +#if HAVE_DW + DwflParser dwfl; + + if (dwfl.isValid()) { + std::ostringstream msg; + + Span<void *const> trace{ backtrace_ }; + for (const void *ip : trace.subspan(skipLevels)) { + if (ip) + msg << dwfl.stackEntry(ip) << std::endl; + else + msg << "???" << std::endl; + } + + return msg.str(); + } +#endif + #if HAVE_BACKTRACE Span<void *const> trace{ backtrace_ }; trace = trace.subspan(skipLevels); diff --git a/src/libcamera/base/meson.build b/src/libcamera/base/meson.build index 85af01a19365..1fa894cf1896 100644 --- a/src/libcamera/base/meson.build +++ b/src/libcamera/base/meson.build @@ -19,13 +19,20 @@ libcamera_base_sources = files([ 'utils.cpp', ]) +libdw = cc.find_library('libdw', required : false) + if cc.has_header_symbol('execinfo.h', 'backtrace') config_h.set('HAVE_BACKTRACE', 1) endif +if libdw.found() + config_h.set('HAVE_DW', 1) +endif + libcamera_base_deps = [ dependency('threads'), libatomic, + libdw, ] # Internal components must use the libcamera_base_private dependency to enable
libdw provides access to debugging information. This allows creating better stack trace entries, with file names and line numbers, but also with demangled symbols as the symbol name is available and can be passed to abi::__cxa_demangle(). With libdw, the backtrace previously generated by backtrace_symbols() src/cam/../libcamera/libcamera.so(_ZN9libcamera14VimcCameraData4initEv+0xbd) [0x7f7dbb73222d] src/cam/../libcamera/libcamera.so(_ZN9libcamera19PipelineHandlerVimc5matchEPNS_16DeviceEnumeratorE+0x3e0) [0x7f7dbb731c40] src/cam/../libcamera/libcamera.so(_ZN9libcamera13CameraManager7Private22createPipelineHandlersEv+0x1a7) [0x7f7dbb5ea027] src/cam/../libcamera/libcamera.so(_ZN9libcamera13CameraManager7Private4initEv+0x98) [0x7f7dbb5e9dc8] src/cam/../libcamera/libcamera.so(_ZN9libcamera13CameraManager7Private3runEv+0x9f) [0x7f7dbb5e9c5f] src/cam/../libcamera/base/libcamera-base.so(_ZN9libcamera6Thread11startThreadEv+0xee) [0x7f7dbb3e95be] src/cam/../libcamera/base/libcamera-base.so(+0x4f9d7) [0x7f7dbb3ec9d7] src/cam/../libcamera/base/libcamera-base.so(+0x4f90e) [0x7f7dbb3ec90e] src/cam/../libcamera/base/libcamera-base.so(+0x4f2c2) [0x7f7dbb3ec2c2] /lib64/libpthread.so.0(+0x7e8e) [0x7f7dbab65e8e] /lib64/libc.so.6(clone+0x3f) [0x7f7dbb10b26f] becomes libcamera::VimcCameraData::init()+0xbd (src/libcamera/libcamera.so [0x00007f9de605b22d]) libcamera::PipelineHandlerVimc::match(libcamera::DeviceEnumerator*)+0x3e0 (src/libcamera/libcamera.so [0x00007f9de605ac40]) libcamera::CameraManager::Private::createPipelineHandlers()+0x1a7 (src/libcamera/libcamera.so [0x00007f9de5f13027]) libcamera::CameraManager::Private::init()+0x98 (src/libcamera/libcamera.so [0x00007f9de5f12dc8]) libcamera::CameraManager::Private::run()+0x9f (src/libcamera/libcamera.so [0x00007f9de5f12c5f]) libcamera::Thread::startThread()+0xee (src/libcamera/base/libcamera-base.so [0x00007f9de5d125be]) decltype(*(std::__1::forward<libcamera::Thread*>(fp0)).*fp()) std::__1::__invoke<void (libcamera::Thread::*)(), libcamera::Thread*, void>(void (libcamera::Thread::*&&)(), libcamera::Thread*&&)+0x77 (src/libcamera/base/libcamera-base.so [0x00007f9de5d159d7]) 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 (src/libcamera/base/libcamera-base.so [0x00007f9de5d1590e]) 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 (src/libcamera/base/libcamera-base.so [0x00007f9de5d152c2]) start_thread+0xde (/var/tmp/portage/sys-libs/glibc-2.33-r1/work/glibc-2.33/nptl/pthread_create.c:482) __clone+0x3f (../sysdeps/unix/sysv/linux/x86_64/clone.S:97) The stack entries related to libcamera are missing source file name and line information, which will be investigated separately, but this is still an improvement. Use libdw when available, falling back to backtrace_symbols() otherwise, or if libdw fails for any reason. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- src/libcamera/base/backtrace.cpp | 120 +++++++++++++++++++++++++++++++ src/libcamera/base/meson.build | 7 ++ 2 files changed, 127 insertions(+)