unofficial mirror of libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Florian Weimer <fweimer@redhat.com>
To: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Cc: libc-alpha@sourceware.org
Subject: Re: [RFC] stdlib: Make atexit to not act as __cxa_atexit
Date: Wed, 03 Jul 2019 12:05:19 +0200	[thread overview]
Message-ID: <87wogztov4.fsf@oldenburg2.str.redhat.com> (raw)
In-Reply-To: <9a349438-34f9-4552-ed82-f3d8607c5b2c@linaro.org> (Adhemerval Zanella's message of "Tue, 2 Jul 2019 13:56:05 -0300")

* Adhemerval Zanella:

>>> We need __cxa_finalize because it is called by __do_global_dtors_aux
>>> from initfini callbacks.
>> 
>> (Note: The comment is about crtbeginS.o built from libgcc sources.)
>> 
>> If all versions of the ELF constructor in crtbeginS.o call
>> __cxa_finalize as their first action, then we can turn __cxa_finalize
>> into a NOP and have the dynamic linker perform the original actions of
>> __cxa_finalize directly, before running the ELF destructors for this
>> object.
>
> In fact I though about it and one issue I found is __cxa_finalize uses
> __dso_handle and it is a hidden symbol for each DSO.  So, at _dl_fini
> we will need to resolve __cxa_finalize from libc.so (or potentially
> other function that handles the exit handlers), then resolve the
> __dso_handle from the referenced object (with potentially a hack to
> lookup for local objects), and finally call __cxa_finalize with
> __dso_handler as argument. Not sure it is really a better design.

So … what I was implying but didn't say explicitly is that at one point,
we will have to map (the address of) __dso_handle to a link map using
_dl_find_dso_for_object.

I need to submit my patch to speed up _dl_find_dso_for_object
substantially.  Then it shouldn't be a problem at all to call it in
response to each call to atexit, and put new entries on both the global
linked list and the per-DSO list.

Not sure to what extend we'd need to avoid quadratic-time algorithms
here.

>>> Also making the loader itself handler atexit
>>> is not the best option IMHO: we will either to call a private symbol on 
>>> libc (which might require an additional lookup) or export the internal 
>>> data so loader can implement the callback issuing itself.
>> 
>> Anything would be better than this pointless indirection through libgcc.
>> We have accumlated lots of cruft in the crt* objects which serve no
>> purpose at all for current binaries, and eventually, we will have to
>> clean this up.
>> 
>> I think crtbegin*.o could completely go away if we turn __cxa_finalize
>> into a NOP on the glibc side.
>
> I think if __dso_handle was exported as a global symbol it could
> simplify the indirection by a bit.

I don't see how this is possible.  It would trigger interposition, and
we need a unique address for each DSO, to identify it.

Now the __dso_handle interface is fairly broken and probably predates
the tight integration of libc and the dynamic linker.  We could easily
cache something that allows to quickly locate the actual linkmap in a
variable link __dso_handle, but we do not.

> One idea is to change the order atexit handlers are handled slightly:
>
>   - atexit handlers are not registered through __cxa_atexit, a new
>     symbol __atexit adds a new entry as ef_at (similar to my proposal).
>
>   - __cxa_finalize is changed to handle ef_at similar to ef_cxa by
>     calling the function pointer and setting it as ef_free. dlclose
>     will issue atexit as expected.
>
>   - __run_exit_handlers, used on both exit and quick_exit, will issue
>     atexit handler (ef_at) first and then handle on_exit (ef_on), and
>     __cxa_atexit (ef_cxa).  This keeps the expected order or atexit
>     handlers, however the order regarding ef_cxa will change.
>
>   - It will required some changes internally to handle atexit and
>     __cxa_atexit differently regarding lock acquisition since before
>     ef_cxa handler we can't get newly added ef_at ones.

I think you also need to do something on dlclose to deallocate atexit
handlers that have run, to avoid a memory leak.  Setting them to ef_free
is not sufficient.

Thanks,
Florian

  reply	other threads:[~2019-07-03 10:05 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-01 20:59 [RFC] stdlib: Make atexit to not act as __cxa_atexit Adhemerval Zanella
2019-07-02  7:44 ` Florian Weimer
2019-07-02 13:12   ` Adhemerval Zanella
2019-07-02 13:24     ` Florian Weimer
2019-07-02 16:56       ` Adhemerval Zanella
2019-07-03 10:05         ` Florian Weimer [this message]
2019-07-03 19:22           ` Adhemerval Zanella
2019-07-08 11:16             ` Florian Weimer
2019-07-08 13:39               ` Adhemerval Zanella
2019-07-08 13:57                 ` Florian Weimer
2019-07-11 19:09                   ` Adhemerval Zanella

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=87wogztov4.fsf@oldenburg2.str.redhat.com \
    --to=fweimer@redhat.com \
    --cc=adhemerval.zanella@linaro.org \
    --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).