[PATCH] lib/oe/package: Add debug_frame support

[PATCH] lib/oe/package: Add debug_frame support

[Mathieu Othacehe]

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

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):
-- 
2.25.1

Re: [OE-core] [PATCH] lib/oe/package: Add debug_frame support

[Quentin Schulz]

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

Re: [OE-core] [PATCH] lib/oe/package: Add debug_frame support

[Mathieu Othacehe]

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

Re: [OE-core] [PATCH] lib/oe/package: Add debug_frame support

[Quentin Schulz]

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