| Message ID | 20250108073921.15856-1-othacehe@gnu.org |
|---|---|
| State | Accepted, archived |
| Commit | 4e29551b15b4d09675205952306ba105332203a4 |
| Headers | show |
| Series | lib/oe/package: Add debug_frame support | expand |
Hi Mathieu, On 1/8/25 8:39 AM, Mathieu Othacehe via lists.openembedded.org wrote: > [You don't often get email from othacehe=gnu.org@lists.openembedded.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] > > On ARMv7, the .ARM.extab and .ARM.exidx unwinding sections are not providing > support to get full backtraces on C++ exceptions: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=117941 > > In addition to those unwinding sections, GCC is providing unwinding > instructions in DWARF format for ARMv7 binaries under the .debug_frame > section. > > By instructing 'strip' not to remove the .debug_frame section, one can use > libunwind on the .debug_frame section to get full backtraces on C++ > exceptions: > > Unexpected exception caught! > Backtrace: > 0x4b0c7d: unexpected_handler() + 0x1b > 0xb6dded55: __cxxabiv1::__terminate(void (*)()) + 0x3 > 0xb6ddedb7: std::terminate() + 0x9 > 0xb6ddf023: __cxa_rethrow + 0x2d > 0x4b0c61: uncaught_function() + 0x7 > 0x4b0c9f: main + 0x11 > 0xb6c317c7: __libc_start_call_main + 0x41 > 0xb6c31871: __libc_start_main + 0x5f > 0x4b0901: _start + 0x27 > > Add a 'PACKAGE_KEEP_DEBUG_FRAME' variable to keep the .debug_frame section > around. By using libunwind + minidebuginfo, that provides a way for ARMv7 > Yocto users to get full backtraces on C++ exceptions, on target. > Would be nice to document this variable in the yocto-docs (https://git.yoctoproject.org/yocto-docs/). Would you mind sending us a patch once this lands (or if there's a v2, maybe alongside or at the same time?)? That would be for the variable glossary at the very least. Thanks! > Signed-off-by: Mathieu Othacehe <othacehe@gnu.org> > --- > meta/classes-global/staging.bbclass | 4 +++- > meta/classes-recipe/kernel.bbclass | 2 +- > meta/lib/oe/package.py | 22 ++++++++++++---------- > 3 files changed, 16 insertions(+), 12 deletions(-) > > diff --git a/meta/classes-global/staging.bbclass b/meta/classes-global/staging.bbclass > index c2213ffa2b..d9ccf32a73 100644 > --- a/meta/classes-global/staging.bbclass > +++ b/meta/classes-global/staging.bbclass > @@ -91,10 +91,12 @@ python sysroot_strip () { > base_libdir = d.getVar("base_libdir") > qa_already_stripped = 'already-stripped' in (d.getVar('INSANE_SKIP:' + pn) or "").split() > strip_cmd = d.getVar("STRIP") > + keep_debug_frame = (d.getVar('PACKAGE_KEEP_DEBUG_FRAME') == '1') Could be bb.utils.to_boolean(d.getVar('PACKAGE_KEEP_DEBUG_FRAME'), False) to convey we're expecting a boolean here (though that would also allow y, yes, true, and n, no, false, and fail if it doesn't match any of the previous (or 1 or 0)). I suppose that's fine here. > > max_process = oe.utils.get_bb_number_threads(d) > oe.package.strip_execs(pn, dstdir, strip_cmd, libdir, base_libdir, max_process, > - qa_already_stripped=qa_already_stripped) > + qa_already_stripped=qa_already_stripped, > + keep_debug_frame=keep_debug_frame) > } > > do_populate_sysroot[dirs] = "${SYSROOT_DESTDIR}" > diff --git a/meta/classes-recipe/kernel.bbclass b/meta/classes-recipe/kernel.bbclass > index a7c4bf0ef4..b052b71be7 100644 > --- a/meta/classes-recipe/kernel.bbclass > +++ b/meta/classes-recipe/kernel.bbclass > @@ -770,7 +770,7 @@ python do_strip() { > if (extra_sections and kernel_image.find(d.getVar('KERNEL_IMAGEDEST') + '/vmlinux') != -1): > kernel_image_stripped = kernel_image + ".stripped" > shutil.copy2(kernel_image, kernel_image_stripped) > - oe.package.runstrip((kernel_image_stripped, 8, strip, extra_sections)) > + oe.package.runstrip((kernel_image_stripped, 8, strip), False, extra_sections) > bb.debug(1, "KERNEL_IMAGE_STRIP_EXTRA_SECTIONS is set, stripping sections: " + \ > extra_sections) > } > diff --git a/meta/lib/oe/package.py b/meta/lib/oe/package.py > index 1af10b7eb0..561df35313 100644 > --- a/meta/lib/oe/package.py > +++ b/meta/lib/oe/package.py > @@ -18,7 +18,7 @@ import shutil > > import oe.cachedpath > > -def runstrip(arg): > +def runstrip(arg, keep_debug_frame=False, extra_strip_sections=''): > # Function to strip a single file, called from split_and_strip_files below > # A working 'file' (one which works on the target architecture) > # > @@ -27,12 +27,7 @@ def runstrip(arg): > # 4 - executable > # 8 - shared library > # 16 - kernel module > - > - if len(arg) == 3: > - (file, elftype, strip) = arg > - extra_strip_sections = '' > - else: > - (file, elftype, strip, extra_strip_sections) = arg > + (file, elftype, strip) = arg > > newmode = None > if not os.access(file, os.W_OK) or os.access(file, os.R_OK): > @@ -44,6 +39,7 @@ def runstrip(arg): > skip_strip = False > # kernel module > if elftype & 16: > + keep_debug_frame = False > if is_kernel_module_signed(file): > bb.debug(1, "Skip strip on signed module %s" % file) > skip_strip = True > @@ -60,6 +56,9 @@ def runstrip(arg): > for section in extra_strip_sections.split(): > stripcmd.extend(["--remove-section=" + section]) > > + if keep_debug_frame: > + stripcmd.extend(["--keep-section=.debug_frame"]) > + thought: would there be a use-case for keeping other sections? Therefore, instead of having multiple PACKAGE_KEEP_xxx variables, should we have PACKAGE_KEEP_SECTION = ".debug_frame"? Cannot bring more to the discussion as that's outside of my knowledge base :) Cheers, Quentin
Hello Quentin, Thanks for having a look! > Would be nice to document this variable in the yocto-docs > (https://git.yoctoproject.org/yocto-docs/). Would you mind sending us a patch > once this lands (or if there's a v2, maybe alongside or at the same time?)? > That would be for the variable glossary at the very least. Thanks! Sure, I will send a documentation patch right after that one is merged. > thought: would there be a use-case for keeping other sections? > > Therefore, instead of having multiple PACKAGE_KEEP_xxx variables, should we > have PACKAGE_KEEP_SECTION = ".debug_frame"? I guess you are right, and being able to keep some specific sections besides .debug_frame can prove to be helpful in the future. I will rework a bit this commit in that direction. I think it would still be interesting to have a PACKAGE_KEEP_DEBUG_FRAME variable around because that is more convenient for the user than PACKAGE_KEEP_SECTION = ".debug_frame". However, we can maybe keep both, by doing something like: if bb.utils.to_boolean(d.getVar("PACKAGE_KEEP_DEBUG_FRAME")) d.appendVar("PACKAGE_KEEP_SECTION", ".debug_frame") Would that be OK? Thanks, Mathieu
Hi, On 1/22/25 9:01 PM, Mathieu Othacehe via lists.openembedded.org wrote: > [You don't often get email from othacehe=gnu.org@lists.openembedded.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] > > Hello Quentin, > > Thanks for having a look! > >> Would be nice to document this variable in the yocto-docs >> (https://git.yoctoproject.org/yocto-docs/). Would you mind sending us a patch >> once this lands (or if there's a v2, maybe alongside or at the same time?)? >> That would be for the variable glossary at the very least. Thanks! > > Sure, I will send a documentation patch right after that one is merged. > >> thought: would there be a use-case for keeping other sections? >> >> Therefore, instead of having multiple PACKAGE_KEEP_xxx variables, should we >> have PACKAGE_KEEP_SECTION = ".debug_frame"? > > I guess you are right, and being able to keep some specific sections > besides .debug_frame can prove to be helpful in the future. I will > rework a bit this commit in that direction. > > I think it would still be interesting to have a PACKAGE_KEEP_DEBUG_FRAME > variable around because that is more convenient for the user than > PACKAGE_KEEP_SECTION = ".debug_frame". > > However, we can maybe keep both, by doing something like: > > if bb.utils.to_boolean(d.getVar("PACKAGE_KEEP_DEBUG_FRAME")) > d.appendVar("PACKAGE_KEEP_SECTION", ".debug_frame") > > Would that be OK? > Not sure we need multiple ways to trigger a specific behavior. But maybe it makes sense, not sure what other reviewers or maintainers would suggest :) Cheers, Quentin
diff --git a/meta/classes-global/staging.bbclass b/meta/classes-global/staging.bbclass index c2213ffa2b..d9ccf32a73 100644 --- a/meta/classes-global/staging.bbclass +++ b/meta/classes-global/staging.bbclass @@ -91,10 +91,12 @@ python sysroot_strip () { base_libdir = d.getVar("base_libdir") qa_already_stripped = 'already-stripped' in (d.getVar('INSANE_SKIP:' + pn) or "").split() strip_cmd = d.getVar("STRIP") + keep_debug_frame = (d.getVar('PACKAGE_KEEP_DEBUG_FRAME') == '1') max_process = oe.utils.get_bb_number_threads(d) oe.package.strip_execs(pn, dstdir, strip_cmd, libdir, base_libdir, max_process, - qa_already_stripped=qa_already_stripped) + qa_already_stripped=qa_already_stripped, + keep_debug_frame=keep_debug_frame) } do_populate_sysroot[dirs] = "${SYSROOT_DESTDIR}" diff --git a/meta/classes-recipe/kernel.bbclass b/meta/classes-recipe/kernel.bbclass index a7c4bf0ef4..b052b71be7 100644 --- a/meta/classes-recipe/kernel.bbclass +++ b/meta/classes-recipe/kernel.bbclass @@ -770,7 +770,7 @@ python do_strip() { if (extra_sections and kernel_image.find(d.getVar('KERNEL_IMAGEDEST') + '/vmlinux') != -1): kernel_image_stripped = kernel_image + ".stripped" shutil.copy2(kernel_image, kernel_image_stripped) - oe.package.runstrip((kernel_image_stripped, 8, strip, extra_sections)) + oe.package.runstrip((kernel_image_stripped, 8, strip), False, extra_sections) bb.debug(1, "KERNEL_IMAGE_STRIP_EXTRA_SECTIONS is set, stripping sections: " + \ extra_sections) } diff --git a/meta/lib/oe/package.py b/meta/lib/oe/package.py index 1af10b7eb0..561df35313 100644 --- a/meta/lib/oe/package.py +++ b/meta/lib/oe/package.py @@ -18,7 +18,7 @@ import shutil import oe.cachedpath -def runstrip(arg): +def runstrip(arg, keep_debug_frame=False, extra_strip_sections=''): # Function to strip a single file, called from split_and_strip_files below # A working 'file' (one which works on the target architecture) # @@ -27,12 +27,7 @@ def runstrip(arg): # 4 - executable # 8 - shared library # 16 - kernel module - - if len(arg) == 3: - (file, elftype, strip) = arg - extra_strip_sections = '' - else: - (file, elftype, strip, extra_strip_sections) = arg + (file, elftype, strip) = arg newmode = None if not os.access(file, os.W_OK) or os.access(file, os.R_OK): @@ -44,6 +39,7 @@ def runstrip(arg): skip_strip = False # kernel module if elftype & 16: + keep_debug_frame = False if is_kernel_module_signed(file): bb.debug(1, "Skip strip on signed module %s" % file) skip_strip = True @@ -60,6 +56,9 @@ def runstrip(arg): for section in extra_strip_sections.split(): stripcmd.extend(["--remove-section=" + section]) + if keep_debug_frame: + stripcmd.extend(["--keep-section=.debug_frame"]) + stripcmd.append(file) bb.debug(1, "runstrip: %s" % stripcmd) @@ -115,7 +114,7 @@ def is_static_lib(path): return start == magic return False -def strip_execs(pn, dstdir, strip_cmd, libdir, base_libdir, max_process, qa_already_stripped=False): +def strip_execs(pn, dstdir, strip_cmd, libdir, base_libdir, max_process, qa_already_stripped=False, keep_debug_frame=False): """ Strip executable code (like executables, shared libraries) _in_place_ - Based on sysroot_strip in staging.bbclass @@ -194,7 +193,8 @@ def strip_execs(pn, dstdir, strip_cmd, libdir, base_libdir, max_process, qa_alre elf_file = int(elffiles[file]) sfiles.append((file, elf_file, strip_cmd)) - oe.utils.multiprocess_launch_mp(runstrip, sfiles, max_process) + oe.utils.multiprocess_launch_mp(runstrip, sfiles, max_process, + extraargs=(keep_debug_frame, '')) TRANSLATE = ( ("@", "@at@"), @@ -1305,7 +1305,9 @@ def process_split_and_strip_files(d): for f in staticlibs: sfiles.append((f, 16, strip)) - oe.utils.multiprocess_launch(oe.package.runstrip, sfiles, d) + keep_debug_frame = (d.getVar('PACKAGE_KEEP_DEBUG_FRAME') == '1') + oe.utils.multiprocess_launch(oe.package.runstrip, sfiles, d, + extraargs=(keep_debug_frame, '')) # Build "minidebuginfo" and reinject it back into the stripped binaries if bb.utils.contains('DISTRO_FEATURES', 'minidebuginfo', True, False, d):
On ARMv7, the .ARM.extab and .ARM.exidx unwinding sections are not providing support to get full backtraces on C++ exceptions: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=117941 In addition to those unwinding sections, GCC is providing unwinding instructions in DWARF format for ARMv7 binaries under the .debug_frame section. By instructing 'strip' not to remove the .debug_frame section, one can use libunwind on the .debug_frame section to get full backtraces on C++ exceptions: Unexpected exception caught! Backtrace: 0x4b0c7d: unexpected_handler() + 0x1b 0xb6dded55: __cxxabiv1::__terminate(void (*)()) + 0x3 0xb6ddedb7: std::terminate() + 0x9 0xb6ddf023: __cxa_rethrow + 0x2d 0x4b0c61: uncaught_function() + 0x7 0x4b0c9f: main + 0x11 0xb6c317c7: __libc_start_call_main + 0x41 0xb6c31871: __libc_start_main + 0x5f 0x4b0901: _start + 0x27 Add a 'PACKAGE_KEEP_DEBUG_FRAME' variable to keep the .debug_frame section around. By using libunwind + minidebuginfo, that provides a way for ARMv7 Yocto users to get full backtraces on C++ exceptions, on target. Signed-off-by: Mathieu Othacehe <othacehe@gnu.org> --- meta/classes-global/staging.bbclass | 4 +++- meta/classes-recipe/kernel.bbclass | 2 +- meta/lib/oe/package.py | 22 ++++++++++++---------- 3 files changed, 16 insertions(+), 12 deletions(-)