Page MenuHomeFreeBSD

libc/csu: Unify INIT_RELOCS across architectures
ClosedPublic

Authored by jrtc27 on Oct 18 2024, 9:09 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Dec 5, 3:34 AM
Unknown Object (File)
Thu, Nov 28, 6:01 AM
Unknown Object (File)
Thu, Nov 21, 5:55 AM
Unknown Object (File)
Wed, Nov 20, 6:28 AM
Unknown Object (File)
Nov 15 2024, 1:43 PM
Unknown Object (File)
Nov 10 2024, 4:18 AM
Unknown Object (File)
Nov 7 2024, 8:16 AM
Unknown Object (File)
Nov 3 2024, 1:15 AM
Subscribers

Details

Summary

Some architectures don't need any arguments, whilst others need auxargs,
which they get by passing in env thanks to INIT_RELOCS referencing the
local variable in __libc_start1(_gcrt) by name. This is unnecessarily
confusing, fragile (one has to look at INIT_IRELOCS's definition to see
that it uses env) and duplicates code between architectures.

Instead, implement it more like rtld-elf. Each architecture provides an
ifunc_init that takes the auxargs directly, and those that don't need it
can just ignore it.

MFC after: 1 month

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 60077
Build 56961: arc lint + arc unit

Event Timeline

kib added inline comments.
lib/libc/csu/libc_start1.c
147

Now this code is compiled, and the calculation is done even on arches that do not need auxv for ifunc_init().

This revision is now accepted and ready to land.Oct 18 2024, 10:06 PM
lib/libc/csu/libc_start1.c
147

In theory everything's visible to the compiler so it should be able to optimise it (it's a loop with no side effects and a non-constant condition so is UB if it doesn't terminate). Weirdly, at least looking at aarch64, Clang does inline everything but leaves the loop in with the result unused. Toy examples on godbolt.org are able to optimise it so there's something special, perhaps just the size, about this file.

I would ultimately like to share this code with TLS by passing it into _init_tls as well, which would make it no longer matter, but don't know what the ABI implications are of changing that. (Can we assume you're not mixing libc.a and crt1.o versions? Hopefully? Also _init_tls is a versioned non-private symbol but in libc_private.h so presumably we don't have to care about *other* users of the API?)

lib/libc/csu/libc_start1.c
147

Might be, some additional application of const would help. E.g. the env arg to handle_irelocs can be const char *env[] and might be const char * const * env.

_init_tls is exported from user namespace FBSD_1.0, so changing its ABI would break the promise. You need to set it to other version.

Sure, we can assume that crt1.o and libc.a match at linking time, but we cannot assume that libc.so.7 matches crt1.o from binary at runtime.

lib/libc/csu/libc_start1.c
147

Ah, we compile the file as gnu99 but that bit of UB was new in C11.

And ok, that should be fine here, it's guarded by checking _DYNAMIC, so long as we're sure no dynamic binaries ever called it.

lib/libc/csu/libc_start1.c
147

It should be perfectly valid to compile all crt in c11 or newer mode. Perhaps as a second change?