mailing list of musl libc
 help / color / mirror / Atom feed
* [musl] option to enable eh_frame
@ 2021-07-16  9:16 Timo Teras
  2021-07-16 15:57 ` Rich Felker
  0 siblings, 1 reply; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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
  2021-07-17 13:25     ` Rich Felker
  1 sibling, 2 replies; 20+ 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] 20+ 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
  1 sibling, 0 replies; 20+ 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] 20+ 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
  1 sibling, 1 reply; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ messages in thread

end of thread, other threads:[~2021-07-24 23:46 UTC | newest]

Thread overview: 20+ 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

mailing list of musl libc

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://inbox.vuxu.org/musl

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 musl musl/ http://inbox.vuxu.org/musl \
		musl@inbox.vuxu.org
	public-inbox-index musl

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.vuxu.org/vuxu.archive.musl


code repositories for the project(s) associated with this inbox:

	https://git.vuxu.org/mirror/musl/

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git