Skip to content

Conversation

@andrewrk
Copy link
Member

@andrewrk andrewrk commented Jan 19, 2025

This means doing more work in parallel which is already good, but it's also a correctnes fix because we need link_task_wait_group.wait() to ensure that no more linker inputs will be generated.

A side effect of this is that the Elf linker now receives link inputs in a nondeterministic order, and that seems to cause problems in particular with musl libc, so I need to troubleshoot that before landing this fix.

Addresses problem introduced by #22220.

@andrewrk
Copy link
Member Author

Alright I've narrowed this down to:

SEGV:

/home/andy/.cache/zig/o/8e5eca0f6246672df75a95f8055b59b8/crtn.o
/home/andy/dev/zig/.zig-cache/o/30ee3f1687b2d0eff37486889f34a848/hello.o
/home/andy/.cache/zig/o/5f7c52e0a7777f827964c9edc51450a1/crti.o
/home/andy/.cache/zig/o/2e6b69bc34a9e7a3779fcf6a193969a4/crt1.o
/home/andy/.cache/zig/o/dd015203d402123b36075d79d7bc4a22/libcompiler_rt.a
/home/andy/.cache/zig/o/ecf9a1d953e01c03655ddac8b404de89/libc.a

OK:

/home/andy/.cache/zig/o/5f7c52e0a7777f827964c9edc51450a1/crti.o
/home/andy/dev/zig/.zig-cache/o/30ee3f1687b2d0eff37486889f34a848/hello.o
/home/andy/.cache/zig/o/8e5eca0f6246672df75a95f8055b59b8/crtn.o
/home/andy/.cache/zig/o/2e6b69bc34a9e7a3779fcf6a193969a4/crt1.o
/home/andy/.cache/zig/o/dd015203d402123b36075d79d7bc4a22/libcompiler_rt.a
/home/andy/.cache/zig/o/ecf9a1d953e01c03655ddac8b404de89/libc.a

Clearly the difference here is the ordering of crti.o and crtn.o, as I suspected.

@andrewrk andrewrk added breaking Implementing this issue could cause existing code to no longer compile or have different behavior. linking release notes This PR should be mentioned in the release notes. labels Jan 21, 2025
@andrewrk
Copy link
Member Author

Technically breaking now since it drops support for legacy .ini/.fini mechanism. See the relevant commit for more details.

This means doing more work in parallel which is already good, but it's
also a correctnes fix because we need link_task_wait_group.wait() to
ensure that no more linker inputs will be generated.
Move all the remaining Jobs that produce linker inputs to be spawned
earlier in the pipeline and registered with link_task_wait_group.
crti.o/crtn.o is a legacy strategy for calling constructor functions
upon object loading that has been superseded by the
init_array/fini_array mechanism.

Zig code depends on neither, since the language intentionally has no way
to initialize data at runtime, but alas the Zig linker still must
support this feature since popular languages depend on it.

Anyway, the way it works is that crti.o has the machine code prelude of
two functions called _init and _fini, each in their own section with the
respective name. crtn.o has the machine code instructions comprising the
exitlude for each function. In between, objects use the .init and .fini
link section to populate the function body.

This function is then expected to be called upon object initialization
and deinitialization.

This mechanism is depended on by libc, for example musl and glibc, but
only for older ISAs. By the time the libcs gained support for newer
ISAs, they had moved on to the init_array/fini_array mechanism instead.

For the Zig linker, we are trying to move the linker towards
order-independent objects which is incompatible with the legacy
crti/crtn mechanism.

Therefore, this commit drops support entirely for crti/crtn mechanism,
which is necessary since the other commits in this branch make it
nondeterministic in which order the libc objects and the other link
inputs are sent to the linker.

The linker is still expected to produce a deterministic output, however,
by ignoring object input order for the purposes of symbol resolution.
Turns out that even modern Debian aarch64 glibc libc_nonshared.a has
references to _init, meaning that the previous commit caused a
regression when trying to build any -lc executable on that target.

This commit backs out the changes to LibCInstallation.

There is still a fork in the road coming up when the self-hosted ELF
linker becomes load bearing on that target.
@andrewrk andrewrk merged commit b31a2c9 into master Jan 21, 2025
10 checks passed
@andrewrk andrewrk deleted the pipeline branch January 21, 2025 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking Implementing this issue could cause existing code to no longer compile or have different behavior. linking release notes This PR should be mentioned in the release notes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants