zig icon indicating copy to clipboard operation
zig copied to clipboard
verified publisher icon ziglang

compiler: resolve library paths in the frontend

Open andrewrk opened this issue 2 years ago • 10 comments

search_strategy is no longer passed to Compilation at all; instead it is used in the CLI code only.

When using Zig CLI mode, -l no longer has the ability to link statically; use positional arguments for this.

The CLI has a small abstraction around library resolution handling which is used to remove some code duplication regarding static libraries, as well as handle the difference between zig cc CLI mode and zig CLI mode.

Thanks to this, system libraries are now included in the cache hash, and thus changes to them will correctly cause cache misses.

In the future, lib_dirs should no longer be passed to Compilation at all, because it is a frontend-only concept.

Previously, -search_paths_first and -search_dylibs_first were Darwin-only arguments; they now work the same for all targets. Same thing with --sysroot.

Improved the error reporting for failure to find a system library. An example error now looks like this:

$ zig build-exe test.zig -lfoo -L. -L/a -target x86_64-macos --sysroot /home/andy/local
error: unable to find Dynamic system library 'foo' using strategy 'no_fallback'. search paths:
  ./libfoo.tbd
  ./libfoo.dylib
  ./libfoo.so
  /home/andy/local/a/libfoo.tbd
  /home/andy/local/a/libfoo.dylib
  /home/andy/local/a/libfoo.so
  /a/libfoo.tbd
  /a/libfoo.dylib
  /a/libfoo.so

If I use the zig cc frontend instead, it uses a different search strategy:

$ zig cc test.zig -lfoo -L. -L/a -target x86_64-macos --sysroot /home/andy/local
error: unable to find Dynamic system library 'foo' using strategy 'paths_first'. searched paths:
  ./libfoo.tbd
  ./libfoo.dylib
  ./libfoo.so
  ./libfoo.a
  /home/andy/local/a/libfoo.tbd
  /home/andy/local/a/libfoo.dylib
  /home/andy/local/a/libfoo.so
  /home/andy/local/a/libfoo.a
  /a/libfoo.tbd
  /a/libfoo.dylib
  /a/libfoo.so
  /a/libfoo.a

Using the Zig CLI, -search_dylibs_first can now be used for any target:

$ zig build-exe test.zig -search_dylibs_first -lfoo -L. -L/a   --sysroot /home/andy/local
error: unable to find Dynamic system library 'foo' using strategy 'mode_first'. searched paths:
  ./libfoo.so
  /home/andy/local/a/libfoo.so
  /a/libfoo.so
  ./libfoo.a
  /home/andy/local/a/libfoo.a
  /a/libfoo.a

closes #14963

andrewrk avatar Jun 16 '23 00:06 andrewrk

This needs another commit to fix handling of system DLLs on Windows such as kernel32, but it's ready to go for other targets.

andrewrk avatar Jun 16 '23 00:06 andrewrk

I ran into an issue with this right away when testing with one of my projects linking system libraries using std.Build.linkSystemLibrary() which invokes pkg-config to determine the required linker flags. pkg-config assumes the flags will be passed to the system linker and omits flags that would be redundant with the system linker's defaults. For example, pkg-config --libs xkbcommon on my system prints only -lxkbcommon since /usr/lib is a default search path of my system linker.

With this patch we no longer rely on the library searching of the system linker however so libxkbcommon is not found as the code in main.zig is not aware of the /usr/lib system default.

As for how best to fix this change in behavior for my use-case it appears adding --keep-system-libs to the pkg-config invocation in std.Build.Compile is sufficient.

Unfortunately this affects zig cc as well however and users will expect zig cc's linker to continue to have the traditional default search paths. I guess we can probably look at lld's source to determine exactly what these paths are and add the necessary logic to main.zig to match the behavior there.

To recap, this works perfectly for my use-cases with the following patch!

diff --git a/lib/std/Build/Step/Compile.zig b/lib/std/Build/Step/Compile.zig
index 092fdf7e6..51ab882db 100644
--- a/lib/std/Build/Step/Compile.zig
+++ b/lib/std/Build/Step/Compile.zig
@@ -842,6 +842,7 @@ fn runPkgConfig(self: *Compile, lib_name: []const u8) ![]const []const u8 {
         pkg_name,
         "--cflags",
         "--libs",
+        "--keep-system-libs",
     }, &code, .Ignore)) |stdout| stdout else |err| switch (err) {
         error.ProcessTerminated => return error.PkgConfigCrashed,
         error.ExecNotSupported => return error.PkgConfigFailed,

This looks like an incorrect assumption of the system linker (hello GNU ld). Having said that, zig doesn't use system linker on linux - it uses ld.lld so I am surprised how this issue arises. Anyhow, library search paths, IMHO, should always be passed explicitly to the linker to avoid confusion. There are also a few env vars we could read such as LD_LIBRARY_PATH. If you have a minute, you could check against zld/ELF or mold and see if the same problem happens with and without these changes.

kubkon avatar Jun 16 '23 10:06 kubkon

I mentioned zld and mold as neither assumes any default search paths and hence their output differs from that of gold.

kubkon avatar Jun 16 '23 10:06 kubkon

@kubkon It looks like I was wrong about where these defaults that pkg-config assumes come from. They are defined in clang and gcc not the linker. (here for clang).

It appears zig 0.10 discovered and passed several library paths to lld that aren't currently being searched for libraries on this branch.

For this 0.10 zig build-exe invocation 
/usr/bin/zig
build-exe
/home/ifreund/projects/river/river/main.zig
-lc
-I/usr/include/libevdev-1.0/
-levdev
-linput
-I/home/ifreund/.local/include
-L/home/ifreund/.local/lib
-lwayland-server
-lxkbcommon
-I/usr/include/pixman-1
-lpixman-1
-I/home/ifreund/.local/include
-I/usr/include/libdrm
-I/usr/include/pixman-1
-I/usr/include/elogind
-L/home/ifreund/.local/lib
-lwlroots
-lxcb-render
-lxcb
-cflags
-std=c99
-O2
--
/home/ifreund/projects/river/river/wlroots_log_wrapper.c
-cflags
-std=c99
--
/home/ifreund/projects/river/zig-cache/zig-wayland/river-control-unstable-v1-protocol.c
-cflags
-std=c99
--
/home/ifreund/projects/river/zig-cache/zig-wayland/river-status-unstable-v1-protocol.c
-cflags
-std=c99
--
/home/ifreund/projects/river/zig-cache/zig-wayland/river-layout-v3-protocol.c
-cflags
-std=c99
--
/home/ifreund/projects/river/zig-cache/zig-wayland/wlr-layer-shell-unstable-v1-protocol.c
-cflags
-std=c99
--
/home/ifreund/projects/river/zig-cache/zig-wayland/wlr-output-power-management-unstable-v1-protocol.c
-cflags
-std=c99
--
/home/ifreund/projects/river/zig-cache/zig-wayland/wayland-protocol.c
-cflags
-std=c99
--
/home/ifreund/projects/river/zig-cache/zig-wayland/xdg-shell-protocol.c
-cflags
-std=c99
--
/home/ifreund/projects/river/zig-cache/zig-wayland/ext-session-lock-v1-protocol.c
-cflags
-std=c99
--
/home/ifreund/projects/river/zig-cache/zig-wayland/pointer-gestures-unstable-v1-protocol.c
-cflags
-std=c99
--
/home/ifreund/projects/river/zig-cache/zig-wayland/pointer-constraints-unstable-v1-protocol.c
-cflags
-std=c99
--
/home/ifreund/projects/river/zig-cache/zig-wayland/xdg-decoration-unstable-v1-protocol.c
--verbose-link
-fno-strip
--cache-dir
/home/ifreund/projects/river/zig-cache
--global-cache-dir
/home/ifreund/.cache/zig
--name
river
--pkg-begin
build_options
/home/ifreund/projects/river/zig-cache/options/PFeQBqsbBfUE1Hxk6euUwQxjZyqmNbiHLz7NeGT-cli0bJ0Gwa3C9KRImKOZfRFZ
--pkg-end
--pkg-begin
wayland
/home/ifreund/projects/river/zig-cache/zig-wayland/wayland.zig
--pkg-end
--pkg-begin
xkbcommon
/home/ifreund/projects/river/deps/zig-xkbcommon/src/xkbcommon.zig
--pkg-end
--pkg-begin
pixman
/home/ifreund/projects/river/deps/zig-pixman/pixman.zig
--pkg-end
--pkg-begin
wlroots
/home/ifreund/projects/river/deps/zig-wlroots/src/wlroots.zig
--pkg-begin
wayland
/home/ifreund/projects/river/zig-cache/zig-wayland/wayland.zig
--pkg-end
--pkg-begin
xkbcommon
/home/ifreund/projects/river/deps/zig-xkbcommon/src/xkbcommon.zig
--pkg-end
--pkg-begin
pixman
/home/ifreund/projects/river/deps/zig-pixman/pixman.zig
--pkg-end
--pkg-end
--pkg-begin
flags
/home/ifreund/projects/river/common/flags.zig
--pkg-end
--pkg-begin
globber
/home/ifreund/projects/river/common/globber.zig
--pkg-end
-fno-PIE
--enable-cache
This is the lld invocation, notably with -L paths not passed to zig build-exe 
LLD
Link...
ld.lld
--error-limit=0
-O0
-z
stack-size=16777216
--gc-sections
-znow
-m
elf_x86_64
-o
/home/ifreund/projects/river/zig-cache/o/f211c5300e0bd9a8c792222e98477eb8/river
/usr/lib64/gcc/x86_64-unknown-linux-gnu/12.2.0/../../../../lib64/crt1.o
/usr/lib64/gcc/x86_64-unknown-linux-gnu/12.2.0/../../../../lib64/crti.o
-rpath
/home/ifreund/.local/lib
-rpath
/lib64
-rpath
/lib
-rpath
/usr/lib64
-rpath
/usr/lib
-L
/home/ifreund/.local/lib
-L
/home/ifreund/.local/lib
-L
/usr/local/lib64
-L
/usr/local/lib
-L
/usr/lib/x86_64-linux-gnu
-L
/lib64
-L
/lib
-L
/usr/lib64
-L
/usr/lib
-L
/lib/x86_64-linux-gnu
-L
/usr/lib64/gcc/x86_64-unknown-linux-gnu/12.2.0/../../../../lib64
-dynamic-linker
/lib64/ld-linux-x86-64.so.2
/home/ifreund/projects/river/zig-cache/o/6bd51a4e9e3a8206829692dcbfba5597/wlroots_log_wrapper.o
/home/ifreund/projects/river/zig-cache/o/f1c73443dbc049d7a9b343d02f0fd37f/river-control-unstable-v1-protocol.o
/home/ifreund/projects/river/zig-cache/o/ffceb842e67fcfad859fb3c9b043494f/river-status-unstable-v1-protocol.o
/home/ifreund/projects/river/zig-cache/o/564426e805589cea984b22febbd3f658/river-layout-v3-protocol.o
/home/ifreund/projects/river/zig-cache/o/eb48adeb59558bdc61ee7daad7c66441/wlr-layer-shell-unstable-v1-protocol.o
/home/ifreund/projects/river/zig-cache/o/4cf6df4c782bd5c468677cbcfe02a976/wlr-output-power-management-unstable-v1-protocol.o
/home/ifreund/projects/river/zig-cache/o/566938818770f24821f1f2dbf395e812/wayland-protocol.o
/home/ifreund/projects/river/zig-cache/o/2908e3d7ff2c44fbc41069a2541f9a7d/xdg-shell-protocol.o
/home/ifreund/projects/river/zig-cache/o/aa59332b62a050ddd3b50686b5f40d6e/ext-session-lock-v1-protocol.o
/home/ifreund/projects/river/zig-cache/o/fbdadbd90f0aa91adfe677f4c1c0c9d2/pointer-gestures-unstable-v1-protocol.o
/home/ifreund/projects/river/zig-cache/o/0cc1c95f3b0a63875580c4911e1b999b/pointer-constraints-unstable-v1-protocol.o
/home/ifreund/projects/river/zig-cache/o/bcdd16725f5f754203bbb7490f8b18cd/xdg-decoration-unstable-v1-protocol.o
/home/ifreund/projects/river/zig-cache/o/f211c5300e0bd9a8c792222e98477eb8/river.o
--as-needed
-levdev
-linput
-lwayland-server
-lxkbcommon
-lpixman-1
-lwlroots
-lxcb-render
-lxcb
-lm
-lpthread
-lc
-ldl
-lrt
-lutil
/home/ifreund/.cache/zig/o/10ff83478723b881dcc3f83108d8f8d4/libcompiler_rt.a
/usr/lib64/gcc/x86_64-unknown-linux-gnu/12.2.0/../../../../lib64/crtn.o

ifreund avatar Jun 16 '23 10:06 ifreund

Indeed... why do you have to be like this gnu :/

$ cat /usr/lib/libgcc_s.so
/* GNU ld script
   Use the shared library, but some functions are only in
   the static library.  */
GROUP ( libgcc_s.so.1 -lgcc )

ifreund avatar Jun 16 '23 14:06 ifreund

So, this may be a bit counter-intuitive, but I actually think there is a strong argument for moving parsing of these fake .so files into the CLI as well. Hear me out...

https://github.com/ziglang/zig/blob/0f5aff34414bcb024443540fe905039f3783803a/src/main.zig#L2459-L2471

This code here is in master branch, and checks if -lgcc_s is provided on the command line. Look familiar?

The frontend is in the business of knowing which libraries are part of libc, part of compiler_rt, or other, because it needs this information to set up the compilation pipeline - potentially scheduling the creation of compiler_rt to run in parallel, for example.

Also, I think it is telling that inside these files are command line arguments so it actually does make sense if you think about it to handle in the command line argument parsing code.

andrewrk avatar Jun 16 '23 22:06 andrewrk

I searched my hard drive for .so files that are actually text files, and came up with a small list of unique test cases. The entire list of unique file basenames in this sample set is:

libbsd.so
libc++.so
libc.so
libgcc_s.so
libm.so
libpthread.so
libtbb.so
libtbbmalloc.so
libtbbmalloc_proxy.so

All the files concatenated together, with redundant contents manually removed:

----------------------------
/* GNU ld script
   Use the shared library, but some functions are only in
   the static library, so try that secondarily.  */
OUTPUT_FORMAT(elf64-x86-64)
GROUP ( /nix/store/2j8jqmnd9l7plihhf713yf291c9vyqjm-glibc-2.35-224/lib/libc.so.6 /nix/store/2j8jqmnd9l7plihhf713yf291c9vyqjm-glibc-2.35-224/lib/libc_nonshared.a  AS_NEEDED ( /nix/store/2j8jqmnd9l7plihhf713yf291c9vyqjm-glibc-2.35-224/lib/ld-linux-x86-64.so.2 ) )
----------------------------
/* GNU ld script
   Use the shared library, but some functions are only in
   the static library.  */
GROUP ( libgcc_s.so.1 -lgcc )
----------------------------
/* GNU ld script
*/
OUTPUT_FORMAT(elf64-x86-64)
GROUP ( /nix/store/2j8jqmnd9l7plihhf713yf291c9vyqjm-glibc-2.35-224/lib/libm.so.6  AS_NEEDED ( /nix/store/2j8jqmnd9l7plihhf713yf291c9vyqjm-glibc-2.35-224/lib/libmvec.so.1 ) )
----------------------------
/* GNU ld script
 * The MD5 functions are provided by the libmd library. */
OUTPUT_FORMAT(elf32-i386)
GROUP(/nix/store/9jjcvqp0x2kyvmbbvaxmp4qfps66sffr-libbsd-0.11.6/lib/libbsd.so.0.11.6 AS_NEEDED(-lmd))
----------------------------
/* GNU ld script
   Use the shared library, but some functions are only in
   the static library, so try that secondarily.  */
OUTPUT_FORMAT(elf32-i386)
GROUP ( /nix/store/bzw74a7dla8iqk2h8wshwi03fbgj2c5h-glibc-2.35-163/lib/libc.so.6 /nix/store/bzw74a7dla8iqk2h8wshwi03fbgj2c5h-glibc-2.35-163/lib/libc_nonshared.a  AS_NEEDED ( /nix/store/bzw74a7dla8iqk2h8wshwi03fbgj2c5h-glibc-2.35-163/lib/ld-linux.so.2 ) )
----------------------------
INPUT (libtbb.so.2)
----------------------------
INPUT (libtbbmalloc.so.2)
----------------------------
INPUT (libtbbmalloc_proxy.so.2)
----------------------------
/* GNU ld script
   Use the shared library, but some functions are only in
   the static library, so try that secondarily.  */
OUTPUT_FORMAT(elf32-i386)
GROUP ( libc.so.6 libc_nonshared.a  AS_NEEDED ( ld-linux.so.2 ) )
----------------------------
/* GNU ld script
   Use the shared library, but some functions are only in
   the static library, so try that secondarily.  */
OUTPUT_FORMAT(elf32-i386)
GROUP ( libpthread.so.0 libpthread_nonshared.a )
----------------------------
INPUT(libc++.so.1 -lc++abi)
----------------------------
/* GNU ld script
   Use the shared library, but some functions are only in
   the static library, so try that secondarily.  */
OUTPUT_FORMAT(elf64-x86-64)
GROUP ( libc.so.6 libc_nonshared.a  AS_NEEDED ( ld-linux-x86-64.so.2 ) )
----------------------------
/* GNU ld script
*/
OUTPUT_FORMAT(elf64-x86-64)
GROUP ( libm.so.6  AS_NEEDED ( libmvec_nonshared.a libmvec.so.1 ) )
----------------------------
/* GNU ld script
   Use the shared library, but some functions are only in
   the static library, so try that secondarily.  */
OUTPUT_FORMAT(elf64-x86-64)
GROUP ( libpthread.so.0 libpthread_nonshared.a )
----------------------------
/* GNU ld script
   Use the shared library, but some functions are only in
   the static library, so try that secondarily.  */
OUTPUT_FORMAT(elf64-x86-64)
GROUP ( /nix/store/vnwdak3n1w2jjil119j65k8mw1z23p84-glibc-2.35-224/lib/libc.so.6 /nix/store/vnwdak3n1w2jjil119j65k8mw1z23p84-glibc-2.35-224/lib/libc_nonshared.a  AS_NEEDED ( /nix/store/vnwdak3n1w2jjil119j65k8mw1z23p84-glibc-2.35-224/lib/ld-linux-x86-64.so.2 ) )
----------------------------

As you can see, there are not very many different concepts at play here. All this stuff directly corresponds to command line arguments.

andrewrk avatar Jun 16 '23 23:06 andrewrk

Yeah, I think we should be able to do this in the CLI frontend. As far as I understand, we only need to support a limited ld script syntax to be able to correctly parse those files, and the good news is I've got that implemented in zld already. I could then upstream the corresponding functionality of the linker and make it part of the CLI which would look something like this:

  1. Preopen each positional and check if it's a linker script.
  2. Parse the script and substitute it for its contents preserving the link state (this involves correctly handling GROUP together with AS_NEEDED).
  3. Repeat 2. recursively as a script may refer another script.
  4. Pass the parsed positionals to the linker.

What do you think @andrewrk and @ifreund?

kubkon avatar Jun 17 '23 05:06 kubkon

I think it sounds like the path forward!

andrewrk avatar Jun 17 '23 08:06 andrewrk

I think it sounds like the path forward!

Excellent!

kubkon avatar Jun 17 '23 09:06 kubkon

I don't think this accounts for mingw's lib<name>.dll.a libraries

ypsvlq avatar Aug 03 '23 20:08 ypsvlq

Please file a bug report and we can get it fixed in 0.11.1. The logic is here: https://github.com/ziglang/zig/blob/a327d8b9957f8adee20eea93907b9497fc1b8ce4/src/main.zig#L6240-L6318

this is growing pains from moving logic from LLD into the frontend, as we move to using our own linker implementations.

andrewrk avatar Aug 03 '23 20:08 andrewrk