-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
std.zig.system.NativeTargetInfo: improve glibc version and dynamic linker detection #12788
Conversation
25489c2
to
0d70c04
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Doesn't work on my Gentoo system :( rpath_offset is null |
@BratishkaErik thank you for checking. I will poke around on Gentoo in a Docker container and then push another commit for you to try shortly. |
// Empirically, glibc 2.34 libc.so .dynstr section is 32441 bytes on my system. | ||
// Here I use this value plus some headroom. This makes it only need | ||
// a single read syscall here. | ||
var buf: [40000]u8 = undefined; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like if we just wait for enough glibc releases this buffer will be overrun eventually, since new glibc versions will essentially only ever add new symbols
As a datapoint, .dynstr
on my system glibc 2.34 is 35062 bytes and 2.12 is 22098 bytes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair point, I will update this to account for any size.
Previously, this code would fail to detect glibc version because it relied on libc.so.6 being a symlink which revealed the answer. On modern distros, this is no longer the case. This new strategy finds the path to libc.so.6 from /usr/bin/env, then inspects the .dynstr section of libc.so.6, looking for symbols that start with "GLIBC_2.". It then parses those as semantic versions and takes the maximum value as the system-native glibc version. closes #6469 see #11137 closes #12567
Before, native glibc and dynamic linker detection attempted to use the executable's own binary if it was dynamically linked to answer both the C ABI question and the dynamic linker question. However, this could be problematic on a system that uses a RUNPATH for the compiler binary, locking it to an older glibc version, while system binaries such as /usr/bin/env use a newer glibc version. The problem is that libc.so.6 glibc version will match that of the system while the dynamic linker will match that of the compiler binary. Executables with these versions mismatching will fail to run. Therefore, this commit changes the logic to be the same regardless of whether the compiler binary is dynamically or statically linked. It inspects `/usr/bin/env` as an ELF file to find the answer to these questions, or if there is a shebang line, then it chases the referenced file recursively. If that does not provide the answer, then the function falls back to defaults. This commit also solves a TODO to remove an Allocator parameter to the detect() function.
This commit removes the check that takes advantage of when the dynamic linker is a symlink. Instead, it falls back on the same directory as the dynamic linker as a de facto runpath. Empirically, this gives correct results on Gentoo and NixOS. Unfortunately it is still falling short for Debian, which has libc.so.6 in a different directory as the dynamic linker.
This is a partial revert of the previous commit, fixing a regression on Debian. However, the commit additionally improves the detectAbiAndDynamicLinker function to read more than 1 byte at a time when detecting a shebang line.
0d70c04
to
9f40f34
Compare
@BratishkaErik I have pushed more commits that use an additional heuristic that solves Gentoo. It looks like unfortunately a future enhancement may require a full integration with |
Yes. it works fine! Thank you! pub const os = std.Target.Os{
.tag = .linux,
.version_range = .{ .linux = .{
.range = .{
.min = .{
.major = 5,
.minor = 19,
.patch = 6,
},
.max = .{
.major = 5,
.minor = 19,
.patch = 6,
},
},
.glibc = .{
.major = 2,
.minor = 35,
.patch = 0,
},
}},
}; |
I'm seeing bad results on Debian bookworm (testing) - glibc 2.34 but Zig reports 2.19 It looks like the latest glibc updates removed the versioned symlinks for Debian too, unfortunately |
After failing to find RUNPATH in the ELF of /usr/bin/env, not finding the answer in a symlink of the dynamic interpreter, and not finding libc.so.6 in the same directory as the dynamic interpreter, Zig will check `/lib/$triple`. This fixes incorrect native glibc version detected on Debian bookworm.
Alright another commit pushed, now Debian bookworm is handled. If anyone wants to test the logic without compiling anything, you can do this:
Any more distros not working still? |
Can confirm that this detects the correct glibc version for me and I can build the example in #11137 if I explicitly specify an earlier version that Zig can provide. |
The failure affects master branch. I will address it in master branch by adding more debug printing to help when the problem happens again. |
Previously, this code would fail to detect glibc version because it
relied on libc.so.6 being a symlink which revealed the answer. On modern
distro versions, this is no longer the case.
This new strategy finds the path to libc.so.6 from
/usr/bin/env
, theninspects the
.dynstr
section of libc.so.6, looking for symbols thatstart with "GLIBC_2.". It then parses those as semantic versions and
takes the maximum value as the system-native glibc version.
Before, native glibc and dynamic linker detection attempted to use the
executable's own binary if it was dynamically linked to answer both the
C ABI question and the dynamic linker question. However, this could be
problematic on a system that uses a RUNPATH for the compiler binary,
locking it to an older glibc version, while system binaries such as
/usr/bin/env use a newer glibc version. The problem is that libc.so.6
glibc version will match that of the system while the dynamic linker
will match that of the compiler binary. Executables with these versions
mismatching will fail to run.
Therefore, this pull request changes the logic to be the same regardless of
whether the compiler binary is dynamically or statically linked. It
inspects
/usr/bin/env
as an ELF file to find the answer to thesequestions, or if there is a shebang line, then it chases the referenced
file recursively. If that does not provide the answer, then the function
falls back to defaults.
This pull request also solves a TODO to remove an Allocator parameter to the
detect() function.
closes #6469
closes #12567
@Vexu can you check if this solves #11137 for you?