Skip to content

Commit bbcd48b

Browse files
andrewrkandrewrk
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 1253d59 commit bbcd48b

File tree

7 files changed

+449
-244
lines changed

7 files changed

+449
-244
lines changed

‎src/Compilation.zig‎

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

449+
pub const Framework = link.Framework;
449450
pub const SystemLib = link.SystemLib;
450451
pub const CacheMode = link.CacheMode;
451452

@@ -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:
@@ -641,8 +642,6 @@ pub const InitOptions = struct {
641642
entitlements: ?[]const u8 = null,
642643
/// (Darwin) size of the __PAGEZERO segment
643644
pagezero_size: ?u64 = null,
644-
/// (Darwin) search strategy for system libraries
645-
search_strategy: ?link.File.MachO.SearchStrategy = null,
646645
/// (Darwin) set minimum space for future expansion of the load commands
647646
headerpad_size: ?u32 = null,
648647
/// (Darwin) set enough space as if all paths were MATPATHLEN
@@ -1535,7 +1534,6 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation {
15351534
.install_name = options.install_name,
15361535
.entitlements = options.entitlements,
15371536
.pagezero_size = options.pagezero_size,
1538-
.search_strategy = options.search_strategy,
15391537
.headerpad_size = options.headerpad_size,
15401538
.headerpad_max_install_names = options.headerpad_max_install_names,
15411539
.dead_strip_dylibs = options.dead_strip_dylibs,
@@ -1695,15 +1693,18 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation {
16951693

16961694
// When linking mingw-w64 there are some import libs we always need.
16971695
for (mingw.always_link_libs) |name| {
1698-
try comp.bin_file.options.system_libs.put(comp.gpa, name, .{});
1696+
try comp.bin_file.options.system_libs.put(comp.gpa, name, .{
1697+
.needed = false,
1698+
.weak = false,
1699+
.path = name,
1700+
});
16991701
}
17001702
}
17011703
// Generate Windows import libs.
17021704
if (target.os.tag == .windows) {
17031705
const count = comp.bin_file.options.system_libs.count();
17041706
try comp.work_queue.ensureUnusedCapacity(count);
1705-
var i: usize = 0;
1706-
while (i < count) : (i += 1) {
1707+
for (0..count) |i| {
17071708
comp.work_queue.writeItemAssumeCapacity(.{ .windows_import_lib = i });
17081709
}
17091710
}
@@ -2314,7 +2315,7 @@ fn addNonIncrementalStuffToCacheManifest(comp: *Compilation, man: *Cache.Manifes
23142315
}
23152316
man.hash.addOptionalBytes(comp.bin_file.options.soname);
23162317
man.hash.addOptional(comp.bin_file.options.version);
2317-
link.hashAddSystemLibs(&man.hash, comp.bin_file.options.system_libs);
2318+
try link.hashAddSystemLibs(man, comp.bin_file.options.system_libs);
23182319
man.hash.addListOfBytes(comp.bin_file.options.force_undefined_symbols.keys());
23192320
man.hash.addOptional(comp.bin_file.options.allow_shlib_undefined);
23202321
man.hash.add(comp.bin_file.options.bind_global_refs_locally);
@@ -2331,10 +2332,9 @@ fn addNonIncrementalStuffToCacheManifest(comp: *Compilation, man: *Cache.Manifes
23312332

23322333
// Mach-O specific stuff
23332334
man.hash.addListOfBytes(comp.bin_file.options.framework_dirs);
2334-
link.hashAddSystemLibs(&man.hash, comp.bin_file.options.frameworks);
2335+
link.hashAddFrameworks(&man.hash, comp.bin_file.options.frameworks);
23352336
try man.addOptionalFile(comp.bin_file.options.entitlements);
23362337
man.hash.addOptional(comp.bin_file.options.pagezero_size);
2337-
man.hash.addOptional(comp.bin_file.options.search_strategy);
23382338
man.hash.addOptional(comp.bin_file.options.headerpad_size);
23392339
man.hash.add(comp.bin_file.options.headerpad_max_install_names);
23402340
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);
@@ -179,9 +200,12 @@ pub const Options = struct {
179200

180201
objects: []Compilation.LinkObject,
181202
framework_dirs: []const []const u8,
182-
frameworks: std.StringArrayHashMapUnmanaged(SystemLib),
203+
frameworks: std.StringArrayHashMapUnmanaged(Framework),
204+
/// These are *always* dynamically linked. Static libraries will be
205+
/// provided as positional arguments.
183206
system_libs: std.StringArrayHashMapUnmanaged(SystemLib),
184207
wasi_emulated_libs: []const wasi_libc.CRTFile,
208+
// TODO: remove this. libraries are resolved by the frontend.
185209
lib_dirs: []const []const u8,
186210
rpath_list: []const []const u8,
187211

@@ -221,9 +245,6 @@ pub const Options = struct {
221245
/// (Darwin) size of the __PAGEZERO segment
222246
pagezero_size: ?u64 = null,
223247

224-
/// (Darwin) search strategy for system libraries
225-
search_strategy: ?File.MachO.SearchStrategy = null,
226-
227248
/// (Darwin) set minimum space for future expansion of the load commands
228249
headerpad_size: ?u32 = null,
229250

‎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
@@ -1427,7 +1427,7 @@ fn linkWithLLD(self: *Elf, comp: *Compilation, prog_node: *std.Progress.Node) !v
14271427
}
14281428
man.hash.addOptionalBytes(self.base.options.soname);
14291429
man.hash.addOptional(self.base.options.version);
1430-
link.hashAddSystemLibs(&man.hash, self.base.options.system_libs);
1430+
try link.hashAddSystemLibs(&man, self.base.options.system_libs);
14311431
man.hash.addListOfBytes(self.base.options.force_undefined_symbols.keys());
14321432
man.hash.add(allow_shlib_undefined);
14331433
man.hash.add(self.base.options.bind_global_refs_locally);
@@ -1823,8 +1823,8 @@ fn linkWithLLD(self: *Elf, comp: *Compilation, prog_node: *std.Progress.Node) !v
18231823
argv.appendAssumeCapacity("--as-needed");
18241824
var as_needed = true;
18251825

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

18491847
if (!as_needed) {

‎src/link/MachO.zig‎

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

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

60-
pub const SearchStrategy = enum {
61-
paths_first,
62-
dylibs_first,
63-
};
64-
6560
/// Mode of operation of the linker.
6661
pub const Mode = enum {
6762
/// Incremental mode will preallocate segments/sections and is compatible with
@@ -843,7 +838,11 @@ pub fn resolveLibSystem(
843838
// re-exports every single symbol definition.
844839
for (search_dirs) |dir| {
845840
if (try resolveLib(arena, dir, "System", ".tbd")) |full_path| {
846-
try out_libs.put(full_path, .{ .needed = true });
841+
try out_libs.put(full_path, .{
842+
.needed = true,
843+
.weak = false,
844+
.path = full_path,
845+
});
847846
libsystem_available = true;
848847
break :blk;
849848
}
@@ -853,8 +852,16 @@ pub fn resolveLibSystem(
853852
for (search_dirs) |dir| {
854853
if (try resolveLib(arena, dir, "System", ".dylib")) |libsystem_path| {
855854
if (try resolveLib(arena, dir, "c", ".dylib")) |libc_path| {
856-
try out_libs.put(libsystem_path, .{ .needed = true });
857-
try out_libs.put(libc_path, .{ .needed = true });
855+
try out_libs.put(libsystem_path, .{
856+
.needed = true,
857+
.weak = false,
858+
.path = libsystem_path,
859+
});
860+
try out_libs.put(libc_path, .{
861+
.needed = true,
862+
.weak = false,
863+
.path = libc_path,
864+
});
858865
libsystem_available = true;
859866
break :blk;
860867
}
@@ -868,7 +875,11 @@ pub fn resolveLibSystem(
868875
const full_path = try comp.zig_lib_directory.join(arena, &[_][]const u8{
869876
"libc", "darwin", libsystem_name,
870877
});
871-
try out_libs.put(full_path, .{ .needed = true });
878+
try out_libs.put(full_path, .{
879+
.needed = true,
880+
.weak = false,
881+
.path = full_path,
882+
});
872883
}
873884
}
874885

‎src/link/MachO/zld.zig‎

Lines changed: 11 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -3508,21 +3508,20 @@ pub fn linkWithZld(macho_file: *MachO, comp: *Compilation, prog_node: *std.Progr
35083508
// installation sources because they are always a product of the compiler version + target information.
35093509
man.hash.add(stack_size);
35103510
man.hash.addOptional(options.pagezero_size);
3511-
man.hash.addOptional(options.search_strategy);
35123511
man.hash.addOptional(options.headerpad_size);
35133512
man.hash.add(options.headerpad_max_install_names);
35143513
man.hash.add(gc_sections);
35153514
man.hash.add(options.dead_strip_dylibs);
35163515
man.hash.add(options.strip);
35173516
man.hash.addListOfBytes(options.lib_dirs);
35183517
man.hash.addListOfBytes(options.framework_dirs);
3519-
link.hashAddSystemLibs(&man.hash, options.frameworks);
3518+
link.hashAddFrameworks(&man.hash, options.frameworks);
35203519
man.hash.addListOfBytes(options.rpath_list);
35213520
if (is_dyn_lib) {
35223521
man.hash.addOptionalBytes(options.install_name);
35233522
man.hash.addOptional(options.version);
35243523
}
3525-
link.hashAddSystemLibs(&man.hash, options.system_libs);
3524+
try link.hashAddSystemLibs(&man, options.system_libs);
35263525
man.hash.addOptionalBytes(options.sysroot);
35273526
man.hash.addListOfBytes(options.force_undefined_symbols.keys());
35283527
try man.addOptionalFile(options.entitlements);
@@ -3648,84 +3647,20 @@ pub fn linkWithZld(macho_file: *MachO, comp: *Compilation, prog_node: *std.Progr
36483647
try positionals.append(comp.libcxx_static_lib.?.full_object_path);
36493648
}
36503649

3651-
// Shared and static libraries passed via `-l` flag.
3652-
var candidate_libs = std.StringArrayHashMap(link.SystemLib).init(arena);
3653-
3654-
const system_lib_names = options.system_libs.keys();
3655-
for (system_lib_names) |system_lib_name| {
3656-
// By this time, we depend on these libs being dynamically linked libraries and not static libraries
3657-
// (the check for that needs to be earlier), but they could be full paths to .dylib files, in which
3658-
// case we want to avoid prepending "-l".
3659-
if (Compilation.classifyFileExt(system_lib_name) == .shared_library) {
3660-
try positionals.append(system_lib_name);
3661-
continue;
3662-
}
3663-
3664-
const system_lib_info = options.system_libs.get(system_lib_name).?;
3665-
try candidate_libs.put(system_lib_name, .{
3666-
.needed = system_lib_info.needed,
3667-
.weak = system_lib_info.weak,
3668-
});
3669-
}
3670-
3671-
var lib_dirs = std.ArrayList([]const u8).init(arena);
3672-
for (options.lib_dirs) |dir| {
3673-
if (try MachO.resolveSearchDir(arena, dir, options.sysroot)) |search_dir| {
3674-
try lib_dirs.append(search_dir);
3675-
} else {
3676-
log.warn("directory not found for '-L{s}'", .{dir});
3677-
}
3650+
{
3651+
// Add all system library paths to positionals.
3652+
const vals = options.system_libs.values();
3653+
try positionals.ensureUnusedCapacity(vals.len);
3654+
for (vals) |info| positionals.appendAssumeCapacity(info.path);
36783655
}
36793656

36803657
var libs = std.StringArrayHashMap(link.SystemLib).init(arena);
36813658

3682-
// Assume ld64 default -search_paths_first if no strategy specified.
3683-
const search_strategy = options.search_strategy orelse .paths_first;
3684-
outer: for (candidate_libs.keys()) |lib_name| {
3685-
switch (search_strategy) {
3686-
.paths_first => {
3687-
// Look in each directory for a dylib (stub first), and then for archive
3688-
for (lib_dirs.items) |dir| {
3689-
for (&[_][]const u8{ ".tbd", ".dylib", ".a" }) |ext| {
3690-
if (try MachO.resolveLib(arena, dir, lib_name, ext)) |full_path| {
3691-
try libs.put(full_path, candidate_libs.get(lib_name).?);
3692-
continue :outer;
3693-
}
3694-
}
3695-
} else {
3696-
log.warn("library not found for '-l{s}'", .{lib_name});
3697-
lib_not_found = true;
3698-
}
3699-
},
3700-
.dylibs_first => {
3701-
// First, look for a dylib in each search dir
3702-
for (lib_dirs.items) |dir| {
3703-
for (&[_][]const u8{ ".tbd", ".dylib" }) |ext| {
3704-
if (try MachO.resolveLib(arena, dir, lib_name, ext)) |full_path| {
3705-
try libs.put(full_path, candidate_libs.get(lib_name).?);
3706-
continue :outer;
3707-
}
3708-
}
3709-
} else for (lib_dirs.items) |dir| {
3710-
if (try MachO.resolveLib(arena, dir, lib_name, ".a")) |full_path| {
3711-
try libs.put(full_path, candidate_libs.get(lib_name).?);
3712-
} else {
3713-
log.warn("library not found for '-l{s}'", .{lib_name});
3714-
lib_not_found = true;
3715-
}
3716-
}
3717-
},
3718-
}
3659+
for (options.system_libs.values()) |v| {
3660+
try libs.put(v.path, v);
37193661
}
37203662

3721-
if (lib_not_found) {
3722-
log.warn("Library search paths:", .{});
3723-
for (lib_dirs.items) |dir| {
3724-
log.warn(" {s}", .{dir});
3725-
}
3726-
}
3727-
3728-
try MachO.resolveLibSystem(arena, comp, options.sysroot, target, lib_dirs.items, &libs);
3663+
try MachO.resolveLibSystem(arena, comp, options.sysroot, target, options.lib_dirs, &libs);
37293664

37303665
// frameworks
37313666
var framework_dirs = std.ArrayList([]const u8).init(arena);
@@ -3745,6 +3680,7 @@ pub fn linkWithZld(macho_file: *MachO, comp: *Compilation, prog_node: *std.Progr
37453680
try libs.put(full_path, .{
37463681
.needed = info.needed,
37473682
.weak = info.weak,
3683+
.path = full_path,
37483684
});
37493685
continue :outer;
37503686
}
@@ -3796,11 +3732,6 @@ pub fn linkWithZld(macho_file: *MachO, comp: *Compilation, prog_node: *std.Progr
37963732
try argv.append(try std.fmt.allocPrint(arena, "0x{x}", .{pagezero_size}));
37973733
}
37983734

3799-
if (options.search_strategy) |strat| switch (strat) {
3800-
.paths_first => try argv.append("-search_paths_first"),
3801-
.dylibs_first => try argv.append("-search_dylibs_first"),
3802-
};
3803-
38043735
if (options.headerpad_size) |headerpad_size| {
38053736
try argv.append("-headerpad_size");
38063737
try argv.append(try std.fmt.allocPrint(arena, "0x{x}", .{headerpad_size}));

0 commit comments

Comments
 (0)