* [musl] option to enable eh_frame @ 2021-07-16 9:16 Timo Teras 2021-07-16 15:57 ` Rich Felker 0 siblings, 1 reply; 21+ messages in thread From: Timo Teras @ 2021-07-16 9:16 UTC (permalink / raw) To: musl [-- Attachment #1: Type: text/plain, Size: 1565 bytes --] Hi, This has been discussed few times, and I know there are arguments also not to do this. But at this time we at Alpine think the reasons to keep eh_frame outweight the reasons to not include it. Main arguments against .eh_frame being: 1) Having .eh_frame makes it seem like C++ exception throwing works through C-library functions (e.g. throwing exception form qsort callback to return over qsort back to application). Counter arguments: - C++ exceptions is just one way to jump through musl functions. E.g. setjmp/longjmp can do that just fine even without .eh_frame 2) Having application unwind itself for backtrace printing purposes especially in signal handler is bad. This is agreed, but there's still other cases when unwinding is good for debugging, and other reasons. The fix for this root cause is to remove the unwinding from signal handlers. Arguments to have .eh_frame: - It allows debugging things even if musl-dbg is not or cannot be installed for some reason (e.g. gdb, valgrind), or is no longer available - libunwind/libexecinfo will start to work and give meaningful backtraces - Continuous kernel based profiling (e.g. perf record -g dwarf) will work Given that the main arguments against are either making UB crash, or not the best fix, and keeping eh_frame enables useful features to work, I think it would make sense to allow enabling it. Please consider the the attached patch to make it a configure option to enable keeping eh_frame (defaulting still to not keeping it). Thanks, Timo [-- Attachment #2: eh-frame.patch --] [-- Type: text/x-patch, Size: 5164 bytes --] From b2d24e3cc6015aa6a4b01b1fdbad1e68b6fccb96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20Ter=C3=A4s?= <timo.teras@iki.fi> Date: Fri, 9 Jul 2021 10:57:20 +0300 Subject: add configure option to enable .eh_frame generation Add --enable-eh-frame to enable .eh_frame generation. This adds about 80kB to ELF size on x86_64. This is useful to run continuous profilers, gdb, valgrind and other debugging utilities to generate backtrace information without having to install the full musl-dbg package. As side effect, this might seem to make exception handling work through C-library fuctions when they are calling a callback (e.g. qsort), but this continues to be UB and is not supported. This actually is the case on ARM where .ARM.exidx is used for unwind info which is present always on the musl DSO. --- Makefile | 4 ++-- configure | 30 ++++++++++++++++++++++-------- tools/add-cfi.i386.awk | 2 +- tools/add-cfi.x86_64.awk | 2 +- 4 files changed, 26 insertions(+), 12 deletions(-) diff --git a/Makefile b/Makefile index e8cc4436..2b501c25 100644 --- a/Makefile +++ b/Makefile @@ -134,8 +134,8 @@ $(LOBJS) $(LDSO_OBJS): CFLAGS_ALL += -fPIC CC_CMD = $(CC) $(CFLAGS_ALL) -c -o $@ $< # Choose invocation of assembler to be used -ifeq ($(ADD_CFI),yes) - AS_CMD = LC_ALL=C awk -f $(srcdir)/tools/add-cfi.common.awk -f $(srcdir)/tools/add-cfi.$(ARCH).awk $< | $(CC) $(CFLAGS_ALL) -x assembler -c -o $@ - +ifneq ($(ADD_CFI),no) + AS_CMD = LC_ALL=C awk -v CFI_SECTIONS="$(ADD_CFI)" -f $(srcdir)/tools/add-cfi.common.awk -f $(srcdir)/tools/add-cfi.$(ARCH).awk $< | $(CC) $(CFLAGS_ALL) -x assembler -c -o $@ - else AS_CMD = $(CC_CMD) endif diff --git a/configure b/configure index a5231a0e..eea16e6c 100755 --- a/configure +++ b/configure @@ -30,6 +30,7 @@ System types: Optional features: --enable-optimize=... optimize listed components for speed over size [auto] --enable-debug build with debugging information [disabled] + --enable-eh-frame keep .eh_frame on main binary [disabled] --disable-warnings build with recommended warnings flags [enabled] --enable-wrapper=... build given musl toolchain wrapper [auto] --disable-shared inhibit building shared library [enabled] @@ -142,6 +143,7 @@ static=yes wrapper=auto gcc_wrapper=no clang_wrapper=no +eh_frame=no malloc_dir=mallocng for arg ; do @@ -172,6 +174,8 @@ case "$arg" in --disable-wrapper|--enable-wrapper=no) wrapper=no ;; --enable-gcc-wrapper|--enable-gcc-wrapper=yes) wrapper=yes ; gcc_wrapper=yes ;; --disable-gcc-wrapper|--enable-gcc-wrapper=no) wrapper=no ;; +--enable-eh-frame|--enable-eh-frame=yes) eh_frame=yes ;; +--disable-eh-frame|--enable-eh-frame=no) eh_frame=no ;; --with-malloc=*) malloc_dir=${arg#*=} ;; --enable-*|--disable-*|--with-*|--without-*|--*dir=*) ;; --host=*|--target=*) target=${arg#*=} ;; @@ -407,14 +411,22 @@ test "$debug" = yes && CFLAGS_AUTO=-g # enabled, our assembler supports the needed directives, and the # preprocessing script has been written for our architecture. # -printf "checking whether we should preprocess assembly to add debugging information... " -if fnmatch '-g*|*\ -g*' "$CFLAGS_AUTO $CFLAGS" && - test -f "tools/add-cfi.$ARCH.awk" && +printf "checking whether we should preprocess assembly to add unwind information... " + +ADD_CFI="no" +if test -f "tools/add-cfi.$ARCH.awk" && printf ".file 1 \"srcfile.s\"\n.line 1\n.cfi_startproc\n.cfi_endproc" | $CC -g -x assembler -c -o /dev/null 2>/dev/null - then - ADD_CFI=yes -else - ADD_CFI=no + if test "$eh_frame" = "yes" && fnmatch '-g*|*\ -g*' "$CFLAGS_AUTO $CFLAGS" + then + ADD_CFI=".eh_frame, .debug_frame" + elif test "$eh_frame" = "yes" + then + ADD_CFI=".eh_frame" + elif fnmatch '-g*|*\ -g*' "$CFLAGS_AUTO $CFLAGS" + then + ADD_CFI=".debug_frame" + fi fi printf "%s\n" "$ADD_CFI" @@ -478,8 +490,10 @@ fi # unstrippable. These options force them back to debug sections (and # cause them not to get generated at all if debugging is off). # -tryflag CFLAGS_AUTO -fno-unwind-tables -tryflag CFLAGS_AUTO -fno-asynchronous-unwind-tables +if test "$eh_frame" = "no"; then + tryflag CFLAGS_AUTO -fno-unwind-tables + tryflag CFLAGS_AUTO -fno-asynchronous-unwind-tables +fi # # Attempt to put each function and each data object in its own diff --git a/tools/add-cfi.i386.awk b/tools/add-cfi.i386.awk index d05037de..f758acec 100644 --- a/tools/add-cfi.i386.awk +++ b/tools/add-cfi.i386.awk @@ -9,7 +9,7 @@ BEGIN { # don't put CFI data in the .eh_frame ELF section (which we don't keep) - print ".cfi_sections .debug_frame" + print ".cfi_sections " CFI_SECTIONS # only emit CFI directives inside a function in_function = 0 diff --git a/tools/add-cfi.x86_64.awk b/tools/add-cfi.x86_64.awk index 7e1513d6..4a2ae029 100644 --- a/tools/add-cfi.x86_64.awk +++ b/tools/add-cfi.x86_64.awk @@ -2,7 +2,7 @@ BEGIN { # don't put CFI data in the .eh_frame ELF section (which we don't keep) - print ".cfi_sections .debug_frame" + print ".cfi_sections " CFI_SECTIONS # only emit CFI directives inside a function in_function = 0 -- 2.32.0 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [musl] option to enable eh_frame 2021-07-16 9:16 [musl] option to enable eh_frame Timo Teras @ 2021-07-16 15:57 ` Rich Felker 2021-07-16 22:06 ` Ariadne Conill 2021-07-17 7:33 ` Timo Teras 0 siblings, 2 replies; 21+ messages in thread From: Rich Felker @ 2021-07-16 15:57 UTC (permalink / raw) To: Timo Teras; +Cc: musl On Fri, Jul 16, 2021 at 12:16:25PM +0300, Timo Teras wrote: > Hi, > > This has been discussed few times, and I know there are arguments also > not to do this. But at this time we at Alpine think the reasons to keep > eh_frame outweight the reasons to not include it. As explained on the tracker issue, what you'r requesting is a patch to make a configure option that changes the public interface contract of musl, making different interface contract profiles. This is not a change to be made lightly. There has been *some* consideration in the past for accepting this kind of option in the opposite direction: omitting large functionality that might not be needed in some contexts (like iconv), but where the default is to have it and omitting it is just a choice particular users can make for working in a very constrained environment. But if you add the option, you're essentially making "having it" the de facto default, even if configure has "disable" as the default. Once something is using it, there's an implicit requirement to have it. Honestly, proposing that it always be available (or configurable but on by default) would be less controversial than a configure option. I'd still be against it but at least some of the badness is gone. > Main arguments against .eh_frame being: > > 1) Having .eh_frame makes it seem like C++ exception throwing works > through C-library functions (e.g. throwing exception form qsort > callback to return over qsort back to application). > > Counter arguments: > - C++ exceptions is just one way to jump through musl functions. > E.g. setjmp/longjmp can do that just fine even without .eh_frame > > 2) Having application unwind itself for backtrace printing purposes > especially in signal handler is bad. This is agreed, but there's > still other cases when unwinding is good for debugging, and other > reasons. The fix for this root cause is to remove the unwinding from > signal handlers. The debugger already can do debugging/unwinding because it has access to the debug information (if you've installed it) and there is a clear, understood-by-users contract that this information is not an inherent part of the program but something optional for external debugging tools only. > Arguments to have .eh_frame: > - It allows debugging things even if musl-dbg is not or cannot be > installed for some reason (e.g. gdb, valgrind), or is no longer > available > - libunwind/libexecinfo will start to work and give meaningful > backtraces This is explicitly a reason not to. backtrace() considered harmful. > - Continuous kernel based profiling (e.g. perf record -g dwarf) will > work This already works if you have debug info. > Given that the main arguments against are either making UB crash, or > not the best fix, and keeping eh_frame enables useful features to work, > I think it would make sense to allow enabling it. > > Please consider the the attached patch to make it a configure option to > enable keeping eh_frame (defaulting still to not keeping it). You can solve this problem just as well for the things you want to have work by including the (part of) the debug info you want in the main libc.so binary: .debug_frame. Of course I can't stop Alpine from doing it in a different way locally, but I would strongly recommend you do that rather than making a contract that diverges from musl. Rich ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [musl] option to enable eh_frame 2021-07-16 15:57 ` Rich Felker @ 2021-07-16 22:06 ` Ariadne Conill 2021-07-17 0:41 ` Rich Felker 2021-07-17 7:33 ` Timo Teras 1 sibling, 1 reply; 21+ messages in thread From: Ariadne Conill @ 2021-07-16 22:06 UTC (permalink / raw) To: musl; +Cc: Timo Teras Hello, On Fri, 16 Jul 2021, Rich Felker wrote: > On Fri, Jul 16, 2021 at 12:16:25PM +0300, Timo Teras wrote: >> Hi, >> >> This has been discussed few times, and I know there are arguments also >> not to do this. But at this time we at Alpine think the reasons to keep >> eh_frame outweight the reasons to not include it. > > As explained on the tracker issue, what you'r requesting is a patch to > make a configure option that changes the public interface contract of > musl, making different interface contract profiles. This is not a > change to be made lightly. There has been *some* consideration in the > past for accepting this kind of option in the opposite direction: > omitting large functionality that might not be needed in some contexts > (like iconv), but where the default is to have it and omitting it is > just a choice particular users can make for working in a very > constrained environment. But if you add the option, you're essentially > making "having it" the de facto default, even if configure has > "disable" as the default. Once something is using it, there's an > implicit requirement to have it. > > Honestly, proposing that it always be available (or configurable but > on by default) would be less controversial than a configure option. > I'd still be against it but at least some of the badness is gone. > >> Main arguments against .eh_frame being: >> >> 1) Having .eh_frame makes it seem like C++ exception throwing works >> through C-library functions (e.g. throwing exception form qsort >> callback to return over qsort back to application). >> >> Counter arguments: >> - C++ exceptions is just one way to jump through musl functions. >> E.g. setjmp/longjmp can do that just fine even without .eh_frame >> >> 2) Having application unwind itself for backtrace printing purposes >> especially in signal handler is bad. This is agreed, but there's >> still other cases when unwinding is good for debugging, and other >> reasons. The fix for this root cause is to remove the unwinding from >> signal handlers. > > The debugger already can do debugging/unwinding because it has access > to the debug information (if you've installed it) and there is a > clear, understood-by-users contract that this information is not an > inherent part of the program but something optional for external > debugging tools only. > >> Arguments to have .eh_frame: >> - It allows debugging things even if musl-dbg is not or cannot be >> installed for some reason (e.g. gdb, valgrind), or is no longer >> available >> - libunwind/libexecinfo will start to work and give meaningful >> backtraces > > This is explicitly a reason not to. backtrace() considered harmful. > >> - Continuous kernel based profiling (e.g. perf record -g dwarf) will >> work > > This already works if you have debug info. > >> Given that the main arguments against are either making UB crash, or >> not the best fix, and keeping eh_frame enables useful features to work, >> I think it would make sense to allow enabling it. >> >> Please consider the the attached patch to make it a configure option to >> enable keeping eh_frame (defaulting still to not keeping it). > > You can solve this problem just as well for the things you want to > have work by including the (part of) the debug info you want in the > main libc.so binary: .debug_frame. Of course I can't stop Alpine from > doing it in a different way locally, but I would strongly recommend > you do that rather than making a contract that diverges from musl. The problem is that Alpine users want backtrace(3) to work. You consider it harmful, but users complain frequently. We also want C++ exception throwing across libc boundary to work, but admittedly that is a lot harder to achieve. I am concerned about the unilateral approach we have taken to enable backtrace(3) though, if we are forking the musl ABI, we probably will wind up forking the musl API too to add user-requested functionality. We (Alpine) should consider whether we actually want this. Historically, we have been able to find compromises that allow us to enable the user requests in a way that Rich finds acceptable. Ariadne ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [musl] option to enable eh_frame 2021-07-16 22:06 ` Ariadne Conill @ 2021-07-17 0:41 ` Rich Felker 2021-07-17 1:08 ` Ariadne Conill 2021-07-18 8:16 ` Timo Teras 0 siblings, 2 replies; 21+ messages in thread From: Rich Felker @ 2021-07-17 0:41 UTC (permalink / raw) To: Ariadne Conill; +Cc: musl, Timo Teras On Fri, Jul 16, 2021 at 05:06:48PM -0500, Ariadne Conill wrote: > Hello, > > On Fri, 16 Jul 2021, Rich Felker wrote: > > >On Fri, Jul 16, 2021 at 12:16:25PM +0300, Timo Teras wrote: > >>Hi, > >> > >>This has been discussed few times, and I know there are arguments also > >>not to do this. But at this time we at Alpine think the reasons to keep > >>eh_frame outweight the reasons to not include it. > > > >As explained on the tracker issue, what you'r requesting is a patch to > >make a configure option that changes the public interface contract of > >musl, making different interface contract profiles. This is not a > >change to be made lightly. There has been *some* consideration in the > >past for accepting this kind of option in the opposite direction: > >omitting large functionality that might not be needed in some contexts > >(like iconv), but where the default is to have it and omitting it is > >just a choice particular users can make for working in a very > >constrained environment. But if you add the option, you're essentially > >making "having it" the de facto default, even if configure has > >"disable" as the default. Once something is using it, there's an > >implicit requirement to have it. > > > >Honestly, proposing that it always be available (or configurable but > >on by default) would be less controversial than a configure option. > >I'd still be against it but at least some of the badness is gone. > > > >>Main arguments against .eh_frame being: > >> > >>1) Having .eh_frame makes it seem like C++ exception throwing works > >> through C-library functions (e.g. throwing exception form qsort > >> callback to return over qsort back to application). > >> > >> Counter arguments: > >> - C++ exceptions is just one way to jump through musl functions. > >> E.g. setjmp/longjmp can do that just fine even without .eh_frame > >> > >>2) Having application unwind itself for backtrace printing purposes > >> especially in signal handler is bad. This is agreed, but there's > >> still other cases when unwinding is good for debugging, and other > >> reasons. The fix for this root cause is to remove the unwinding from > >> signal handlers. > > > >The debugger already can do debugging/unwinding because it has access > >to the debug information (if you've installed it) and there is a > >clear, understood-by-users contract that this information is not an > >inherent part of the program but something optional for external > >debugging tools only. > > > >>Arguments to have .eh_frame: > >> - It allows debugging things even if musl-dbg is not or cannot be > >> installed for some reason (e.g. gdb, valgrind), or is no longer > >> available > >> - libunwind/libexecinfo will start to work and give meaningful > >> backtraces > > > >This is explicitly a reason not to. backtrace() considered harmful. > > > >> - Continuous kernel based profiling (e.g. perf record -g dwarf) will > >> work > > > >This already works if you have debug info. > > > >>Given that the main arguments against are either making UB crash, or > >>not the best fix, and keeping eh_frame enables useful features to work, > >>I think it would make sense to allow enabling it. > >> > >>Please consider the the attached patch to make it a configure option to > >>enable keeping eh_frame (defaulting still to not keeping it). > > > >You can solve this problem just as well for the things you want to > >have work by including the (part of) the debug info you want in the > >main libc.so binary: .debug_frame. Of course I can't stop Alpine from > >doing it in a different way locally, but I would strongly recommend > >you do that rather than making a contract that diverges from musl. > > The problem is that Alpine users want backtrace(3) to work. You This is a very different narrative from the request I received to take this patch. I'm concerned about that. > consider it harmful, but users complain frequently. We also want Alpine is a security-oriented distribution. Doing introspective debugging when a crash happens is *inviting* exploitation; the reason that's the case has been discussed extensively before. Applications trying to do this should be patched not to do it. > C++ exception throwing across libc boundary to work, but admittedly > that is a lot harder to achieve. Do you have in mind some place where this would be remotely valid usage? I don't mean per the spec, where it's never valid, but some kind of "at least this makes sense someone could want to do it" sense. There are only a few places where the application can even see an environment called back from libc. Most of these are things like comparison functions that are required to be pure. There is no way in which throwing from them could ever make sense, because throwing is not a pure operation. Another is things like enumeration callbacks. Most of these necessarily have inconsistent state and/or resources held that would be lost if you could asynchronously jump out of them, and most of them have a way for the callback to request the operation to stop. This means that if you want to use exceptions with them, the top-level callback just has to catch the exception and convert it to a return value. The same applies for things like fopencookie. > I am concerned about the unilateral approach we have taken to enable > backtrace(3) though, if we are forking the musl ABI, we probably > will wind up forking the musl API too to add user-requested > functionality. This would be very unfortunate, and would make me actively recommend against use of Alpine. One of the core values of musl is consensus process, not attempts to unilaterally force something like this, especially when it's been done in a way that feels misleading. Communication with me/upstream musl started out as presenting this as being about debugging, but it feels like that was a bait-and-switch for getting something else. > We (Alpine) should consider whether we actually want this. > Historically, we have been able to find compromises that allow us to > enable the user requests in a way that Rich finds acceptable. Yes. As a distro, you oftentimes have users asking for something that's wrong, that shouldn't exist. One core job of a maintainer -- a distro maintainer too -- is saying no. Here, users seem to be asking for something that makes software less secure, and that's not a portable feature one can rely on having. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [musl] option to enable eh_frame 2021-07-17 0:41 ` Rich Felker @ 2021-07-17 1:08 ` Ariadne Conill 2021-07-17 1:10 ` Érico Nogueira 2021-07-17 7:24 ` Laurent Bercot 2021-07-18 8:16 ` Timo Teras 1 sibling, 2 replies; 21+ messages in thread From: Ariadne Conill @ 2021-07-17 1:08 UTC (permalink / raw) To: musl; +Cc: Ariadne Conill, Timo Teras Hello, On Fri, 16 Jul 2021, Rich Felker wrote: > On Fri, Jul 16, 2021 at 05:06:48PM -0500, Ariadne Conill wrote: >> Hello, >> >> On Fri, 16 Jul 2021, Rich Felker wrote: >> >>> On Fri, Jul 16, 2021 at 12:16:25PM +0300, Timo Teras wrote: >>>> Hi, >>>> >>>> This has been discussed few times, and I know there are arguments also >>>> not to do this. But at this time we at Alpine think the reasons to keep >>>> eh_frame outweight the reasons to not include it. >>> >>> As explained on the tracker issue, what you'r requesting is a patch to >>> make a configure option that changes the public interface contract of >>> musl, making different interface contract profiles. This is not a >>> change to be made lightly. There has been *some* consideration in the >>> past for accepting this kind of option in the opposite direction: >>> omitting large functionality that might not be needed in some contexts >>> (like iconv), but where the default is to have it and omitting it is >>> just a choice particular users can make for working in a very >>> constrained environment. But if you add the option, you're essentially >>> making "having it" the de facto default, even if configure has >>> "disable" as the default. Once something is using it, there's an >>> implicit requirement to have it. >>> >>> Honestly, proposing that it always be available (or configurable but >>> on by default) would be less controversial than a configure option. >>> I'd still be against it but at least some of the badness is gone. >>> >>>> Main arguments against .eh_frame being: >>>> >>>> 1) Having .eh_frame makes it seem like C++ exception throwing works >>>> through C-library functions (e.g. throwing exception form qsort >>>> callback to return over qsort back to application). >>>> >>>> Counter arguments: >>>> - C++ exceptions is just one way to jump through musl functions. >>>> E.g. setjmp/longjmp can do that just fine even without .eh_frame >>>> >>>> 2) Having application unwind itself for backtrace printing purposes >>>> especially in signal handler is bad. This is agreed, but there's >>>> still other cases when unwinding is good for debugging, and other >>>> reasons. The fix for this root cause is to remove the unwinding from >>>> signal handlers. >>> >>> The debugger already can do debugging/unwinding because it has access >>> to the debug information (if you've installed it) and there is a >>> clear, understood-by-users contract that this information is not an >>> inherent part of the program but something optional for external >>> debugging tools only. >>> >>>> Arguments to have .eh_frame: >>>> - It allows debugging things even if musl-dbg is not or cannot be >>>> installed for some reason (e.g. gdb, valgrind), or is no longer >>>> available >>>> - libunwind/libexecinfo will start to work and give meaningful >>>> backtraces >>> >>> This is explicitly a reason not to. backtrace() considered harmful. >>> >>>> - Continuous kernel based profiling (e.g. perf record -g dwarf) will >>>> work >>> >>> This already works if you have debug info. >>> >>>> Given that the main arguments against are either making UB crash, or >>>> not the best fix, and keeping eh_frame enables useful features to work, >>>> I think it would make sense to allow enabling it. >>>> >>>> Please consider the the attached patch to make it a configure option to >>>> enable keeping eh_frame (defaulting still to not keeping it). >>> >>> You can solve this problem just as well for the things you want to >>> have work by including the (part of) the debug info you want in the >>> main libc.so binary: .debug_frame. Of course I can't stop Alpine from >>> doing it in a different way locally, but I would strongly recommend >>> you do that rather than making a contract that diverges from musl. >> >> The problem is that Alpine users want backtrace(3) to work. You > > This is a very different narrative from the request I received to take > this patch. I'm concerned about that. > >> consider it harmful, but users complain frequently. We also want > > Alpine is a security-oriented distribution. Doing introspective > debugging when a crash happens is *inviting* exploitation; the reason > that's the case has been discussed extensively before. Applications > trying to do this should be patched not to do it. Alpine does patch applications not to do it. This is not about enabling backtrace(3) for Alpine-shipped packages, but allowing end-users to use backtrace(3) and tools like Valgrind with their own applications that they build locally. While we advise users to install musl-dbg to solve these problems, they argue that basic functionality is available on their GNU/Linux distribution without having to install libc6-dbg or glibc-debuginfo or whatever equivalent package. This comes up a lot with users using Alpine as a mechanism to get a musl-based runtime environment for Docker. The lifecycle for applications running inside Docker is not particularly friendly to traditional debug methods, and so having working backtrace(3) is helpful for that scenario, despite unwinding in a SIGSEGV handler being dangerous. >> C++ exception throwing across libc boundary to work, but admittedly >> that is a lot harder to achieve. > > Do you have in mind some place where this would be remotely valid > usage? I don't mean per the spec, where it's never valid, but some > kind of "at least this makes sense someone could want to do it" sense. > > There are only a few places where the application can even see an > environment called back from libc. Most of these are things like > comparison functions that are required to be pure. There is no way in > which throwing from them could ever make sense, because throwing is > not a pure operation. Another is things like enumeration callbacks. > Most of these necessarily have inconsistent state and/or resources > held that would be lost if you could asynchronously jump out of them, > and most of them have a way for the callback to request the operation > to stop. This means that if you want to use exceptions with them, the > top-level callback just has to catch the exception and convert it to a > return value. The same applies for things like fopencookie. This is just a commonly requested thing. Alpine does not really care about enabling this usecase right now. I should clarify what Alpine wants here: we want users who are building their applications on Alpine to not be surprised by musl's behaviour. That can be solved either through allowing exceptions to cross libc (like on GNU/Linux with glibc) or through documentation. I prefer to solve this issue with documentation, personally. But regardless, backtrace(3) should work for our users if they wish to use it in their downstream applications. >> I am concerned about the unilateral approach we have taken to enable >> backtrace(3) though, if we are forking the musl ABI, we probably >> will wind up forking the musl API too to add user-requested >> functionality. > > This would be very unfortunate, and would make me actively recommend > against use of Alpine. One of the core values of musl is consensus > process, not attempts to unilaterally force something like this, > especially when it's been done in a way that feels misleading. > Communication with me/upstream musl started out as presenting this as > being about debugging, but it feels like that was a bait-and-switch > for getting something else. It is about debugging, but specifically enabling downstream users to do basic debugging without having to install the musl-dbg package. This is commonly requested both in public Alpine channels, and also in various support engagements Alpine developers have with their customers in a personal capacity. It has been a point of contention for many years in the Alpine community, as a result. Nonetheless, I am disappointed that the implementation of `.eh_frame` was done unilaterally. I should have responded sooner on the tracker item, but had forgotten about it until it was implemented today. My concern is that Alpine is entering a "we broke it, we bought it" situation with this kind of unilateral decision making. While we have built a very nice platform around musl, I don't want to be in a situation where we "own" a substantively modified version of musl. We have been in that situation before with uClibc, and we do not want to go back there. Accordingly, I would strongly prefer that Alpine try to solve this issue with `.debug_frame` first, and then we can look at `.eh_frame` later if it winds up being insufficient for making basic debug functionality work (on at least the same level as glibc). >> We (Alpine) should consider whether we actually want this. >> Historically, we have been able to find compromises that allow us to >> enable the user requests in a way that Rich finds acceptable. > > Yes. As a distro, you oftentimes have users asking for something > that's wrong, that shouldn't exist. One core job of a maintainer -- a > distro maintainer too -- is saying no. Here, users seem to be asking > for something that makes software less secure, and that's not a > portable feature one can rely on having. I am not convinced that backtrace(3) and basic Valgrind functionality makes Alpine less secure. Making the software *we* ship to downstream users spew backtrace logs all over the place when it crashes or hits an assertion would make Alpine less secure, but we don't and won't be doing that. Ariadne ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [musl] option to enable eh_frame 2021-07-17 1:08 ` Ariadne Conill @ 2021-07-17 1:10 ` Érico Nogueira 2021-07-17 7:24 ` Laurent Bercot 1 sibling, 0 replies; 21+ messages in thread From: Érico Nogueira @ 2021-07-17 1:10 UTC (permalink / raw) To: musl; +Cc: Ariadne Conill, Timo Teras Hullo, Alpine MR is <https://gitlab.alpinelinux.org/alpine/aports/-/merge_requests/22958> On Fri Jul 16, 2021 at 10:08 PM -03, Ariadne Conill wrote: > Hello, > > On Fri, 16 Jul 2021, Rich Felker wrote: > > > On Fri, Jul 16, 2021 at 05:06:48PM -0500, Ariadne Conill wrote: > >> Hello, > >> > >> On Fri, 16 Jul 2021, Rich Felker wrote: > >> > >>> On Fri, Jul 16, 2021 at 12:16:25PM +0300, Timo Teras wrote: > >>>> Hi, > >>>> > >>>> This has been discussed few times, and I know there are arguments also > >>>> not to do this. But at this time we at Alpine think the reasons to keep > >>>> eh_frame outweight the reasons to not include it. > >>> > >>> As explained on the tracker issue, what you'r requesting is a patch to > >>> make a configure option that changes the public interface contract of > >>> musl, making different interface contract profiles. This is not a > >>> change to be made lightly. There has been *some* consideration in the > >>> past for accepting this kind of option in the opposite direction: > >>> omitting large functionality that might not be needed in some contexts > >>> (like iconv), but where the default is to have it and omitting it is > >>> just a choice particular users can make for working in a very > >>> constrained environment. But if you add the option, you're essentially > >>> making "having it" the de facto default, even if configure has > >>> "disable" as the default. Once something is using it, there's an > >>> implicit requirement to have it. > >>> > >>> Honestly, proposing that it always be available (or configurable but > >>> on by default) would be less controversial than a configure option. > >>> I'd still be against it but at least some of the badness is gone. > >>> > >>>> Main arguments against .eh_frame being: > >>>> > >>>> 1) Having .eh_frame makes it seem like C++ exception throwing works > >>>> through C-library functions (e.g. throwing exception form qsort > >>>> callback to return over qsort back to application). > >>>> > >>>> Counter arguments: > >>>> - C++ exceptions is just one way to jump through musl functions. > >>>> E.g. setjmp/longjmp can do that just fine even without .eh_frame > >>>> > >>>> 2) Having application unwind itself for backtrace printing purposes > >>>> especially in signal handler is bad. This is agreed, but there's > >>>> still other cases when unwinding is good for debugging, and other > >>>> reasons. The fix for this root cause is to remove the unwinding from > >>>> signal handlers. > >>> > >>> The debugger already can do debugging/unwinding because it has access > >>> to the debug information (if you've installed it) and there is a > >>> clear, understood-by-users contract that this information is not an > >>> inherent part of the program but something optional for external > >>> debugging tools only. > >>> > >>>> Arguments to have .eh_frame: > >>>> - It allows debugging things even if musl-dbg is not or cannot be > >>>> installed for some reason (e.g. gdb, valgrind), or is no longer > >>>> available > >>>> - libunwind/libexecinfo will start to work and give meaningful > >>>> backtraces > >>> > >>> This is explicitly a reason not to. backtrace() considered harmful. > >>> > >>>> - Continuous kernel based profiling (e.g. perf record -g dwarf) will > >>>> work > >>> > >>> This already works if you have debug info. > >>> > >>>> Given that the main arguments against are either making UB crash, or > >>>> not the best fix, and keeping eh_frame enables useful features to work, > >>>> I think it would make sense to allow enabling it. > >>>> > >>>> Please consider the the attached patch to make it a configure option to > >>>> enable keeping eh_frame (defaulting still to not keeping it). > >>> > >>> You can solve this problem just as well for the things you want to > >>> have work by including the (part of) the debug info you want in the > >>> main libc.so binary: .debug_frame. Of course I can't stop Alpine from > >>> doing it in a different way locally, but I would strongly recommend > >>> you do that rather than making a contract that diverges from musl. > >> > >> The problem is that Alpine users want backtrace(3) to work. You > > > > This is a very different narrative from the request I received to take > > this patch. I'm concerned about that. > > > >> consider it harmful, but users complain frequently. We also want > > > > Alpine is a security-oriented distribution. Doing introspective > > debugging when a crash happens is *inviting* exploitation; the reason > > that's the case has been discussed extensively before. Applications > > trying to do this should be patched not to do it. > > Alpine does patch applications not to do it. This is not about enabling > backtrace(3) for Alpine-shipped packages, but allowing end-users to use > backtrace(3) and tools like Valgrind with their own applications that > they > build locally. > > While we advise users to install musl-dbg to solve these problems, they > argue that basic functionality is available on their GNU/Linux > distribution without having to install libc6-dbg or glibc-debuginfo or > whatever equivalent package. It would probably double the size, but could alpine offer a "development" image that already comes with musl-dbg (assuming the dislike for installing musl-dbg is about having to add it to Dockerfiles or w/e)? Void avoids this "debate" by simply not stripping libc (either musl or glibc), since it makes local debugging with gdb/valgrind so much easier. I wasn't aware libexecinfo also took so much advantage of it. > > This comes up a lot with users using Alpine as a mechanism to get a > musl-based runtime environment for Docker. The lifecycle for > applications > running inside Docker is not particularly friendly to traditional debug > methods, and so having working backtrace(3) is helpful for that > scenario, > despite unwinding in a SIGSEGV handler being dangerous. backtrace(3) in dev can easily become backtrace(3) in prod; I'd certainly be against encouraging proliferation of its usage. However, I haven't had to deal with corporate distro clients either, so I don't have a handle on the complete picture. Maybe document some setup to export core dumps to outside of a container in the case of crashes? (is that doable? I'm not a container person) Catching SIGSEGV at all has always seemed like a pretty weird idea to me. > > >> C++ exception throwing across libc boundary to work, but admittedly > >> that is a lot harder to achieve. > > > > Do you have in mind some place where this would be remotely valid > > usage? I don't mean per the spec, where it's never valid, but some > > kind of "at least this makes sense someone could want to do it" sense. > > > > There are only a few places where the application can even see an > > environment called back from libc. Most of these are things like > > comparison functions that are required to be pure. There is no way in > > which throwing from them could ever make sense, because throwing is > > not a pure operation. Another is things like enumeration callbacks. > > Most of these necessarily have inconsistent state and/or resources > > held that would be lost if you could asynchronously jump out of them, > > and most of them have a way for the callback to request the operation > > to stop. This means that if you want to use exceptions with them, the > > top-level callback just has to catch the exception and convert it to a > > return value. The same applies for things like fopencookie. > > This is just a commonly requested thing. Alpine does not really care > about enabling this usecase right now. I too would be interested to know what examples came with such requests, if you can share them. > > I should clarify what Alpine wants here: we want users who are building > their applications on Alpine to not be surprised by musl's behaviour. > That can be solved either through allowing exceptions to cross libc > (like > on GNU/Linux with glibc) or through documentation. I prefer to solve > this > issue with documentation, personally. Do you think this sort of thing fits better in the wiki [1] or in the WIP musl manual [2]? Part of the reason people choose musl at all is because of its design choices, which make it more than just a glibc clone. I'm not sure it's possible to have it all - benefits of musl *and* glibc in the same package. Hopefully that notion can appease those who specifically chose musl, but it would probably be discouraging to anyone who wants this functionality to debug their CI failures, where musl is more about coverage and less about a platform choice. Links for anyone unaware, though they are available from the homepage: [1] https://wiki.musl-libc.org/ [2] http://musl.libc.org/manual.html > > But regardless, backtrace(3) should work for our users if they wish to > use > it in their downstream applications. > > >> I am concerned about the unilateral approach we have taken to enable > >> backtrace(3) though, if we are forking the musl ABI, we probably > >> will wind up forking the musl API too to add user-requested > >> functionality. > > > > This would be very unfortunate, and would make me actively recommend > > against use of Alpine. One of the core values of musl is consensus > > process, not attempts to unilaterally force something like this, > > especially when it's been done in a way that feels misleading. > > Communication with me/upstream musl started out as presenting this as > > being about debugging, but it feels like that was a bait-and-switch > > for getting something else. > > It is about debugging, but specifically enabling downstream users to do > basic debugging without having to install the musl-dbg package. This is > commonly requested both in public Alpine channels, and also in various > support engagements Alpine developers have with their customers in a > personal capacity. It has been a point of contention for many years in > the Alpine community, as a result. > > Nonetheless, I am disappointed that the implementation of `.eh_frame` > was > done unilaterally. I should have responded sooner on the tracker item, > but had forgotten about it until it was implemented today. > > My concern is that Alpine is entering a "we broke it, we bought it" > situation with this kind of unilateral decision making. While we have > built a very nice platform around musl, I don't want to be in a > situation > where we "own" a substantively modified version of musl. We have been in > that situation before with uClibc, and we do not want to go back there. > > Accordingly, I would strongly prefer that Alpine try to solve this issue > with `.debug_frame` first, and then we can look at `.eh_frame` later if > it > winds up being insufficient for making basic debug functionality work > (on > at least the same level as glibc). > > >> We (Alpine) should consider whether we actually want this. > >> Historically, we have been able to find compromises that allow us to > >> enable the user requests in a way that Rich finds acceptable. > > > > Yes. As a distro, you oftentimes have users asking for something > > that's wrong, that shouldn't exist. One core job of a maintainer -- a > > distro maintainer too -- is saying no. Here, users seem to be asking > > for something that makes software less secure, and that's not a > > portable feature one can rely on having. > > I am not convinced that backtrace(3) and basic Valgrind functionality > makes Alpine less secure. Making the software *we* ship to downstream > users spew backtrace logs all over the place when it crashes or hits an > assertion would make Alpine less secure, but we don't and won't be doing > that. Can you share an example of valgrind output before and after the .eh_frame change? It would be nice to have some more context. > > Ariadne ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [musl] option to enable eh_frame 2021-07-17 1:08 ` Ariadne Conill 2021-07-17 1:10 ` Érico Nogueira @ 2021-07-17 7:24 ` Laurent Bercot 1 sibling, 0 replies; 21+ messages in thread From: Laurent Bercot @ 2021-07-17 7:24 UTC (permalink / raw) To: musl >It is about debugging, but specifically enabling downstream users to do basic debugging without having to install the musl-dbg package. This is commonly requested both in public Alpine channels, and also in various support engagements Alpine developers have with their customers in a personal capacity. It has been a point of contention for many years in the Alpine community, as a result. From what I can see, this point of contention has not exactly stopped Alpine's expansion. Using Alpine comes with benefits; it also comes with some drawbacks, and every reasonable user should be able to understand this. As a maintainer, it obviously makes sense to try and minimize these drawbacks, but are we really talking about a situation where the user's complaint is 'I have to type "apk add musl-dbg" before performing debugging' ? Are we really talking about a situation where the proposed solution to that complaint is *forking the libc that is a huge part of Alpine's success in the first place* ? I suspect there is a lot being left unsaid here, because at face value, the cost-benefit analysis is obvious and I don't understand how this is even a question. Without any more information, what it looks like to me is corporate lobbying, which Alpine has always managed to deal with in a reasonable manner in the past and I am not sure what changed. -- Laurent ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [musl] option to enable eh_frame 2021-07-17 0:41 ` Rich Felker 2021-07-17 1:08 ` Ariadne Conill @ 2021-07-18 8:16 ` Timo Teras 1 sibling, 0 replies; 21+ messages in thread From: Timo Teras @ 2021-07-18 8:16 UTC (permalink / raw) To: Rich Felker; +Cc: Ariadne Conill, musl Hi, I want to still clarify few things here. On Fri, 16 Jul 2021 20:41:31 -0400 Rich Felker <dalias@libc.org> wrote: > On Fri, Jul 16, 2021 at 05:06:48PM -0500, Ariadne Conill wrote: > > > > On Fri, 16 Jul 2021, Rich Felker wrote: > > > > >You can solve this problem just as well for the things you want to > > >have work by including the (part of) the debug info you want in the > > >main libc.so binary: .debug_frame. Of course I can't stop Alpine > > >from doing it in a different way locally, but I would strongly > > >recommend you do that rather than making a contract that diverges > > >from musl. > > > > The problem is that Alpine users want backtrace(3) to work. You > > This is a very different narrative from the request I received to take > this patch. I'm concerned about that. This was not the driving pattern for me. And is actually sort of unrelated as backtrace(3) can be made work libunwind --enable-debug-frame and .debug_frame. I see this as quite different thing. > Alpine is a security-oriented distribution. Doing introspective > debugging when a crash happens is *inviting* exploitation; the reason > that's the case has been discussed extensively before. Applications > trying to do this should be patched not to do it. I think musl .eh_frame is not big thing on this. Most backtrace() issues would likely be exploitable if the application has .eh_frame and it starts backtracing it's own stack. This is about whether or not to include backtrace() at all in OS. > > I am concerned about the unilateral approach we have taken to enable > > backtrace(3) though, if we are forking the musl ABI, we probably > > will wind up forking the musl API too to add user-requested > > functionality. > > This would be very unfortunate, and would make me actively recommend > against use of Alpine. One of the core values of musl is consensus > process, not attempts to unilaterally force something like this, > especially when it's been done in a way that feels misleading. This is quite different stand from what you communicated on the MR -or at least how I understood it. I got the impression from your comment that we can do whatever we want and you don't care too much, but your preference is to use .debug_frame. I did not understand that you feel so strongly about this that it would affect your recommendation or relationship to us. We all want to work together with you, not against. The fact that I presented more questions and counter-arguments in the MR and not getting responses lead me to think that you don't care. This was a misinterpretation on my side. We have at times carried some patches for long time on musl - usually cherry-picked commits - but at times non-upstreamed ones too, and I think there is still at least one non-upstreamed. Please also understand that me and Ariadne have been talking about the reasons separately as individuals, not on behalf of the Alpine project. And we had not established any scheme to get this done to sneak in unwanted features by misleading. My motivation has been only the debugging side. I know others have other reasons to it too. As explained I have valid technical reasons to want and prefer .eh_frame for debugging purposes, and I for me the "contract" to users is the same regardless of which section we provide. Ariadne went already to revert the change - if she hadn't, I would have done it now. And as said in the other thread, I'm working to do the .debug_frame. But given the reasons, I'd still hope we can discuss the matter of .eh_frame (as default-on or default-off) could be made acceptable as debugging feature by some other changes that still give same "contract" of showing user that it's not there to support unwinding through C-library? E.g. by wiring the most problematic functions to abort on unwind attempt. Timo ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [musl] option to enable eh_frame 2021-07-16 15:57 ` Rich Felker 2021-07-16 22:06 ` Ariadne Conill @ 2021-07-17 7:33 ` Timo Teras 2021-07-17 10:38 ` Timo Teras ` (2 more replies) 1 sibling, 3 replies; 21+ messages in thread From: Timo Teras @ 2021-07-17 7:33 UTC (permalink / raw) To: Rich Felker; +Cc: musl On Fri, 16 Jul 2021 11:57:35 -0400 Rich Felker <dalias@libc.org> wrote: > On Fri, Jul 16, 2021 at 12:16:25PM +0300, Timo Teras wrote: > > The debugger already can do debugging/unwinding because it has access > to the debug information (if you've installed it) and there is a > clear, understood-by-users contract that this information is not an > inherent part of the program but something optional for external > debugging tools only. This apparently the fundamental difference in thinking. The debugging information is often huge, and people don't want to install it on production or embedded devices. It ships ton of other features which are not needed. However, the unwind/backtrace feature does provide significant value for error reports and profiling. > > > Arguments to have .eh_frame: > > - It allows debugging things even if musl-dbg is not or cannot be > > installed for some reason (e.g. gdb, valgrind), or is no longer > > available > > - libunwind/libexecinfo will start to work and give meaningful > > backtraces > > This is explicitly a reason not to. backtrace() considered harmful. Please elaborate. I agree it's harmful from SEGV handler when program might be targeted. But used in other situations where it is useful. Do you have additional reasons why backtrace() is harmful? Do note that libunwind --enable-debug-frame is enabled default by upstream package on ARM and aarch64, and this would work if the debug package is installed. This gives inconsistent behaviour based on arch and whether -dbg is installed. Developer might think the unwind stuff works, only to find it breaks on different install, and make the application depend on -dbg. > > - Continuous kernel based profiling (e.g. perf record -g dwarf) > > will work > > This already works if you have debug info. As said, it was desire to make it work without debug info. > > Given that the main arguments against are either making UB crash, or > > not the best fix, and keeping eh_frame enables useful features to > > work, I think it would make sense to allow enabling it. > > > > Please consider the the attached patch to make it a configure > > option to enable keeping eh_frame (defaulting still to not keeping > > it). > > You can solve this problem just as well for the things you want to > have work by including the (part of) the debug info you want in the > main libc.so binary: .debug_frame. Of course I can't stop Alpine from > doing it in a different way locally, but I would strongly recommend > you do that rather than making a contract that diverges from musl. The .debug_frame in main is a possibility, but no one else does it. Well, apparently some ship the full debug build always to achieve some of the same things. I think this also achieves the same divergence from the normal expectation. As small side note, the -fno-*unwind-tables flags are also in "tryflags" so if compiler would not support it, you don't get it. So depending on compiler you might still get .eh_frame ;) The .eh_frame is a ELF abi feature normally turned on, and usually needs good reasons to turn it off. Could you Rich please explain in detail all the reasons why you think it's so evil? Mostly I heard "feature X is evil and people should not use it" and I find that your personal opinion, not a valid technical reason. But listening more and earlier things, I've heard: 1. Having it makes it seem an application can jump through C library from e.g. qsort callback. Not having .eh_frame makes C++ exception fail, but does not help with e.g. setjmp/longjmp. So this is not complete solution. 2. Explicitly disabling backtrace() universally. I assume this is to disable to widespread usage of it from SEGV handler. But there seem to be also implementations reading .debug_frame, so this does not really fix that either fully. And I think there are valid use cases for backtrace() also, e.g. when having debug build and having internal debug printing trigger it. 3. Adds little bit if size to the binary. And also the runtime memory usage as it goes to LOAD segment. But 80kB is not a concern on modern environment even on embedded platforms. 4. You want to still enforce this so people don't do 1 or 2. For this, time has taught me, people need education, not enforcement. If you don't teach them, they'll still go and do the same stupid things even more stupid ways. Please add in any reasons I may have missed. I would like to have your complete list of reasons why to remove .eh_frame. So far it has been coming out in pieces. I'd like constructive discussion if some of these items could be implemented stronger in other means than removing .eh_frame. Please also explain why you think the .eh_frame is sufficient even though it does not cover various situations as explained above. Now it may have been little bit early for me to go ahead and merge the change in Alpine before going through this discussion. The intention was not upset anyone or be unilateral. This is why I made the PR and waited for a good while for comments. Unfortunately, I did not find the comments convincing, but granted I should have asked the above questions before merging. Currently, I still think reasons to have .eh_frame than not are more weighty - it enables valid use cases in expected way. But I'm also willing to revert if there's stronger reasons to do it. Either technical, or just keeping peace. So far the technical reasons do not convince me, but keeping peace and good relationship is something I definitely want. I personally did not see this as an ABI or a "contract" change, and this is perhaps why I made the merge earlier than later. While the "contract" may *seem* somewhat different. It was still mentioned in the commit message that the change was not made to make C++ exception through C library work. The "contract promise" has not changed; though, granted, the user perceived "de facto contract" has changed in this case. As said, for me the primary reason is observability: being able to gdb and get back traces, and continuous profiling work without debug info. Those are precious things to have on system where debug info is simply not available. As mentioned backtrace() working is a side effect, and I think it has also valid use cases even if it's also often misused. But fundamentally I think if we ship .debug_frame, all people wanting do backtrace() and the silly stuff, will just enable .debug_frame support in their code and still do the silly stuff in worse way, than if .eh_frame was enabled. Since technically they are the same, even if the intended use case is different. As seen libunwind does have --enable-debug-frame. For me it's quite unusual to have .debug_frame but not other debug sections. I assume it works. But the tooling certain does not endorse this setup. I suppose it can be achieved with some objcopy magic, but requires care. Timo ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [musl] option to enable eh_frame 2021-07-17 7:33 ` Timo Teras @ 2021-07-17 10:38 ` Timo Teras 2021-07-17 13:25 ` Rich Felker 2022-04-17 19:50 ` Fangrui Song 2 siblings, 0 replies; 21+ messages in thread From: Timo Teras @ 2021-07-17 10:38 UTC (permalink / raw) To: Rich Felker; +Cc: musl On Sat, 17 Jul 2021 10:33:38 +0300 Timo Teras <timo.teras@iki.fi> wrote: > On Fri, 16 Jul 2021 11:57:35 -0400 > Rich Felker <dalias@libc.org> wrote: > > > You can solve this problem just as well for the things you want to > > have work by including the (part of) the debug info you want in the > > main libc.so binary: .debug_frame. > > For me it's quite unusual to have .debug_frame but not other debug > sections. I assume it works. But the tooling certain does not endorse > this setup. I suppose it can be achieved with some objcopy magic, but > requires care. I'm investigating this. And seems that "strip -g --keep-section=.debug_frame" works after all. I remember objcopy often not doing what wanted, but happily this is doable in a simple enough way. However, there actually is some reasons why .eh_frame that goes to LOAD phdr is helpful. That is core dump (when all of the mapped memory is dumped). This allows easy investigation of crashed program state with proper tooling even if none of the debug info is available. .debug_info would not go there because it's not mapped. Granted, the above is a bit of rare condition and probably not of huge importance for many. But I do use it regularly. Another difference is that there's no .eh_frame_hdr equivalent which allows binary searchable tree based on PC. But then again, this is mostly useful for backtrace() style usage. External stuff like debuggers and profilers etc. can build this runtime if needed - it just is a small (hopefully onetime) extra performance hit. I think just keeping .debug_frame would solve the problems I'm having (modulo the core dump case). But since the backtrace() and other things are also involved, I hope we could have a proper discussion on the technical side. And also consider if "the badness" could be made "bad" (by crashing or similar) even if including .eh_frame. Speaking of this, I'm wondering how many C-library functions can call callbacks? Another approach would be to arm these functions with __attribute__((cleanup())) where the clean up crashes if called via unwind. Probably bad idea extra work wise, but would make it very obvious unconditionally that unwinding C-functions is not wanted. Makes one hope gcc had a way to insert personality function to crash on unwind attempt... Also, what if the code base is compiled with frame-pointers enabled? Does that make unwinding work even without eh_frame? Is the "-fomit-frame-pointer" vs. "-fno-omit-frame-pointer" affecting current perceived "contract"? Timo ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [musl] option to enable eh_frame 2021-07-17 7:33 ` Timo Teras 2021-07-17 10:38 ` Timo Teras @ 2021-07-17 13:25 ` Rich Felker 2021-07-17 15:40 ` Timo Teras 2022-04-17 19:50 ` Fangrui Song 2 siblings, 1 reply; 21+ messages in thread From: Rich Felker @ 2021-07-17 13:25 UTC (permalink / raw) To: Timo Teras; +Cc: musl On Sat, Jul 17, 2021 at 10:33:38AM +0300, Timo Teras wrote: > On Fri, 16 Jul 2021 11:57:35 -0400 > Rich Felker <dalias@libc.org> wrote: > > > On Fri, Jul 16, 2021 at 12:16:25PM +0300, Timo Teras wrote: > > > > The debugger already can do debugging/unwinding because it has access > > to the debug information (if you've installed it) and there is a > > clear, understood-by-users contract that this information is not an > > inherent part of the program but something optional for external > > debugging tools only. > > This apparently the fundamental difference in thinking. The debugging > information is often huge, and people don't want to install it on > production or embedded devices. It ships ton of other features which > are not needed. However, the unwind/backtrace feature does provide > significant value for error reports and profiling. > > > > > Arguments to have .eh_frame: > > > - It allows debugging things even if musl-dbg is not or cannot be > > > installed for some reason (e.g. gdb, valgrind), or is no longer > > > available > > > - libunwind/libexecinfo will start to work and give meaningful > > > backtraces > > > > This is explicitly a reason not to. backtrace() considered harmful. > > Please elaborate. I agree it's harmful from SEGV handler when program > might be targeted. But used in other situations where it is useful. Do > you have additional reasons why backtrace() is harmful? That's the main one in which it's explicitly *harmful*, but in general, introspection is not part of the C language and puts a lot of constraints on implementation if you want to programs to be able to do it in a meaningful way. This has never been something musl intended to support, for that reason. Note that we're even cautious (or regretful) over leaky abstractions like RTLD_NEXT which fails to work right if you happen to call dlsym in a tail call position. > Do note that libunwind --enable-debug-frame is enabled default by > upstream package on ARM and aarch64, and this would work if the debug > package is installed. > > This gives inconsistent behaviour based on arch and whether -dbg is > installed. Developer might think the unwind stuff works, only to find > it breaks on different install, and make the application depend on -dbg. This is an important distinction and why it should be in -dbg. "Application depending on.." is not functionality musl supports. "Application interfacing with a debugging tool that's loading it's own -dbg files" is visibly something different. > The .debug_frame in main is a possibility, but no one else does it. > Well, apparently some ship the full debug build always to achieve some > of the same things. I think this also achieves the same divergence from > the normal expectation. We have a lot of divergence from "normal expectations" when normal expectations come from questionable things GNU projects pushed. > As small side note, the -fno-*unwind-tables flags are also in "tryflags" > so if compiler would not support it, you don't get it. So depending on > compiler you might still get .eh_frame ;) In practice I'm not aware of any that support generation at all but don't honor it. However, more to the point, C is a language with undefined behavior. There is no way to enforce that the tooling prevent applications from doing broken things. Rather there's only avoiding explicitly doing things to give the impression that something works when it's not intended to. "It's a fluke and that happened to work for your particular environment only" is very different from "we actively made a change to make that work". The latter comes with some implied obligation to support it. For example if we changed freeaddrinfo to support freeing a "null sublist" (which we arguably should; see past discussion), that would entail an obligation not to revert it. But if the null pointer deref present now just happened not to crash on some arch, or if we just made a random change where the deref no longer happened in the code path, that would not entail an obligation to keep it "working". > The .eh_frame is a ELF abi feature normally turned on, and usually > needs good reasons to turn it off. Could you Rich please explain in > detail all the reasons why you think it's so evil? Mostly I heard > "feature X is evil and people should not use it" and I find that > your personal opinion, not a valid technical reason. But listening > more and earlier things, I've heard: > > 1. Having it makes it seem an application can jump through C library > from e.g. qsort callback. Not having .eh_frame makes C++ exception > fail, but does not help with e.g. setjmp/longjmp. So this is not > complete solution. The same here. You seem to think this is an effort to make technical measures to "block" doing something undefined. That's fundamentally impossible, at least without a very different type of C implementation. > 2. Explicitly disabling backtrace() universally. I assume this is to > disable to widespread usage of it from SEGV handler. But there > seem to be also implementations reading .debug_frame, so this does > not really fix that either fully. And I think there are valid use > cases for backtrace() also, e.g. when having debug build and having > internal debug printing trigger it. *If* this is the intent, that's more of a reason to have it load debug packages to operate: so it's explicit that it's a debugging-only operation and doesn't belong in production, and that you're using features outside of what the implementation provides/supports. > 3. Adds little bit if size to the binary. And also the runtime memory > usage as it goes to LOAD segment. But 80kB is not a concern on > modern environment even on embedded platforms. There are embedded platforms with only 8MB or so, half or more consumed by kernel, and entire fs in initramfs. I don't think this is the most significant reason not to have it, but you're coming from a very "large system" perspective here. > 4. You want to still enforce this so people don't do 1 or 2. > For this, time has taught me, people need education, not enforcement. > If you don't teach them, they'll still go and do the same stupid > things even more stupid ways. > > Please add in any reasons I may have missed. I would like to have your > complete list of reasons why to remove .eh_frame. So far it has been > coming out in pieces. I'd like constructive discussion if some of these > items could be implemented stronger in other means than removing > ..eh_frame. You're coming at it from the wrong direction. For new, nonstandard functionality requests, musl has a well established process of criteria for inclusion and exclusion, based on invasiveness (this is not just a matter of code change size, but of constraints it imposes), size, how often the lack of the functionality impacts portable or important FOSS programs users want to run on musl, and whether there are other ways the applications that want it could achieve what they want. In this case, all of those criteria indicate against doing it. > But fundamentally I think if we ship .debug_frame, all people wanting > do backtrace() and the silly stuff, will just enable .debug_frame > support in their code and still do the silly stuff in worse way, than > if .eh_frame was enabled. Since technically they are the same, even if > the intended use case is different. As seen libunwind does have > --enable-debug-frame. They might, but then they're clearly using a debugging interface that only works when debug files are available (or if you include this in main libc.so, when libc.so is not stripped). > For me it's quite unusual to have .debug_frame but not other debug > sections. I assume it works. But the tooling certain does not endorse > this setup. I suppose it can be achieved with some objcopy magic, but > requires care. If tooling doesn't like .debug_frame in main library but additional debug info split out, one way to do this would be providing separate versions of the -dbg package, one that's .debug_frame only and one that's detailed. I like this because there's an option not to install it at all and it's clear that this is debug functionality. But either way should be ok. Rich ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [musl] option to enable eh_frame 2021-07-17 13:25 ` Rich Felker @ 2021-07-17 15:40 ` Timo Teras 2021-07-17 16:09 ` Rich Felker 0 siblings, 1 reply; 21+ messages in thread From: Timo Teras @ 2021-07-17 15:40 UTC (permalink / raw) To: Rich Felker; +Cc: musl On Sat, 17 Jul 2021 09:25:44 -0400 Rich Felker <dalias@libc.org> wrote: > > Please add in any reasons I may have missed. I would like to have > > your complete list of reasons why to remove .eh_frame. So far it > > has been coming out in pieces. I'd like constructive discussion if > > some of these items could be implemented stronger in other means > > than removing ..eh_frame. > > You're coming at it from the wrong direction. For new, nonstandard > functionality requests, musl has a well established process of > criteria for inclusion and exclusion, based on invasiveness (this is > not just a matter of code change size, but of constraints it imposes), > size, how often the lack of the functionality impacts portable or > important FOSS programs users want to run on musl, and whether there > are other ways the applications that want it could achieve what they > want. In this case, all of those criteria indicate against doing it. Just to be on record, musl used to include .eh_frame until 2012 and commit b439c051c7eee4eb4b93fc382f993aa6305ce530 or musl 0.9.5. So back then existing "contract" was broken. If this discussion was done then, this would be the other angle: why to break something that is working. > > But fundamentally I think if we ship .debug_frame, all people > > wanting do backtrace() and the silly stuff, will just enable > > .debug_frame support in their code and still do the silly stuff in > > worse way, than if .eh_frame was enabled. Since technically they > > are the same, even if the intended use case is different. As seen > > libunwind does have --enable-debug-frame. > > They might, but then they're clearly using a debugging interface that > only works when debug files are available (or if you include this in > main libc.so, when libc.so is not stripped). But how is this different from the "contract" viewpoint? You are agreeable to have .debug_frame by default in the main DSO. And if we make musl have .debug_frame, and build libunwind with --enable-debug-frame, the user gets quite similar "contract" experience as with .eh_frame. But just worse than having .eh_frame, because for the unwinder to be able to use .debug_frame it needs to do more silly things. Users don't care if it's .debug_frame or .eh_frame as long as the higher level functionality works. And it gives same "de facto contract" experience. Sorry for being "stupid" here, but I'm trying to understand your viewpoint. So far it's sounding like: - same things can be achieved by doing X or Y - you argue X is evil and breaking contract, but Y is ok - on system/user level the "contract" or "experience" is same regardless of doing X or Y Basically I'm trying to understand why do you consider .eh_frame so much more important "contract" than .debug_frame? Timo ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [musl] option to enable eh_frame 2021-07-17 15:40 ` Timo Teras @ 2021-07-17 16:09 ` Rich Felker 2021-07-17 19:52 ` Ariadne Conill 0 siblings, 1 reply; 21+ messages in thread From: Rich Felker @ 2021-07-17 16:09 UTC (permalink / raw) To: Timo Teras; +Cc: musl On Sat, Jul 17, 2021 at 06:40:10PM +0300, Timo Teras wrote: > On Sat, 17 Jul 2021 09:25:44 -0400 > Rich Felker <dalias@libc.org> wrote: > > > > Please add in any reasons I may have missed. I would like to have > > > your complete list of reasons why to remove .eh_frame. So far it > > > has been coming out in pieces. I'd like constructive discussion if > > > some of these items could be implemented stronger in other means > > > than removing ..eh_frame. > > > > You're coming at it from the wrong direction. For new, nonstandard > > functionality requests, musl has a well established process of > > criteria for inclusion and exclusion, based on invasiveness (this is > > not just a matter of code change size, but of constraints it imposes), > > size, how often the lack of the functionality impacts portable or > > important FOSS programs users want to run on musl, and whether there > > are other ways the applications that want it could achieve what they > > want. In this case, all of those criteria indicate against doing it. > > Just to be on record, musl used to include .eh_frame until 2012 and > commit b439c051c7eee4eb4b93fc382f993aa6305ce530 or musl 0.9.5. So back > then existing "contract" was broken. If this discussion was done then, > this would be the other angle: why to break something that is working. You're still missing the point. Having it there when it just happened to be (and likewise on archs where there's some weird non-DWARF unwind table that we haven't opted out of) is not a contract to support it; it's an artifact of the toolchain. *Explicitly making a change for the purpose of adding it*, with no other plausible purpose, is such a contract. By the way, I think you're slightly wrong on the history. Prior to that commit, unwind info was already omitted unless debugging was enabled; this was because I was not aware that GCC would emit it in .debug_frame if needed. > > > But fundamentally I think if we ship .debug_frame, all people > > > wanting do backtrace() and the silly stuff, will just enable > > > .debug_frame support in their code and still do the silly stuff in > > > worse way, than if .eh_frame was enabled. Since technically they > > > are the same, even if the intended use case is different. As seen > > > libunwind does have --enable-debug-frame. > > > > They might, but then they're clearly using a debugging interface that > > only works when debug files are available (or if you include this in > > main libc.so, when libc.so is not stripped). > > But how is this different from the "contract" viewpoint? > > You are agreeable to have .debug_frame by default in the main DSO. And > if we make musl have .debug_frame, and build libunwind with > --enable-debug-frame, the user gets quite similar "contract" experience > as with .eh_frame. But just worse than having .eh_frame, because for the > unwinder to be able to use .debug_frame it needs to do more silly > things. It needs to do "silly" things which explicitly break if debug info is stripped, making it clear that this is not functionality of the libc but (strippable) debugging metadata that can't be relied on. > Users don't care if it's .debug_frame or .eh_frame as long as the higher > level functionality works. And it gives same "de facto contract" > experience. > > Sorry for being "stupid" here, but I'm trying to understand your > viewpoint. So far it's sounding like: > - same things can be achieved by doing X or Y > - you argue X is evil and breaking contract, but Y is ok > - on system/user level the "contract" or "experience" is same > regardless of doing X or Y > > Basically I'm trying to understand why do you consider .eh_frame so > much more important "contract" than .debug_frame? Because .eh_frame is part of the loaded program segment that ELF semantics define as being available to the runtime (which is what makes it non-strippable), and .debug_frame is not. Rich ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [musl] option to enable eh_frame 2021-07-17 16:09 ` Rich Felker @ 2021-07-17 19:52 ` Ariadne Conill 2021-07-17 23:56 ` Rich Felker 0 siblings, 1 reply; 21+ messages in thread From: Ariadne Conill @ 2021-07-17 19:52 UTC (permalink / raw) To: musl; +Cc: Timo Teras Hello, On Sat, 17 Jul 2021, Rich Felker wrote: > On Sat, Jul 17, 2021 at 06:40:10PM +0300, Timo Teras wrote: >> On Sat, 17 Jul 2021 09:25:44 -0400 >> Rich Felker <dalias@libc.org> wrote: >> >>>> Please add in any reasons I may have missed. I would like to have >>>> your complete list of reasons why to remove .eh_frame. So far it >>>> has been coming out in pieces. I'd like constructive discussion if >>>> some of these items could be implemented stronger in other means >>>> than removing ..eh_frame. >>> >>> You're coming at it from the wrong direction. For new, nonstandard >>> functionality requests, musl has a well established process of >>> criteria for inclusion and exclusion, based on invasiveness (this is >>> not just a matter of code change size, but of constraints it imposes), >>> size, how often the lack of the functionality impacts portable or >>> important FOSS programs users want to run on musl, and whether there >>> are other ways the applications that want it could achieve what they >>> want. In this case, all of those criteria indicate against doing it. >> >> Just to be on record, musl used to include .eh_frame until 2012 and >> commit b439c051c7eee4eb4b93fc382f993aa6305ce530 or musl 0.9.5. So back >> then existing "contract" was broken. If this discussion was done then, >> this would be the other angle: why to break something that is working. > > You're still missing the point. Having it there when it just happened > to be (and likewise on archs where there's some weird non-DWARF unwind > table that we haven't opted out of) is not a contract to support it; > it's an artifact of the toolchain. *Explicitly making a change for the > purpose of adding it*, with no other plausible purpose, is such a > contract. > > By the way, I think you're slightly wrong on the history. Prior to > that commit, unwind info was already omitted unless debugging was > enabled; this was because I was not aware that GCC would emit it in > .debug_frame if needed. > >>>> But fundamentally I think if we ship .debug_frame, all people >>>> wanting do backtrace() and the silly stuff, will just enable >>>> .debug_frame support in their code and still do the silly stuff in >>>> worse way, than if .eh_frame was enabled. Since technically they >>>> are the same, even if the intended use case is different. As seen >>>> libunwind does have --enable-debug-frame. >>> >>> They might, but then they're clearly using a debugging interface that >>> only works when debug files are available (or if you include this in >>> main libc.so, when libc.so is not stripped). >> >> But how is this different from the "contract" viewpoint? >> >> You are agreeable to have .debug_frame by default in the main DSO. And >> if we make musl have .debug_frame, and build libunwind with >> --enable-debug-frame, the user gets quite similar "contract" experience >> as with .eh_frame. But just worse than having .eh_frame, because for the >> unwinder to be able to use .debug_frame it needs to do more silly >> things. > > It needs to do "silly" things which explicitly break if debug info is > stripped, making it clear that this is not functionality of the libc > but (strippable) debugging metadata that can't be relied on. > >> Users don't care if it's .debug_frame or .eh_frame as long as the higher >> level functionality works. And it gives same "de facto contract" >> experience. >> >> Sorry for being "stupid" here, but I'm trying to understand your >> viewpoint. So far it's sounding like: >> - same things can be achieved by doing X or Y >> - you argue X is evil and breaking contract, but Y is ok >> - on system/user level the "contract" or "experience" is same >> regardless of doing X or Y >> >> Basically I'm trying to understand why do you consider .eh_frame so >> much more important "contract" than .debug_frame? > > Because .eh_frame is part of the loaded program segment that ELF > semantics define as being available to the runtime (which is what > makes it non-strippable), and .debug_frame is not. In this case, wouldn't we want to emit .eh_frame for libc.so? If not, why? If it's supposed to be available, that seems like a more compelling argument for including it, not excluding it. Ariadne ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [musl] option to enable eh_frame 2021-07-17 19:52 ` Ariadne Conill @ 2021-07-17 23:56 ` Rich Felker 2021-07-18 1:40 ` Ariadne Conill 2021-07-18 6:09 ` Timo Teras 0 siblings, 2 replies; 21+ messages in thread From: Rich Felker @ 2021-07-17 23:56 UTC (permalink / raw) To: Ariadne Conill; +Cc: musl, Timo Teras On Sat, Jul 17, 2021 at 02:52:48PM -0500, Ariadne Conill wrote: > Hello, > > On Sat, 17 Jul 2021, Rich Felker wrote: > > >On Sat, Jul 17, 2021 at 06:40:10PM +0300, Timo Teras wrote: > >>On Sat, 17 Jul 2021 09:25:44 -0400 > >>Rich Felker <dalias@libc.org> wrote: > >> > >>>>Please add in any reasons I may have missed. I would like to have > >>>>your complete list of reasons why to remove .eh_frame. So far it > >>>>has been coming out in pieces. I'd like constructive discussion if > >>>>some of these items could be implemented stronger in other means > >>>>than removing ..eh_frame. > >>> > >>>You're coming at it from the wrong direction. For new, nonstandard > >>>functionality requests, musl has a well established process of > >>>criteria for inclusion and exclusion, based on invasiveness (this is > >>>not just a matter of code change size, but of constraints it imposes), > >>>size, how often the lack of the functionality impacts portable or > >>>important FOSS programs users want to run on musl, and whether there > >>>are other ways the applications that want it could achieve what they > >>>want. In this case, all of those criteria indicate against doing it. > >> > >>Just to be on record, musl used to include .eh_frame until 2012 and > >>commit b439c051c7eee4eb4b93fc382f993aa6305ce530 or musl 0.9.5. So back > >>then existing "contract" was broken. If this discussion was done then, > >>this would be the other angle: why to break something that is working. > > > >You're still missing the point. Having it there when it just happened > >to be (and likewise on archs where there's some weird non-DWARF unwind > >table that we haven't opted out of) is not a contract to support it; > >it's an artifact of the toolchain. *Explicitly making a change for the > >purpose of adding it*, with no other plausible purpose, is such a > >contract. > > > >By the way, I think you're slightly wrong on the history. Prior to > >that commit, unwind info was already omitted unless debugging was > >enabled; this was because I was not aware that GCC would emit it in > >.debug_frame if needed. > > > >>>>But fundamentally I think if we ship .debug_frame, all people > >>>>wanting do backtrace() and the silly stuff, will just enable > >>>>.debug_frame support in their code and still do the silly stuff in > >>>>worse way, than if .eh_frame was enabled. Since technically they > >>>>are the same, even if the intended use case is different. As seen > >>>>libunwind does have --enable-debug-frame. > >>> > >>>They might, but then they're clearly using a debugging interface that > >>>only works when debug files are available (or if you include this in > >>>main libc.so, when libc.so is not stripped). > >> > >>But how is this different from the "contract" viewpoint? > >> > >>You are agreeable to have .debug_frame by default in the main DSO. And > >>if we make musl have .debug_frame, and build libunwind with > >>--enable-debug-frame, the user gets quite similar "contract" experience > >>as with .eh_frame. But just worse than having .eh_frame, because for the > >>unwinder to be able to use .debug_frame it needs to do more silly > >>things. > > > >It needs to do "silly" things which explicitly break if debug info is > >stripped, making it clear that this is not functionality of the libc > >but (strippable) debugging metadata that can't be relied on. > > > >>Users don't care if it's .debug_frame or .eh_frame as long as the higher > >>level functionality works. And it gives same "de facto contract" > >>experience. > >> > >>Sorry for being "stupid" here, but I'm trying to understand your > >>viewpoint. So far it's sounding like: > >> - same things can be achieved by doing X or Y > >> - you argue X is evil and breaking contract, but Y is ok > >> - on system/user level the "contract" or "experience" is same > >> regardless of doing X or Y > >> > >>Basically I'm trying to understand why do you consider .eh_frame so > >>much more important "contract" than .debug_frame? > > > >Because .eh_frame is part of the loaded program segment that ELF > >semantics define as being available to the runtime (which is what > >makes it non-strippable), and .debug_frame is not. > > In this case, wouldn't we want to emit .eh_frame for libc.so? If > not, why? If it's supposed to be available, that seems like a more > compelling argument for including it, not excluding it. I'm not sure who "we" is here and what perspective it's supposed to be from. I thought I'd explained the difference. Having something as part of the loadable program segment means it's always available, not strippable or missing if debug packages are not installed. Having it be in a non-load section (not segment) means it's metadata about the binary for tooling (e.g. debugger) usage, to be accessed by reading the file(s) not runtime data structures mapped in memory. From my perspective, the reason for it to be the latter is to reflect that this is debug info available only in a debug environment, not part of musl's runtime interfaces. Rich ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [musl] option to enable eh_frame 2021-07-17 23:56 ` Rich Felker @ 2021-07-18 1:40 ` Ariadne Conill 2021-07-18 2:23 ` Ariadne Conill 2021-07-18 6:09 ` Timo Teras 1 sibling, 1 reply; 21+ messages in thread From: Ariadne Conill @ 2021-07-18 1:40 UTC (permalink / raw) To: musl; +Cc: Ariadne Conill, Timo Teras Hello, On Sat, 17 Jul 2021, Rich Felker wrote: > On Sat, Jul 17, 2021 at 02:52:48PM -0500, Ariadne Conill wrote: >> Hello, >> >> On Sat, 17 Jul 2021, Rich Felker wrote: >> >>> On Sat, Jul 17, 2021 at 06:40:10PM +0300, Timo Teras wrote: >>>> On Sat, 17 Jul 2021 09:25:44 -0400 >>>> Rich Felker <dalias@libc.org> wrote: >>>> >>>>>> Please add in any reasons I may have missed. I would like to have >>>>>> your complete list of reasons why to remove .eh_frame. So far it >>>>>> has been coming out in pieces. I'd like constructive discussion if >>>>>> some of these items could be implemented stronger in other means >>>>>> than removing ..eh_frame. >>>>> >>>>> You're coming at it from the wrong direction. For new, nonstandard >>>>> functionality requests, musl has a well established process of >>>>> criteria for inclusion and exclusion, based on invasiveness (this is >>>>> not just a matter of code change size, but of constraints it imposes), >>>>> size, how often the lack of the functionality impacts portable or >>>>> important FOSS programs users want to run on musl, and whether there >>>>> are other ways the applications that want it could achieve what they >>>>> want. In this case, all of those criteria indicate against doing it. >>>> >>>> Just to be on record, musl used to include .eh_frame until 2012 and >>>> commit b439c051c7eee4eb4b93fc382f993aa6305ce530 or musl 0.9.5. So back >>>> then existing "contract" was broken. If this discussion was done then, >>>> this would be the other angle: why to break something that is working. >>> >>> You're still missing the point. Having it there when it just happened >>> to be (and likewise on archs where there's some weird non-DWARF unwind >>> table that we haven't opted out of) is not a contract to support it; >>> it's an artifact of the toolchain. *Explicitly making a change for the >>> purpose of adding it*, with no other plausible purpose, is such a >>> contract. >>> >>> By the way, I think you're slightly wrong on the history. Prior to >>> that commit, unwind info was already omitted unless debugging was >>> enabled; this was because I was not aware that GCC would emit it in >>> .debug_frame if needed. >>> >>>>>> But fundamentally I think if we ship .debug_frame, all people >>>>>> wanting do backtrace() and the silly stuff, will just enable >>>>>> .debug_frame support in their code and still do the silly stuff in >>>>>> worse way, than if .eh_frame was enabled. Since technically they >>>>>> are the same, even if the intended use case is different. As seen >>>>>> libunwind does have --enable-debug-frame. >>>>> >>>>> They might, but then they're clearly using a debugging interface that >>>>> only works when debug files are available (or if you include this in >>>>> main libc.so, when libc.so is not stripped). >>>> >>>> But how is this different from the "contract" viewpoint? >>>> >>>> You are agreeable to have .debug_frame by default in the main DSO. And >>>> if we make musl have .debug_frame, and build libunwind with >>>> --enable-debug-frame, the user gets quite similar "contract" experience >>>> as with .eh_frame. But just worse than having .eh_frame, because for the >>>> unwinder to be able to use .debug_frame it needs to do more silly >>>> things. >>> >>> It needs to do "silly" things which explicitly break if debug info is >>> stripped, making it clear that this is not functionality of the libc >>> but (strippable) debugging metadata that can't be relied on. >>> >>>> Users don't care if it's .debug_frame or .eh_frame as long as the higher >>>> level functionality works. And it gives same "de facto contract" >>>> experience. >>>> >>>> Sorry for being "stupid" here, but I'm trying to understand your >>>> viewpoint. So far it's sounding like: >>>> - same things can be achieved by doing X or Y >>>> - you argue X is evil and breaking contract, but Y is ok >>>> - on system/user level the "contract" or "experience" is same >>>> regardless of doing X or Y >>>> >>>> Basically I'm trying to understand why do you consider .eh_frame so >>>> much more important "contract" than .debug_frame? >>> >>> Because .eh_frame is part of the loaded program segment that ELF >>> semantics define as being available to the runtime (which is what >>> makes it non-strippable), and .debug_frame is not. >> >> In this case, wouldn't we want to emit .eh_frame for libc.so? If >> not, why? If it's supposed to be available, that seems like a more >> compelling argument for including it, not excluding it. > > I'm not sure who "we" is here and what perspective it's supposed to be > from. "We" being in the general sense: if there are tools expecting .eh_frame to be present, as part of the ELF specification, then would it not be desirable to have it? > I thought I'd explained the difference. Having something as part > of the loadable program segment means it's always available, not > strippable or missing if debug packages are not installed. Having it > be in a non-load section (not segment) means it's metadata about the > binary for tooling (e.g. debugger) usage, to be accessed by reading > the file(s) not runtime data structures mapped in memory. From my > perspective, the reason for it to be the latter is to reflect that > this is debug info available only in a debug environment, not part of > musl's runtime interfaces. This is not Alpine's position per se: we would like basic debugging to work in any environment. The splitting of debuginfo to -dbg packages is intended to reduce image size only. That decision is not considered by us to be a security-impacting decision, even if that is a side effect (and I personally consider it not to be a security feature at all, as an attacker can simply recreate their attack in a debug environment). Ultimately, the question is one of how to enable basic debugging features without having debuginfo present. Or in other words, we would like basic debugging information, but not necessarily DWARF sections as part of the runtime package. In my opinion -- the solution is to modify the strip functionality in abuild to include .debug_frame, and then adjust our libunwind and other programs to make use of .debug_frame. This gives us the basic debugging functionality, while also complying with the wishes of upstream musl. (As a side note, I really do not want to get into a position where Alpine is shipping a defacto fork of musl, we already did this with uClibc and it created more problems than solutions. Working with upstream implies a social contract that we will abide by upstream's wishes whenever reasonable, and I do not see any reason why preferring .debug_frame over .eh_frame to solve this problem is an unreasonable request.) Ariadne ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [musl] option to enable eh_frame 2021-07-18 1:40 ` Ariadne Conill @ 2021-07-18 2:23 ` Ariadne Conill 0 siblings, 0 replies; 21+ messages in thread From: Ariadne Conill @ 2021-07-18 2:23 UTC (permalink / raw) To: musl; +Cc: Ariadne Conill, Timo Teras Hello, On Sat, 17 Jul 2021, Ariadne Conill wrote: > Hello, > > On Sat, 17 Jul 2021, Rich Felker wrote: > >> On Sat, Jul 17, 2021 at 02:52:48PM -0500, Ariadne Conill wrote: >>> Hello, >>> >>> On Sat, 17 Jul 2021, Rich Felker wrote: >>> >>>> On Sat, Jul 17, 2021 at 06:40:10PM +0300, Timo Teras wrote: >>>>> On Sat, 17 Jul 2021 09:25:44 -0400 >>>>> Rich Felker <dalias@libc.org> wrote: >>>>> >>>>>>> Please add in any reasons I may have missed. I would like to have >>>>>>> your complete list of reasons why to remove .eh_frame. So far it >>>>>>> has been coming out in pieces. I'd like constructive discussion if >>>>>>> some of these items could be implemented stronger in other means >>>>>>> than removing ..eh_frame. >>>>>> >>>>>> You're coming at it from the wrong direction. For new, nonstandard >>>>>> functionality requests, musl has a well established process of >>>>>> criteria for inclusion and exclusion, based on invasiveness (this is >>>>>> not just a matter of code change size, but of constraints it imposes), >>>>>> size, how often the lack of the functionality impacts portable or >>>>>> important FOSS programs users want to run on musl, and whether there >>>>>> are other ways the applications that want it could achieve what they >>>>>> want. In this case, all of those criteria indicate against doing it. >>>>> >>>>> Just to be on record, musl used to include .eh_frame until 2012 and >>>>> commit b439c051c7eee4eb4b93fc382f993aa6305ce530 or musl 0.9.5. So back >>>>> then existing "contract" was broken. If this discussion was done then, >>>>> this would be the other angle: why to break something that is working. >>>> >>>> You're still missing the point. Having it there when it just happened >>>> to be (and likewise on archs where there's some weird non-DWARF unwind >>>> table that we haven't opted out of) is not a contract to support it; >>>> it's an artifact of the toolchain. *Explicitly making a change for the >>>> purpose of adding it*, with no other plausible purpose, is such a >>>> contract. >>>> >>>> By the way, I think you're slightly wrong on the history. Prior to >>>> that commit, unwind info was already omitted unless debugging was >>>> enabled; this was because I was not aware that GCC would emit it in >>>> .debug_frame if needed. >>>> >>>>>>> But fundamentally I think if we ship .debug_frame, all people >>>>>>> wanting do backtrace() and the silly stuff, will just enable >>>>>>> .debug_frame support in their code and still do the silly stuff in >>>>>>> worse way, than if .eh_frame was enabled. Since technically they >>>>>>> are the same, even if the intended use case is different. As seen >>>>>>> libunwind does have --enable-debug-frame. >>>>>> >>>>>> They might, but then they're clearly using a debugging interface that >>>>>> only works when debug files are available (or if you include this in >>>>>> main libc.so, when libc.so is not stripped). >>>>> >>>>> But how is this different from the "contract" viewpoint? >>>>> >>>>> You are agreeable to have .debug_frame by default in the main DSO. And >>>>> if we make musl have .debug_frame, and build libunwind with >>>>> --enable-debug-frame, the user gets quite similar "contract" experience >>>>> as with .eh_frame. But just worse than having .eh_frame, because for the >>>>> unwinder to be able to use .debug_frame it needs to do more silly >>>>> things. >>>> >>>> It needs to do "silly" things which explicitly break if debug info is >>>> stripped, making it clear that this is not functionality of the libc >>>> but (strippable) debugging metadata that can't be relied on. >>>> >>>>> Users don't care if it's .debug_frame or .eh_frame as long as the higher >>>>> level functionality works. And it gives same "de facto contract" >>>>> experience. >>>>> >>>>> Sorry for being "stupid" here, but I'm trying to understand your >>>>> viewpoint. So far it's sounding like: >>>>> - same things can be achieved by doing X or Y >>>>> - you argue X is evil and breaking contract, but Y is ok >>>>> - on system/user level the "contract" or "experience" is same >>>>> regardless of doing X or Y >>>>> >>>>> Basically I'm trying to understand why do you consider .eh_frame so >>>>> much more important "contract" than .debug_frame? >>>> >>>> Because .eh_frame is part of the loaded program segment that ELF >>>> semantics define as being available to the runtime (which is what >>>> makes it non-strippable), and .debug_frame is not. >>> >>> In this case, wouldn't we want to emit .eh_frame for libc.so? If >>> not, why? If it's supposed to be available, that seems like a more >>> compelling argument for including it, not excluding it. >> >> I'm not sure who "we" is here and what perspective it's supposed to be >> from. > > "We" being in the general sense: if there are tools expecting .eh_frame to be > present, as part of the ELF specification, then would it not be desirable to > have it? > >> I thought I'd explained the difference. Having something as part >> of the loadable program segment means it's always available, not >> strippable or missing if debug packages are not installed. Having it >> be in a non-load section (not segment) means it's metadata about the >> binary for tooling (e.g. debugger) usage, to be accessed by reading >> the file(s) not runtime data structures mapped in memory. From my >> perspective, the reason for it to be the latter is to reflect that >> this is debug info available only in a debug environment, not part of >> musl's runtime interfaces. > > This is not Alpine's position per se: we would like basic debugging to work > in any environment. The splitting of debuginfo to -dbg packages is intended > to reduce image size only. That decision is not considered by us to be a > security-impacting decision, even if that is a side effect (and I personally > consider it not to be a security feature at all, as an attacker can simply > recreate their attack in a debug environment). > > Ultimately, the question is one of how to enable basic debugging features > without having debuginfo present. Or in other words, we would like basic > debugging information, but not necessarily DWARF sections as part of the > runtime package. > > In my opinion -- the solution is to modify the strip functionality in abuild > to include .debug_frame, and then adjust our libunwind and other programs to > make use of .debug_frame. This gives us the basic debugging functionality, > while also complying with the wishes of upstream musl. > > (As a side note, I really do not want to get into a position where Alpine is > shipping a defacto fork of musl, we already did this with uClibc and it > created more problems than solutions. Working with upstream implies a social > contract that we will abide by upstream's wishes whenever reasonable, and I > do not see any reason why preferring .debug_frame over .eh_frame to solve > this problem is an unreasonable request.) As another side note: this has been reverted in Alpine musl 1.2.2-r5. Let us work together on the musl list to find consensus on a solution, then push that solution out in musl 1.2.2-r6. Ariadne ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [musl] option to enable eh_frame 2021-07-17 23:56 ` Rich Felker 2021-07-18 1:40 ` Ariadne Conill @ 2021-07-18 6:09 ` Timo Teras 2021-07-18 7:20 ` Timo Teras 1 sibling, 1 reply; 21+ messages in thread From: Timo Teras @ 2021-07-18 6:09 UTC (permalink / raw) To: Rich Felker; +Cc: Ariadne Conill, musl On Sat, 17 Jul 2021 19:56:36 -0400 Rich Felker <dalias@libc.org> wrote: > Having something as part of the loadable program segment means it's > always available, not strippable or missing if debug packages are not > installed. Having it be in a non-load section (not segment) means > it's metadata about the binary for tooling (e.g. debugger) usage, to > be accessed by reading the file(s) not runtime data structures mapped > in memory. From my perspective, the reason for it to be the latter is > to reflect that this is debug info available only in a debug > environment, not part of musl's runtime interfaces. So basically if I understand correctly: 1. You want all musl libraries to have equal (or forward-going) expectation of what is included in the LOAD segments. 2. Even if users expect/rely on debug_frame it's acceptable because that is strippable and shows this is "optional" or "debug" feature. This does makes sense. However, I thought the primary case was the unwinding through C-library and backtrace(). And trying distro to force patch packages instead of having those work. Just noting this because .debug_frame might make us not notice some of the "badness". I do have some technical reasons why I'd prefer .eh_frame (getting it on core dumps). Fortunately I need these from development boxes where I can have custom build for internal use only. So if .debug_frame is needed for peace, then so be it. Though, I still hope to see the time and a way having .eh_frame would acceptable. Is there anything I could implement or do to make it acceptable? For me it's presence is not "contract" commitment because it's not ABI feature. It only affects the functions we have not promised to implement. As explained just keeping .debug_frame is similar "de facto contract" to distro users. The fact whether it's not strippable or not does not matter, what matters is what is being shipped by default. Timo ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [musl] option to enable eh_frame 2021-07-18 6:09 ` Timo Teras @ 2021-07-18 7:20 ` Timo Teras 2021-07-24 23:45 ` Fangrui Song 0 siblings, 1 reply; 21+ messages in thread From: Timo Teras @ 2021-07-18 7:20 UTC (permalink / raw) To: Rich Felker; +Cc: Ariadne Conill, musl On Sun, 18 Jul 2021 09:09:22 +0300 Timo Teras <timo.teras@iki.fi> wrote: > I do have some technical reasons why I'd prefer .eh_frame (getting it > on core dumps). Fortunately I need these from development boxes where > I can have custom build for internal use only. So if .debug_frame is > needed for peace, then so be it. I'm trying to implement this currently, and have two observations: 1) musl.so seems to have .eh_frame even with default options currently. GCC insists generating it for the PLT. Though otherwise it's empty. This is on gcc 10.3.1. 2) .debug_frame is 109944 bytes, while .eh_frame_hdr + .eh_frame is 83832. The size difference seems to be because of CIE entries not being merged for .debug_frame. Timo ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [musl] option to enable eh_frame 2021-07-18 7:20 ` Timo Teras @ 2021-07-24 23:45 ` Fangrui Song 0 siblings, 0 replies; 21+ messages in thread From: Fangrui Song @ 2021-07-24 23:45 UTC (permalink / raw) To: musl; +Cc: Rich Felker, Ariadne Conill On 2021-07-18, Timo Teras wrote: >On Sun, 18 Jul 2021 09:09:22 +0300 >Timo Teras <timo.teras@iki.fi> wrote: > >> I do have some technical reasons why I'd prefer .eh_frame (getting it >> on core dumps). Fortunately I need these from development boxes where >> I can have custom build for internal use only. So if .debug_frame is >> needed for peace, then so be it. > >I'm trying to implement this currently, and have two observations: > >1) musl.so seems to have .eh_frame even with default options currently. >GCC insists generating it for the PLT. Though otherwise it's empty. >This is on gcc 10.3.1. ld can generate .eh_frame pieces for PLT entries on x86. GNU ld and gold have implemented --ld-generated-unwind-info. It was proposed in 2011 in https://sourceware.org/bugzilla/show_bug.cgi?id=12570 At that time gdb could not unwind from the middle of a PLT entry. (It can do that now.) (ld.lld doesn't have the feature. Considering that only asynchronous unwinding requests in the PLT code sequence may benefit from the feature, this is of very low value. It adds complexity due to breaking phase ordering in a linker.) >2) .debug_frame is 109944 bytes, while .eh_frame_hdr + .eh_frame is >83832. The size difference seems to be because of CIE entries not being >merged for .debug_frame. Yep, ld can optimize .eh_frame_hdr + .eh_frame . .debug_frame is unoptimized. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [musl] option to enable eh_frame 2021-07-17 7:33 ` Timo Teras 2021-07-17 10:38 ` Timo Teras 2021-07-17 13:25 ` Rich Felker @ 2022-04-17 19:50 ` Fangrui Song 2 siblings, 0 replies; 21+ messages in thread From: Fangrui Song @ 2022-04-17 19:50 UTC (permalink / raw) To: musl; +Cc: Rich Felker, Timo Teras On Sat, Jul 17, 2021 at 12:33 AM Timo Teras <timo.teras@iki.fi> wrote: > > On Fri, 16 Jul 2021 11:57:35 -0400 > Rich Felker <dalias@libc.org> wrote: > > > On Fri, Jul 16, 2021 at 12:16:25PM +0300, Timo Teras wrote: > > > > The debugger already can do debugging/unwinding because it has access > > to the debug information (if you've installed it) and there is a > > clear, understood-by-users contract that this information is not an > > inherent part of the program but something optional for external > > debugging tools only. > > This apparently the fundamental difference in thinking. The debugging > information is often huge, and people don't want to install it on > production or embedded devices. It ships ton of other features which > are not needed. However, the unwind/backtrace feature does provide > significant value for error reports and profiling. > > > > > Arguments to have .eh_frame: > > > - It allows debugging things even if musl-dbg is not or cannot be > > > installed for some reason (e.g. gdb, valgrind), or is no longer > > > available > > > - libunwind/libexecinfo will start to work and give meaningful > > > backtraces > > > > This is explicitly a reason not to. backtrace() considered harmful. > > Please elaborate. I agree it's harmful from SEGV handler when program > might be targeted. But used in other situations where it is useful. Do > you have additional reasons why backtrace() is harmful? > > Do note that libunwind --enable-debug-frame is enabled default by > upstream package on ARM and aarch64, and this would work if the debug > package is installed. > > This gives inconsistent behaviour based on arch and whether -dbg is > installed. Developer might think the unwind stuff works, only to find > it breaks on different install, and make the application depend on -dbg. > > > > - Continuous kernel based profiling (e.g. perf record -g dwarf) > > > will work > > > > This already works if you have debug info. > > As said, it was desire to make it work without debug info. > > > > Given that the main arguments against are either making UB crash, or > > > not the best fix, and keeping eh_frame enables useful features to > > > work, I think it would make sense to allow enabling it. > > > > > > Please consider the the attached patch to make it a configure > > > option to enable keeping eh_frame (defaulting still to not keeping > > > it). > > > > You can solve this problem just as well for the things you want to > > have work by including the (part of) the debug info you want in the > > main libc.so binary: .debug_frame. Of course I can't stop Alpine from > > doing it in a different way locally, but I would strongly recommend > > you do that rather than making a contract that diverges from musl. > > The .debug_frame in main is a possibility, but no one else does it. > Well, apparently some ship the full debug build always to achieve some > of the same things. I think this also achieves the same divergence from > the normal expectation. > > As small side note, the -fno-*unwind-tables flags are also in "tryflags" > so if compiler would not support it, you don't get it. So depending on > compiler you might still get .eh_frame ;) > > The .eh_frame is a ELF abi feature normally turned on, and usually > needs good reasons to turn it off. Could you Rich please explain in > detail all the reasons why you think it's so evil? Mostly I heard > "feature X is evil and people should not use it" and I find that > your personal opinion, not a valid technical reason. But listening > more and earlier things, I've heard: > > 1. Having it makes it seem an application can jump through C library > from e.g. qsort callback. Not having .eh_frame makes C++ exception > fail, but does not help with e.g. setjmp/longjmp. So this is not > complete solution. > > 2. Explicitly disabling backtrace() universally. I assume this is to > disable to widespread usage of it from SEGV handler. But there > seem to be also implementations reading .debug_frame, so this does > not really fix that either fully. And I think there are valid use > cases for backtrace() also, e.g. when having debug build and having > internal debug printing trigger it. > > 3. Adds little bit if size to the binary. And also the runtime memory > usage as it goes to LOAD segment. But 80kB is not a concern on > modern environment even on embedded platforms. > > 4. You want to still enforce this so people don't do 1 or 2. > For this, time has taught me, people need education, not enforcement. > If you don't teach them, they'll still go and do the same stupid > things even more stupid ways. > > Please add in any reasons I may have missed. I would like to have your > complete list of reasons why to remove .eh_frame. So far it has been > coming out in pieces. I'd like constructive discussion if some of these > items could be implemented stronger in other means than removing > .eh_frame. > > Please also explain why you think the .eh_frame is sufficient even > though it does not cover various situations as explained above. > > Now it may have been little bit early for me to go ahead and merge the > change in Alpine before going through this discussion. The intention > was not upset anyone or be unilateral. This is why I made the PR and > waited for a good while for comments. Unfortunately, I did not find the > comments convincing, but granted I should have asked the above questions > before merging. > > Currently, I still think reasons to have .eh_frame than not are more > weighty - it enables valid use cases in expected way. But I'm also > willing to revert if there's stronger reasons to do it. Either > technical, or just keeping peace. So far the technical reasons do not > convince me, but keeping peace and good relationship is something I > definitely want. > > I personally did not see this as an ABI or a "contract" change, and this > is perhaps why I made the merge earlier than later. > > While the "contract" may *seem* somewhat different. It was still > mentioned in the commit message that the change was not made to make > C++ exception through C library work. The "contract promise" has not > changed; though, granted, the user perceived "de facto contract" has > changed in this case. > > As said, for me the primary reason is observability: being able to gdb > and get back traces, and continuous profiling work without debug info. > Those are precious things to have on system where debug info is simply > not available. As mentioned backtrace() working is a side effect, and I > think it has also valid use cases even if it's also often misused. > > But fundamentally I think if we ship .debug_frame, all people wanting > do backtrace() and the silly stuff, will just enable .debug_frame > support in their code and still do the silly stuff in worse way, than > if .eh_frame was enabled. Since technically they are the same, even if > the intended use case is different. As seen libunwind does have > --enable-debug-frame. > > For me it's quite unusual to have .debug_frame but not other debug > sections. I assume it works. But the tooling certain does not endorse > this setup. I suppose it can be achieved with some objcopy magic, but > requires care. > > Timo I know this is a very old thread but I'd like to share some findings on unwinding through musl functions. https://maskray.me/blog/2022-04-10-unwinding-through-signal-handler#musl-x86-64 My main finding is that nongnu libunwind does not recognize __restore_rt as a signal trampoline on x86-64. If -funwind-tables is used, __restore_rt may need some CFI instructions. linux perf seems to use libunwind. I am not familiar with the tool and so would like to hear how unwind tables improve linux perf usage on musl. An alternative to -funwind-tables is -fno-omit-frame-pointer. ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2022-04-17 19:51 UTC | newest] Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-07-16 9:16 [musl] option to enable eh_frame Timo Teras 2021-07-16 15:57 ` Rich Felker 2021-07-16 22:06 ` Ariadne Conill 2021-07-17 0:41 ` Rich Felker 2021-07-17 1:08 ` Ariadne Conill 2021-07-17 1:10 ` Érico Nogueira 2021-07-17 7:24 ` Laurent Bercot 2021-07-18 8:16 ` Timo Teras 2021-07-17 7:33 ` Timo Teras 2021-07-17 10:38 ` Timo Teras 2021-07-17 13:25 ` Rich Felker 2021-07-17 15:40 ` Timo Teras 2021-07-17 16:09 ` Rich Felker 2021-07-17 19:52 ` Ariadne Conill 2021-07-17 23:56 ` Rich Felker 2021-07-18 1:40 ` Ariadne Conill 2021-07-18 2:23 ` Ariadne Conill 2021-07-18 6:09 ` Timo Teras 2021-07-18 7:20 ` Timo Teras 2021-07-24 23:45 ` Fangrui Song 2022-04-17 19:50 ` Fangrui Song
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/musl/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).