-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
Avoid using fork() when probing system libstdc++
#60254
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
base: master
Are you sure you want to change the base?
Avoid using fork() when probing system libstdc++
#60254
Conversation
79f02b0 to
b668d14
Compare
535e217 to
6916b81
Compare
50164ec to
39f68a7
Compare
|
@gbaraldi pointed out that Nonetheless our $ nix-shell -p julia_111-bin
$ strace julia -e '' 2>&1 | grep libstdc++
openat(AT_FDCWD, "/nix/store/i3ibgfskl99qd8rslafbpaa1dmxdzh1z-glibc-2.40-66/lib/libstdc++.so.6", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/nix/store/16hvpw4b3r05girazh4rnwbw0jgjkb4l-xgcc-14.3.0-libgcc/lib/libstdc++.so.6", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/nix/store/gs785h7im3vchbdxalrrjnmcan4dsign-julia-bin-1.11.6/lib/julia/libstdc++.so.6", O_RDONLY|O_CLOEXEC) = 3so I think we can likely merge this as-is.
|
400adca to
47f1667
Compare
| **/ | ||
| const char *jl_loader_probe_system_library(const char *libname, const char *symbol) | ||
| { | ||
| char buf[PATH_MAX]; |
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.
PATH_MAX is much smaller than the maximum path length
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.
That's true, but exceeding PATH_MAX usually requires iterating path parts through openat etc. doesn't it?
I didn't think you could avoid ENAMETOOLONG if you try to open() a too-long string
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.
On some OS, it may be the variable {PATH_MAX} on some systems not the constant PATH_MAX (though on linux I suppose it is constant, and long enough): https://pubs.opengroup.org/onlinepubs/009696599/basedefs/limits.h.html, pathconf
8dd7854 to
f8ce0da
Compare
Rather than relying on the linker to do all of the heavy lifting, this changes our probing technique to directly parse the `ld.so.cache` and `libstdc++.so.6` files. As long as we can expect `ld.so.cache` to be available in `/etc` on all systems, this seems to be a reliable way to locate system libraries on Linux systems. That allows us to avoid `fork()`. For a small application that has just started up `fork()` is no big deal, but it's quite heavy-handed for Julia- as-a-library scenarios where resident memory may already be large. Many soft-embedded targets also do not support fork() well, so it is good for our compatibility to adjust this.
f8ce0da to
c23c90d
Compare
|
|
||
| static const char *search_ldcache(struct cache_file_new *cache, const char *libname, size_t *index) | ||
| { | ||
| if (strncmp(cache->magic, CACHEMAGIC_NEW, sizeof(CACHEMAGIC_NEW) - 1) != 0) |
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.
Since we support libc >= 2.17, I think we need to look for the new ld.so.cache format inside the old one, like this: https://elixir.bootlin.com/glibc/glibc-2.42/source/elf/dl-cache.c#L397
This is from a system with glibc 2.28:
$ head -c 11 /etc/ld.so.cache && echo
ld.so-1.7.0
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.
You found one! I tested every file from https://gitlab.com/piperswe/ld-so-cache/-/tree/main/tests/fixtures/generated and a few distros on my own and couldn't find an example of the hybrid or old formats
will add support now
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.
For reference, I have been testing things on ancient Debian (same as when I was debugging the libstdc++ issues): docker run --rm -it debian:10. Here is the ld.so.cache I grabbed from that container, if I helps with testing:
/Td6WFoAAATm1rRGAgAhARwAAAAQz1jM4CDyBehdADYZAcXB//59HL60BmhAMoX2DvJVsQ0Dkcl1
64MjmiP9hfcAiIr7DWf2tEzlJG2QtsR5nDweagzXPF1g7ZMUhDHcaB+Ld3G9Mas68IEnkNz/SDKS
GqakK/VeGkktkXeN+MZiY7Isj9wMy8TNTzSKBW+dEzMeFAYMec2P9P/O5dYd82ecopdUNa0fSK0Y
8AXaxexL7ql3dHP2zmduJZD7uhWOy4zwgT6tTiI3Gnn5HREVRfS5+HNCxaUcNqoHGjNTziH6oDNV
nMNoXlweTm5ZqPifww7OxFe9tDlEutMizsSvfvMYD4A/yvmPXFHmQMklclz0jWRJjYWZJYxzFrdF
7Rkc5z2cTroqBUd+Lz9qaCvgAIye27U/hrpiKHiosUyQMTGNPo59mS+OCfflarwotRi+we5LKWPQ
4q4bsi2nUH+4xIAtgJMjNiJgcbfe4J6By8uVpsLXS4OAAcMcCW114ZMJtwRmzI+jHeBzsyWsmG/3
TwO790LXMKDpeRCezrY1s2k+S+hTE/lokN21uFKsBK2KAVHbhZe+k0W2YZ/PAWrSE4BClF8oS0HB
i3cpRAsFJbKA1DaDkseB65U4Z6rR+lmI6z38jB2xrt128Y19grQG6uTeTRxGQbR45yxOpoDeJdma
pbuEsOiS0HL23XM4fbQmIWmzPn0Z+qajSD/6fSBYC/njt/iCGvR35DLm74OhaY9IRXyyzW9eTRA8
MJsDs4RCQfApUHy/JTg4OFeV1VKmSELfjJ3dxh9oPLXzsyD2m83oNFOrfAwdNwjfGFFBkxaEZbd7
95tP7IABmD1WR4ef4S4ZVmrw/9erczZsP8/LSRIJYPs85fr6+PU+GqqYSaCuCZxPHRk6LDHLaNJu
7kbH4Rqm2d2OCTbspfpAfltJgLysghbN25EEG5Jq6u0mQf7V/ocEL3xiGSf5SLHTJtwPUkRDTd/x
Up3e4FjSHVxxFxOxkEKnvV6wHK2wuf4AK6WXeBkd05LdPD7tGXomiKSaN1N0Yr1/uS/obzPgj2Xe
DMdWRW1DBykjthAHdNW2fW2xiX9RVh24+/2si5HPqJDLjDFpPgOsDbsB7vMQKTJI4B/DV5tRs5hs
R2TnmKYmCKktiO47Px/UGuKkdOwzCkB2+14Qi8PFFZGbN053c2MLBGXFp5RrVhdk7QG/eHCPXwBY
62n/JTMN3PxBgN+XVF0us8LtOl/jAlA9cMOpV2BL8Ih2daahjRIwHC3AF82i18WqKb152uv/L3Rv
TcRoc9cpXw/zUlgiX0ioycMuEJ3v50CuFbBFNLQTNJQREXXBMXI9A45/geqR3DJsUdJosxrmG2Wa
Gu4F+BMu2RnxF7JfKNjQ1ycrjVe5YGf6pz0DYl+r/DoC22mC+btb33pEM/UxGJz2RzNqwuJBPFqa
oZWn2vsKsnQwmvjU70YZ7tX0WXfVu9M7Zk6qWGmaMJtV8ywLQVKW+c+aTaOR8yTIUKUrA2InG4Ff
rYkep22BBhPQNaviHOuOtUzTL1fhydYsnPGWdGcQ/CA8q9imspqYipJUzbd+NRKRqiUcmof3pBeQ
1/LQc8Sjxxxls+kOIMUOmBU2721awexfv8yf0v20v9yE6TmoidZjGt8UXltly23zqsnYKci142K5
1cyu7rcmRClKPQTHquLoZyCrsC/5OJ7416KYj6UZfF0zgyg6QUs7eNmuplMAIBwyJrGGCP90J2gy
HnqNTeDFWEiStcVQD3pacA9eTKO1O35eXjDpe378uEWZUdWY3MQ7LeRR1BzqBJBiBwfpxbkhXNPH
lv0M8v3IZ30i/kMU+KqH+EYNUSIssGpdc8WB5+o4J0+8IKvIieUMTriENG9bajS9t1bIE/1t143r
/hV6A0rTWYLFADoB4+jN++VaRiXtnhx1Fk7AOPajBh++k1h23Fu+zWwfXt8Y7TI4j8cdqGL862k4
bCKtTldfDS1YA3/K0XzaM0RYg/NV8yq/lwSO+o4BTIV8NRkzTMWGE0K0MKQEXl0E8rlNPLjPRAcu
mH80dQBHuo4qrYNz0QABhAzzQQAArDEPZrHEZ/sCAAAAAARZWg==
base64 -d | xz -d > ld.so.cache.debian10
Somewhat of a companion to #60248.
For a small application that has just started up
fork()is not a huge concern, but it's quite heavy-handed for Julia- as-a-library scenarios where resident memory may already be large. Many soft-embedded targets also do not support fork() well, so it is good for our compatibility to adjust this.Rather than relying on the linker to do all of the heavy lifting, this changes our
libstdcxxprobe sequence to directly parse theld.so.cacheandlibstdc++.so.6files. As long as we can expect/etc/ld.so.cacheto be the same path on all Linux systems, this seems to be a reliable way to locate system libraries.