unofficial mirror of libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Florian Weimer <fweimer@redhat.com>
To: Carlos O'Donell <carlos@redhat.com>
Cc: libc-alpha@sourceware.org
Subject: Re: Removing longjmp error handling from the dynamic loader
Date: Mon, 11 Mar 2019 19:39:20 +0100	[thread overview]
Message-ID: <87va0ptfvr.fsf@oldenburg2.str.redhat.com> (raw)
In-Reply-To: <4f7706a4-b914-0ba5-84f6-352703a54e82@redhat.com> (Carlos O'Donell's message of "Mon, 11 Mar 2019 12:13:21 -0400")

* Carlos O'Donell:

> Right, you might be able to get away with a free-list here, but I get your
> point, in a fully generic system you must register a free handler which is
> specific code that knows how to free the object you allocated. I hate to say
> it but we're just designing C++ from the ground up again. Why not just get
> the loader to the point it can run C++? This is a serious question.

We would have to solve the bootstrapping issue.

I expect this topic (use of C++) to be rather controversial.

>>> Let's call this "Method 2: C exceptions"
>>>
>>> For dlopen-et-al I think a DWARF-based unwinder would be great.
>> 
>> It requires moving the unwinder implementation from libgcc_s to libc,
>> though.  The last time we discussed this (related to unwinder
>> performance issues and the dl_iterate_phdr interface), this idea was not
>> well-received.
>
> It might still be the best technical choice.
>
> I do not think we should discard this idea so easily.
>
> Why wouldn't we pursue this option?

Well, it turns this whole exercise from a one-day hack into a
longer-term integration project. 8-)

If this is the direction in which we want to go, it doesn't look
technically unsound to me, just very controversial.

>> If we fix the init/fini and IFUNC resolver bugs the way I intend, we
>> will never have to unwind through user code before hitting the final
>> handler, so there is no problem with the availability of unwinding data.
>> So at least that part is entirely solvable.
>
> I don't understand the exact bugs in question here, what are they?

These bugs are unrelated.  We currently have memory leaks if getaddrinfo
is canceled, for example.  Fixing that by compiling getaddrinfo with
-fexceptions and using RAII wouldn't solve them because getaddrinfo
might call into an NSS service module which is not compiled with
unwinding information.  Hence the need for the dual unwinding with
longjmp and the libgcc_s unwinder in libpthread's cancellation
implementation.  (We might get away with that if we install a setjmp
barrier before calling into an external NSS module, though.)

For the socket file descriptor opened by nss_dns/libresolv and some
other resources used there, we currently use a strategy like the free
list you mentioned (_res and struct resolv_context).  We need to perform
heap allocations for the resolver context because the thread cleanup
code runs only after fully unwinding the stack, so it won't be able to
use the information there.  (This is different in the SJLJ case in the
dynamic linker, where we can easily change what the code does before
calling longjmp.)  But these two mechanisms do not cover everything that
getaddrinfo allocates.

In short, this is an entirely different area, but it could benefit from
experience with other unwinding designs.

>>> I assume you envision that exception handling in C would help cleanup
>>> the code and allow more locally visible cleanups to happen (unwinding
>>> state).
>> 
>> It requires a vastly different coding style.  I have not thought much
>> about it.  I don't think we should rely on a relatively obscure GNU
>> extension so prominently.  Or put differently, if we want to use RAII as
>> an implementation technique, I don't think we should use C.
>
> This is a GNU project and we should do whatever we think is the best
> solution for glibc.
>
> If the obscure GNU extension has bugs, then that's a reason not to use
> it, or if the debugger doesn't support it, then that's a reason not to
> use it, but it can't just be about "obscure GNU extensions."

The extension itself (the cleanup attribute) is increasingly used.
systemd seems quite happy as a user, and there are others.

Combining it with DWARF unwinding is rare and probably restricted to
some limited scenarios related to cancellation.  Fedora and downstream
are compiled with -fexceptions, so there probably aren't any obvious
ICEs in that area.

> Alternatives are using Rust or C++.

Rust is not supposed to have exceptions.  It only supports automatic
error returns based on the return type (a specially annotated sum type).
There's also no GNU front end for it.

(In reality, Rust has exceptions; the test framework even requires them.
Last time I tried, you couldn't compile your code without stack
unwinding support if you wanted to test it.  By default, out-of-bounds
array access in Rust throws, and like in Go, there is just not very
convenient language support to handle the exception, but the support is
there in the run-time library.)

> I would prefer a limited form of C++ without libstdc++ support.

We would have to move the C++ personality routine and the core exception
handling to ld.so.  They live in libstdc++, not libgcc_s, and are not
part of the regular unwinder. 8-/

Maybe we could have a restricted, hidden implementation of C++ exception
support.  I have not put much thought to that.  I didn't expect this
turn of the discussion.

Note that RAII is helpful as an implementation strategy even without
exceptions.  (But it makes some uses of constructors rather cumbersome
because objects are not necessarily in a valid state right after
construction.  It would work fine for struct scratch_buffer and the
dynarrays, though.  Guess why.)

>>>> The alternative to unwinding is an explicit struct dl_exception *
>>>> argument for functions which can fail, and use the return value to
>>>> indicate whether there was a fatal error.  This sometimes causes issues
>>>> where the return value is already used to signal both success and
>>>> non-fatal error (e.g., -1 for failure from open_verify, or NULL for
>>>> RTLD_NOLOAD failure from _dl_map_object_from_fd).
>>>
>>> No, if we're going to change it should be *towards* something where the
>>> compiler can help us get it right.
>>>
>>> I don't want to see an explicit "this" passed into every function by hand.
>>>
>>> Let's call this "Method 3: Explicit this"
>> 
>> The explicit argument really helps to spot places where unwinding
>> happens.  For example, it makes it pretty clear that you have a memory
>> leak if you call a function with such an argument and an allocation is
>> not rooted in a global data structure.
>
> I agree, but is it the best option?
>
> Or are you arguing it's the best value for cost option?

I expect that we have to do this conversion anyway (I already did for
x86-64) to find out the spots which need careful handling in any of the
implicit approaches.

Maybe we could avoid that if we had exact call-graph-based tools, for
all relevant targets, but I don't think they exist.

> So to be clear, you're proposing:
>
> "Method 3: Explicit this"
>
> Which you argue has highest value, with lowest cost.

Yes, and it also has the best tool support, I think.

We could use the knowledge gained by the conversion to fix bugs, but I
think it will bit-rot over time, as people add more
architecture-specific code and support more features in the dynamic
linker.

> While I've been arguing for "Method 2: C exceptions" which you believe
> has too much cost given the value, since we'd have to:
> - move the unwinder to libc
> - switch to some C++ subset

This sounds tempting as well, but I'm not sure if it's the right place
to start with using C++ in glibc.

One thing we have not considered much so far is what you call “free
lists”, and generally not doing any unrecoverable actions until we reach
the point of no return in dlopen (and eliminating the possiblity of
failures in dlclose, by avoiding calling malloc there).  For example, we
could store the temporary file descriptors in the exception context (by
a callout to libc.so.6) and make sure that they are closed when
unwinding.  We can avoid memory leaks for temporary allocations in much
the same way.

In a sense, we would provide a more approachable programming environment
for unwinding.

Thanks,
Florian

  parent reply	other threads:[~2019-03-11 18:39 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-05 16:37 Removing longjmp error handling from the dynamic loader Florian Weimer
2019-03-06 15:40 ` Rich Felker
2019-03-07 14:03   ` Adhemerval Zanella
2019-03-11 13:45   ` Florian Weimer
2019-03-11 22:52     ` Rich Felker
2019-03-12 10:30       ` Florian Weimer
2019-03-12 12:00         ` Szabolcs Nagy
2019-03-11 14:07 ` Carlos O'Donell
2019-03-11 15:29   ` Florian Weimer
2019-03-11 16:13     ` Carlos O'Donell
2019-03-11 16:53       ` Zack Weinberg
2019-03-11 17:08         ` Florian Weimer
2019-03-11 18:39           ` Zack Weinberg
2019-03-11 18:49             ` Florian Weimer
2019-03-11 18:39       ` Florian Weimer [this message]
2019-03-11 19:30         ` Carlos O'Donell
2019-03-13 15:51           ` Florian Weimer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/libc/involved.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87va0ptfvr.fsf@oldenburg2.str.redhat.com \
    --to=fweimer@redhat.com \
    --cc=carlos@redhat.com \
    --cc=libc-alpha@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).