Skip to content

Commit a08cc7d

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 f887b02 commit a08cc7d

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
@@ -3410,21 +3410,20 @@ pub fn linkWithZld(macho_file: *MachO, comp: *Compilation, prog_node: *std.Progr
34103410
// installation sources because they are always a product of the compiler version + target information.
34113411
man.hash.add(stack_size);
34123412
man.hash.addOptional(options.pagezero_size);
3413-
man.hash.addOptional(options.search_strategy);
34143413
man.hash.addOptional(options.headerpad_size);
34153414
man.hash.add(options.headerpad_max_install_names);
34163415
man.hash.add(gc_sections);
34173416
man.hash.add(options.dead_strip_dylibs);
34183417
man.hash.add(options.strip);
34193418
man.hash.addListOfBytes(options.lib_dirs);
34203419
man.hash.addListOfBytes(options.framework_dirs);
3421-
link.hashAddSystemLibs(&man.hash, options.frameworks);
3420+
link.hashAddFrameworks(&man.hash, options.frameworks);
34223421
man.hash.addListOfBytes(options.rpath_list);
34233422
if (is_dyn_lib) {
34243423
man.hash.addOptionalBytes(options.install_name);
34253424
man.hash.addOptional(options.version);
34263425
}
3427-
link.hashAddSystemLibs(&man.hash, options.system_libs);
3426+
try link.hashAddSystemLibs(&man, options.system_libs);
34283427
man.hash.addOptionalBytes(options.sysroot);
34293428
man.hash.addListOfBytes(options.force_undefined_symbols.keys());
34303429
try man.addOptionalFile(options.entitlements);
@@ -3550,84 +3549,20 @@ pub fn linkWithZld(macho_file: *MachO, comp: *Compilation, prog_node: *std.Progr
35503549
try positionals.append(comp.libcxx_static_lib.?.full_object_path);
35513550
}
35523551

3553-
// Shared and static libraries passed via `-l` flag.
3554-
var candidate_libs = std.StringArrayHashMap(link.SystemLib).init(arena);
3555-
3556-
const system_lib_names = options.system_libs.keys();
3557-
for (system_lib_names) |system_lib_name| {
3558-
// By this time, we depend on these libs being dynamically linked libraries and not static libraries
3559-
// (the check for that needs to be earlier), but they could be full paths to .dylib files, in which
3560-
// case we want to avoid prepending "-l".
3561-
if (Compilation.classifyFileExt(system_lib_name) == .shared_library) {
3562-
try positionals.append(system_lib_name);
3563-
continue;
3564-
}
3565-
3566-
const system_lib_info = options.system_libs.get(system_lib_name).?;
3567-
try candidate_libs.put(system_lib_name, .{
3568-
.needed = system_lib_info.needed,
3569-
.weak = system_lib_info.weak,
3570-
});
3571-
}
3572-
3573-
var lib_dirs = std.ArrayList([]const u8).init(arena);
3574-
for (options.lib_dirs) |dir| {
3575-
if (try MachO.resolveSearchDir(arena, dir, options.sysroot)) |search_dir| {
3576-
try lib_dirs.append(search_dir);
3577-
} else {
3578-
log.warn("directory not found for '-L{s}'", .{dir});
3579-
}
3552+
{
3553+
// Add all system library paths to positionals.
3554+
const vals = options.system_libs.values();
3555+
try positionals.ensureUnusedCapacity(vals.len);
3556+
for (vals) |info| positionals.appendAssumeCapacity(info.path);
35803557
}
35813558

35823559
var libs = std.StringArrayHashMap(link.SystemLib).init(arena);
35833560

3584-
// Assume ld64 default -search_paths_first if no strategy specified.
3585-
const search_strategy = options.search_strategy orelse .paths_first;
3586-
outer: for (candidate_libs.keys()) |lib_name| {
3587-
switch (search_strategy) {
3588-
.paths_first => {
3589-
// Look in each directory for a dylib (stub first), and then for archive
3590-
for (lib_dirs.items) |dir| {
3591-
for (&[_][]const u8{ ".tbd", ".dylib", ".a" }) |ext| {
3592-
if (try MachO.resolveLib(arena, dir, lib_name, ext)) |full_path| {
3593-
try libs.put(full_path, candidate_libs.get(lib_name).?);
3594-
continue :outer;
3595-
}
3596-
}
3597-
} else {
3598-
log.warn("library not found for '-l{s}'", .{lib_name});
3599-
lib_not_found = true;
3600-
}
3601-
},
3602-
.dylibs_first => {
3603-
// First, look for a dylib in each search dir
3604-
for (lib_dirs.items) |dir| {
3605-
for (&[_][]const u8{ ".tbd", ".dylib" }) |ext| {
3606-
if (try MachO.resolveLib(arena, dir, lib_name, ext)) |full_path| {
3607-
try libs.put(full_path, candidate_libs.get(lib_name).?);
3608-
continue :outer;
3609-
}
3610-
}
3611-
} else for (lib_dirs.items) |dir| {
3612-
if (try MachO.resolveLib(arena, dir, lib_name, ".a")) |full_path| {
3613-
try libs.put(full_path, candidate_libs.get(lib_name).?);
3614-
} else {
3615-
log.warn("library not found for '-l{s}'", .{lib_name});
3616-
lib_not_found = true;
3617-
}
3618-
}
3619-
},
3620-
}
3621-
}
3622-
3623-
if (lib_not_found) {
3624-
log.warn("Library search paths:", .{});
3625-
for (lib_dirs.items) |dir| {
3626-
log.warn(" {s}", .{dir});
3627-
}
3561+
for (options.system_libs.values()) |v| {
3562+
try libs.put(v.path, v);
36283563
}
36293564

3630-
try MachO.resolveLibSystem(arena, comp, options.sysroot, target, lib_dirs.items, &libs);
3565+
try MachO.resolveLibSystem(arena, comp, options.sysroot, target, options.lib_dirs, &libs);
36313566

36323567
// frameworks
36333568
var framework_dirs = std.ArrayList([]const u8).init(arena);
@@ -3647,6 +3582,7 @@ pub fn linkWithZld(macho_file: *MachO, comp: *Compilation, prog_node: *std.Progr
36473582
try libs.put(full_path, .{
36483583
.needed = info.needed,
36493584
.weak = info.weak,
3585+
.path = full_path,
36503586
});
36513587
continue :outer;
36523588
}
@@ -3698,11 +3634,6 @@ pub fn linkWithZld(macho_file: *MachO, comp: *Compilation, prog_node: *std.Progr
36983634
try argv.append(try std.fmt.allocPrint(arena, "0x{x}", .{pagezero_size}));
36993635
}
37003636

3701-
if (options.search_strategy) |strat| switch (strat) {
3702-
.paths_first => try argv.append("-search_paths_first"),
3703-
.dylibs_first => try argv.append("-search_dylibs_first"),
3704-
};
3705-
37063637
if (options.headerpad_size) |headerpad_size| {
37073638
try argv.append("-headerpad_size");
37083639
try argv.append(try std.fmt.allocPrint(arena, "0x{x}", .{headerpad_size}));

0 commit comments

Comments
 (0)