Skip to content

Commit b7d6902

Browse files
committed
compiler: resolve library paths in the frontend
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 ``` closes #14963
1 parent 9e19969 commit b7d6902

File tree

7 files changed

+454
-246
lines changed

7 files changed

+454
-246
lines changed

src/Compilation.zig

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -448,6 +448,7 @@ pub const ClangPreprocessorMode = enum {
448448
stdout,
449449
};
450450

451+
pub const Framework = link.Framework;
451452
pub const SystemLib = link.SystemLib;
452453
pub const CacheMode = link.CacheMode;
453454

@@ -505,7 +506,7 @@ pub const InitOptions = struct {
505506
c_source_files: []const CSourceFile = &[0]CSourceFile{},
506507
link_objects: []LinkObject = &[0]LinkObject{},
507508
framework_dirs: []const []const u8 = &[0][]const u8{},
508-
frameworks: std.StringArrayHashMapUnmanaged(SystemLib) = .{},
509+
frameworks: std.StringArrayHashMapUnmanaged(Framework) = .{},
509510
system_lib_names: []const []const u8 = &.{},
510511
system_lib_infos: []const SystemLib = &.{},
511512
/// These correspond to the WASI libc emulated subcomponents including:
@@ -644,8 +645,6 @@ pub const InitOptions = struct {
644645
entitlements: ?[]const u8 = null,
645646
/// (Darwin) size of the __PAGEZERO segment
646647
pagezero_size: ?u64 = null,
647-
/// (Darwin) search strategy for system libraries
648-
search_strategy: ?link.File.MachO.SearchStrategy = null,
649648
/// (Darwin) set minimum space for future expansion of the load commands
650649
headerpad_size: ?u32 = null,
651650
/// (Darwin) set enough space as if all paths were MATPATHLEN
@@ -1567,7 +1566,6 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation {
15671566
.install_name = options.install_name,
15681567
.entitlements = options.entitlements,
15691568
.pagezero_size = options.pagezero_size,
1570-
.search_strategy = options.search_strategy,
15711569
.headerpad_size = options.headerpad_size,
15721570
.headerpad_max_install_names = options.headerpad_max_install_names,
15731571
.dead_strip_dylibs = options.dead_strip_dylibs,
@@ -1727,15 +1725,18 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation {
17271725

17281726
// When linking mingw-w64 there are some import libs we always need.
17291727
for (mingw.always_link_libs) |name| {
1730-
try comp.bin_file.options.system_libs.put(comp.gpa, name, .{});
1728+
try comp.bin_file.options.system_libs.put(comp.gpa, name, .{
1729+
.needed = false,
1730+
.weak = false,
1731+
.path = name,
1732+
});
17311733
}
17321734
}
17331735
// Generate Windows import libs.
17341736
if (target.os.tag == .windows) {
17351737
const count = comp.bin_file.options.system_libs.count();
17361738
try comp.work_queue.ensureUnusedCapacity(count);
1737-
var i: usize = 0;
1738-
while (i < count) : (i += 1) {
1739+
for (0..count) |i| {
17391740
comp.work_queue.writeItemAssumeCapacity(.{ .windows_import_lib = i });
17401741
}
17411742
}
@@ -2377,7 +2378,7 @@ fn addNonIncrementalStuffToCacheManifest(comp: *Compilation, man: *Cache.Manifes
23772378
}
23782379
man.hash.addOptionalBytes(comp.bin_file.options.soname);
23792380
man.hash.addOptional(comp.bin_file.options.version);
2380-
link.hashAddSystemLibs(&man.hash, comp.bin_file.options.system_libs);
2381+
try link.hashAddSystemLibs(man, comp.bin_file.options.system_libs);
23812382
man.hash.addListOfBytes(comp.bin_file.options.force_undefined_symbols.keys());
23822383
man.hash.addOptional(comp.bin_file.options.allow_shlib_undefined);
23832384
man.hash.add(comp.bin_file.options.bind_global_refs_locally);
@@ -2395,10 +2396,9 @@ fn addNonIncrementalStuffToCacheManifest(comp: *Compilation, man: *Cache.Manifes
23952396

23962397
// Mach-O specific stuff
23972398
man.hash.addListOfBytes(comp.bin_file.options.framework_dirs);
2398-
link.hashAddSystemLibs(&man.hash, comp.bin_file.options.frameworks);
2399+
link.hashAddFrameworks(&man.hash, comp.bin_file.options.frameworks);
23992400
try man.addOptionalFile(comp.bin_file.options.entitlements);
24002401
man.hash.addOptional(comp.bin_file.options.pagezero_size);
2401-
man.hash.addOptional(comp.bin_file.options.search_strategy);
24022402
man.hash.addOptional(comp.bin_file.options.headerpad_size);
24032403
man.hash.add(comp.bin_file.options.headerpad_max_install_names);
24042404
man.hash.add(comp.bin_file.options.dead_strip_dylibs);

src/link.zig

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,16 @@ const Type = @import("type.zig").Type;
2121
const TypedValue = @import("TypedValue.zig");
2222

2323
/// When adding a new field, remember to update `hashAddSystemLibs`.
24+
/// These are *always* dynamically linked. Static libraries will be
25+
/// provided as positional arguments.
2426
pub const SystemLib = struct {
27+
needed: bool,
28+
weak: bool,
29+
path: []const u8,
30+
};
31+
32+
/// When adding a new field, remember to update `hashAddFrameworks`.
33+
pub const Framework = struct {
2534
needed: bool = false,
2635
weak: bool = false,
2736
};
@@ -31,11 +40,23 @@ pub const SortSection = enum { name, alignment };
3140
pub const CacheMode = enum { incremental, whole };
3241

3342
pub fn hashAddSystemLibs(
34-
hh: *Cache.HashHelper,
43+
man: *Cache.Manifest,
3544
hm: std.StringArrayHashMapUnmanaged(SystemLib),
45+
) !void {
46+
const keys = hm.keys();
47+
man.hash.addListOfBytes(keys);
48+
for (hm.values()) |value| {
49+
man.hash.add(value.needed);
50+
man.hash.add(value.weak);
51+
_ = try man.addFile(value.path, null);
52+
}
53+
}
54+
55+
pub fn hashAddFrameworks(
56+
hh: *Cache.HashHelper,
57+
hm: std.StringArrayHashMapUnmanaged(Framework),
3658
) void {
3759
const keys = hm.keys();
38-
hh.add(keys.len);
3960
hh.addListOfBytes(keys);
4061
for (hm.values()) |value| {
4162
hh.add(value.needed);
@@ -183,9 +204,12 @@ pub const Options = struct {
183204

184205
objects: []Compilation.LinkObject,
185206
framework_dirs: []const []const u8,
186-
frameworks: std.StringArrayHashMapUnmanaged(SystemLib),
207+
frameworks: std.StringArrayHashMapUnmanaged(Framework),
208+
/// These are *always* dynamically linked. Static libraries will be
209+
/// provided as positional arguments.
187210
system_libs: std.StringArrayHashMapUnmanaged(SystemLib),
188211
wasi_emulated_libs: []const wasi_libc.CRTFile,
212+
// TODO: remove this. libraries are resolved by the frontend.
189213
lib_dirs: []const []const u8,
190214
rpath_list: []const []const u8,
191215

@@ -225,9 +249,6 @@ pub const Options = struct {
225249
/// (Darwin) size of the __PAGEZERO segment
226250
pagezero_size: ?u64 = null,
227251

228-
/// (Darwin) search strategy for system libraries
229-
search_strategy: ?File.MachO.SearchStrategy = null,
230-
231252
/// (Darwin) set minimum space for future expansion of the load commands
232253
headerpad_size: ?u32 = null,
233254

src/link/Coff/lld.zig

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ pub fn linkWithLLD(self: *Coff, comp: *Compilation, prog_node: *std.Progress.Nod
8888
}
8989
}
9090
}
91-
link.hashAddSystemLibs(&man.hash, self.base.options.system_libs);
91+
try link.hashAddSystemLibs(&man, self.base.options.system_libs);
9292
man.hash.addListOfBytes(self.base.options.force_undefined_symbols.keys());
9393
man.hash.addOptional(self.base.options.subsystem);
9494
man.hash.add(self.base.options.is_test);

src/link/Elf.zig

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1428,7 +1428,7 @@ fn linkWithLLD(self: *Elf, comp: *Compilation, prog_node: *std.Progress.Node) !v
14281428
}
14291429
man.hash.addOptionalBytes(self.base.options.soname);
14301430
man.hash.addOptional(self.base.options.version);
1431-
link.hashAddSystemLibs(&man.hash, self.base.options.system_libs);
1431+
try link.hashAddSystemLibs(&man, self.base.options.system_libs);
14321432
man.hash.addListOfBytes(self.base.options.force_undefined_symbols.keys());
14331433
man.hash.add(allow_shlib_undefined);
14341434
man.hash.add(self.base.options.bind_global_refs_locally);
@@ -1824,8 +1824,8 @@ fn linkWithLLD(self: *Elf, comp: *Compilation, prog_node: *std.Progress.Node) !v
18241824
argv.appendAssumeCapacity("--as-needed");
18251825
var as_needed = true;
18261826

1827-
for (system_libs, 0..) |link_lib, i| {
1828-
const lib_as_needed = !system_libs_values[i].needed;
1827+
for (system_libs_values) |lib_info| {
1828+
const lib_as_needed = !lib_info.needed;
18291829
switch ((@as(u2, @intFromBool(lib_as_needed)) << 1) | @intFromBool(as_needed)) {
18301830
0b00, 0b11 => {},
18311831
0b01 => {
@@ -1842,9 +1842,7 @@ fn linkWithLLD(self: *Elf, comp: *Compilation, prog_node: *std.Progress.Node) !v
18421842
// libraries and not static libraries (the check for that needs to be earlier),
18431843
// but they could be full paths to .so files, in which case we
18441844
// want to avoid prepending "-l".
1845-
const ext = Compilation.classifyFileExt(link_lib);
1846-
const arg = if (ext == .shared_library) link_lib else try std.fmt.allocPrint(arena, "-l{s}", .{link_lib});
1847-
argv.appendAssumeCapacity(arg);
1845+
argv.appendAssumeCapacity(lib_info.path);
18481846
}
18491847

18501848
if (!as_needed) {

src/link/MachO.zig

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,6 @@ const Rebase = @import("MachO/dyld_info/Rebase.zig");
5858

5959
pub const base_tag: File.Tag = File.Tag.macho;
6060

61-
pub const SearchStrategy = enum {
62-
paths_first,
63-
dylibs_first,
64-
};
65-
6661
/// Mode of operation of the linker.
6762
pub const Mode = enum {
6863
/// Incremental mode will preallocate segments/sections and is compatible with
@@ -840,7 +835,11 @@ pub fn resolveLibSystem(
840835
// re-exports every single symbol definition.
841836
for (search_dirs) |dir| {
842837
if (try resolveLib(arena, dir, "System", ".tbd")) |full_path| {
843-
try out_libs.put(full_path, .{ .needed = true });
838+
try out_libs.put(full_path, .{
839+
.needed = true,
840+
.weak = false,
841+
.path = full_path,
842+
});
844843
libsystem_available = true;
845844
break :blk;
846845
}
@@ -850,8 +849,16 @@ pub fn resolveLibSystem(
850849
for (search_dirs) |dir| {
851850
if (try resolveLib(arena, dir, "System", ".dylib")) |libsystem_path| {
852851
if (try resolveLib(arena, dir, "c", ".dylib")) |libc_path| {
853-
try out_libs.put(libsystem_path, .{ .needed = true });
854-
try out_libs.put(libc_path, .{ .needed = true });
852+
try out_libs.put(libsystem_path, .{
853+
.needed = true,
854+
.weak = false,
855+
.path = libsystem_path,
856+
});
857+
try out_libs.put(libc_path, .{
858+
.needed = true,
859+
.weak = false,
860+
.path = libc_path,
861+
});
855862
libsystem_available = true;
856863
break :blk;
857864
}
@@ -865,7 +872,11 @@ pub fn resolveLibSystem(
865872
const full_path = try comp.zig_lib_directory.join(arena, &[_][]const u8{
866873
"libc", "darwin", libsystem_name,
867874
});
868-
try out_libs.put(full_path, .{ .needed = true });
875+
try out_libs.put(full_path, .{
876+
.needed = true,
877+
.weak = false,
878+
.path = full_path,
879+
});
869880
}
870881
}
871882

src/link/MachO/zld.zig

Lines changed: 11 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -3374,21 +3374,20 @@ pub fn linkWithZld(macho_file: *MachO, comp: *Compilation, prog_node: *std.Progr
33743374
// installation sources because they are always a product of the compiler version + target information.
33753375
man.hash.add(stack_size);
33763376
man.hash.addOptional(options.pagezero_size);
3377-
man.hash.addOptional(options.search_strategy);
33783377
man.hash.addOptional(options.headerpad_size);
33793378
man.hash.add(options.headerpad_max_install_names);
33803379
man.hash.add(gc_sections);
33813380
man.hash.add(options.dead_strip_dylibs);
33823381
man.hash.add(options.strip);
33833382
man.hash.addListOfBytes(options.lib_dirs);
33843383
man.hash.addListOfBytes(options.framework_dirs);
3385-
link.hashAddSystemLibs(&man.hash, options.frameworks);
3384+
link.hashAddFrameworks(&man.hash, options.frameworks);
33863385
man.hash.addListOfBytes(options.rpath_list);
33873386
if (is_dyn_lib) {
33883387
man.hash.addOptionalBytes(options.install_name);
33893388
man.hash.addOptional(options.version);
33903389
}
3391-
link.hashAddSystemLibs(&man.hash, options.system_libs);
3390+
try link.hashAddSystemLibs(&man, options.system_libs);
33923391
man.hash.addOptionalBytes(options.sysroot);
33933392
man.hash.addListOfBytes(options.force_undefined_symbols.keys());
33943393
try man.addOptionalFile(options.entitlements);
@@ -3514,84 +3513,20 @@ pub fn linkWithZld(macho_file: *MachO, comp: *Compilation, prog_node: *std.Progr
35143513
try positionals.append(comp.libcxx_static_lib.?.full_object_path);
35153514
}
35163515

3517-
// Shared and static libraries passed via `-l` flag.
3518-
var candidate_libs = std.StringArrayHashMap(link.SystemLib).init(arena);
3519-
3520-
const system_lib_names = options.system_libs.keys();
3521-
for (system_lib_names) |system_lib_name| {
3522-
// By this time, we depend on these libs being dynamically linked libraries and not static libraries
3523-
// (the check for that needs to be earlier), but they could be full paths to .dylib files, in which
3524-
// case we want to avoid prepending "-l".
3525-
if (Compilation.classifyFileExt(system_lib_name) == .shared_library) {
3526-
try positionals.append(system_lib_name);
3527-
continue;
3528-
}
3529-
3530-
const system_lib_info = options.system_libs.get(system_lib_name).?;
3531-
try candidate_libs.put(system_lib_name, .{
3532-
.needed = system_lib_info.needed,
3533-
.weak = system_lib_info.weak,
3534-
});
3535-
}
3536-
3537-
var lib_dirs = std.ArrayList([]const u8).init(arena);
3538-
for (options.lib_dirs) |dir| {
3539-
if (try MachO.resolveSearchDir(arena, dir, options.sysroot)) |search_dir| {
3540-
try lib_dirs.append(search_dir);
3541-
} else {
3542-
log.warn("directory not found for '-L{s}'", .{dir});
3543-
}
3516+
{
3517+
// Add all system library paths to positionals.
3518+
const vals = options.system_libs.values();
3519+
try positionals.ensureUnusedCapacity(vals.len);
3520+
for (vals) |info| positionals.appendAssumeCapacity(info.path);
35443521
}
35453522

35463523
var libs = std.StringArrayHashMap(link.SystemLib).init(arena);
35473524

3548-
// Assume ld64 default -search_paths_first if no strategy specified.
3549-
const search_strategy = options.search_strategy orelse .paths_first;
3550-
outer: for (candidate_libs.keys()) |lib_name| {
3551-
switch (search_strategy) {
3552-
.paths_first => {
3553-
// Look in each directory for a dylib (stub first), and then for archive
3554-
for (lib_dirs.items) |dir| {
3555-
for (&[_][]const u8{ ".tbd", ".dylib", ".a" }) |ext| {
3556-
if (try MachO.resolveLib(arena, dir, lib_name, ext)) |full_path| {
3557-
try libs.put(full_path, candidate_libs.get(lib_name).?);
3558-
continue :outer;
3559-
}
3560-
}
3561-
} else {
3562-
log.warn("library not found for '-l{s}'", .{lib_name});
3563-
lib_not_found = true;
3564-
}
3565-
},
3566-
.dylibs_first => {
3567-
// First, look for a dylib in each search dir
3568-
for (lib_dirs.items) |dir| {
3569-
for (&[_][]const u8{ ".tbd", ".dylib" }) |ext| {
3570-
if (try MachO.resolveLib(arena, dir, lib_name, ext)) |full_path| {
3571-
try libs.put(full_path, candidate_libs.get(lib_name).?);
3572-
continue :outer;
3573-
}
3574-
}
3575-
} else for (lib_dirs.items) |dir| {
3576-
if (try MachO.resolveLib(arena, dir, lib_name, ".a")) |full_path| {
3577-
try libs.put(full_path, candidate_libs.get(lib_name).?);
3578-
} else {
3579-
log.warn("library not found for '-l{s}'", .{lib_name});
3580-
lib_not_found = true;
3581-
}
3582-
}
3583-
},
3584-
}
3585-
}
3586-
3587-
if (lib_not_found) {
3588-
log.warn("Library search paths:", .{});
3589-
for (lib_dirs.items) |dir| {
3590-
log.warn(" {s}", .{dir});
3591-
}
3525+
for (options.system_libs.values()) |v| {
3526+
try libs.put(v.path, v);
35923527
}
35933528

3594-
try MachO.resolveLibSystem(arena, comp, options.sysroot, target, lib_dirs.items, &libs);
3529+
try MachO.resolveLibSystem(arena, comp, options.sysroot, target, options.lib_dirs, &libs);
35953530

35963531
// frameworks
35973532
var framework_dirs = std.ArrayList([]const u8).init(arena);
@@ -3611,6 +3546,7 @@ pub fn linkWithZld(macho_file: *MachO, comp: *Compilation, prog_node: *std.Progr
36113546
try libs.put(full_path, .{
36123547
.needed = info.needed,
36133548
.weak = info.weak,
3549+
.path = full_path,
36143550
});
36153551
continue :outer;
36163552
}
@@ -3662,11 +3598,6 @@ pub fn linkWithZld(macho_file: *MachO, comp: *Compilation, prog_node: *std.Progr
36623598
try argv.append(try std.fmt.allocPrint(arena, "0x{x}", .{pagezero_size}));
36633599
}
36643600

3665-
if (options.search_strategy) |strat| switch (strat) {
3666-
.paths_first => try argv.append("-search_paths_first"),
3667-
.dylibs_first => try argv.append("-search_dylibs_first"),
3668-
};
3669-
36703601
if (options.headerpad_size) |headerpad_size| {
36713602
try argv.append("-headerpad_size");
36723603
try argv.append(try std.fmt.allocPrint(arena, "0x{x}", .{headerpad_size}));

0 commit comments

Comments
 (0)