Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

glibc.zig: update libc_nonshared to match upstream #17521

Closed
wants to merge 1 commit into from

Conversation

rootbeer
Copy link
Contributor

The libc_nonshared.a archive should only contain stub routines that can be safely statically linked into an executable. These should generally forward to a runtime-provided, private, versioned glibc API.

I'm not entirely clear on how to find new dependencies that go in libc_nonshared. I'm not sure

TODO: Need to get all symbols removed from here added back as symbols in the libc.so archive.

The libc_nonshared.a archive should only contain stub routines that
can be safely statically linked into an executable.  These should
generally forward to a runtime-provided, private, versioned glibc
API.

TODO: Need to get all files removed from here added back as
symbols in the libc.so archive.
@andrewrk
Copy link
Member

Please take note of #17489 which just landed.

Zig supports every version of glibc, so if things were removed from libc_nonshared.a then the logic in glibc.zig needs to check the target glibc version number and then either omit it or include it.

libc.so is generated based on abilists consolidated by this tool. I believe that the libc.so being generated by zig is 100% correct for all versions of glibc, for all architectures. Please do share a counterexample if you have one.

@rootbeer
Copy link
Contributor Author

As I understand things, the current version of Zig's libc_nonshared.a includes code that it should not (see #17034). And removing that code (specifically the fstat implementations) from libc_nonshared.a results in symbol-not-found errors, because the libc.so doesn't contain the required symbols:

$ ../zig/build/stage4/bin/zig build-exe confused-tls.zig -lc -target native-native-gnu
error: ld.lld: undefined symbol: fstatat64
    note: referenced by confused-tls.zig:16
    note:               confused-tls.o:(confused-tls.main)
error: ld.lld: undefined symbol: fstat64
    note: referenced by os.zig:4142 (/home/pat/projects/ziglang/zig/lib/std/os.zig:4142)
    note:               confused-tls.o:(os.fstat)

Oh ... maybe the problem is that glibc doesn't export fstatat64? Ah, no its a glibc v2.33 symbol, so I probably just need to sync forward to pick up your most recent changes to fix this.

If you want to support older versions of libc_nonshared.c in Zig, the library should not directly include the full implementation of the symbols. It needs to include the appropriate redirection implementation (e.g., like https://sourceware.org/git/?p=glibc.git;a=blob;f=io/fstatat64.c;hb=d614a7539657941a9201c236b2f15afac18e1213#l37). That redirects to a symbol that will get satisfied by the dynamically linked glibc implementation at runtime.

How do I chose which version of glibc to link against?

@andrewrk
Copy link
Member

If you want to support older versions of libc_nonshared.c in Zig, the library should not directly include the full implementation of the symbols. It needs to include the appropriate redirection implementation

fstatat.c and fstatat64.c will need to be patched to inspect the __GLIBC_MINOR__ preprocessor definition and either do the pre-2.33 behavior or post-2.33 behavior depending.

How do I chose which version of glibc to link against?

-target $ARCH-linux-gnu.$VERSION -lc where $ARCH is a CPU architecture and $VERSION is a glibc version such as "2.38".

@squeek502
Copy link
Collaborator

squeek502 commented Oct 15, 2023

Just noticed there's also #16970 which overlaps with this (and that PR might have the more complete fix).

@andrewrk
Copy link
Member

Closing in favor of #16970

@andrewrk andrewrk closed this Oct 15, 2023
@rootbeer
Copy link
Contributor Author

Yeah, #16970 looks like the complete fix to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants