From d8d6ea6af25af7ecfea1351c652c51de49169536 Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Wed, 24 Jan 2024 14:30:53 +0100 Subject: [PATCH 1/5] macho: print tried paths for unresolved dylib deps --- src/link/MachO.zig | 114 ++++++++++++++++++++++++++++++++++----------- 1 file changed, 88 insertions(+), 26 deletions(-) diff --git a/src/link/MachO.zig b/src/link/MachO.zig index c9f655fd19..3b945216bb 100644 --- a/src/link/MachO.zig +++ b/src/link/MachO.zig @@ -1163,37 +1163,59 @@ fn isHoisted(self: *MachO, install_name: []const u8) bool { return false; } -fn accessPath(path: []const u8) !bool { +fn accessPath( + arena: Allocator, + test_path: *std.ArrayList(u8), + checked_paths: *std.ArrayList([]const u8), + path: []const u8, +) !bool { + test_path.clearRetainingCapacity(); + try test_path.appendSlice(path); std.fs.cwd().access(path, .{}) catch |err| switch (err) { - error.FileNotFound => return false, + error.FileNotFound => { + try checked_paths.append(try arena.dupe(u8, test_path.items)); + return false; + }, else => |e| return e, }; return true; } -fn resolveLib(arena: Allocator, search_dirs: []const []const u8, name: []const u8) !?[]const u8 { +fn resolveLib( + arena: Allocator, + test_path: *std.ArrayList(u8), + checked_paths: *std.ArrayList([]const u8), + search_dirs: []const []const u8, + name: []const u8, +) !bool { const path = try std.fmt.allocPrint(arena, "lib{s}", .{name}); for (search_dirs) |dir| { for (&[_][]const u8{ ".tbd", ".dylib" }) |ext| { const with_ext = try std.fmt.allocPrint(arena, "{s}{s}", .{ path, ext }); const full_path = try std.fs.path.join(arena, &[_][]const u8{ dir, with_ext }); - if (try accessPath(full_path)) return full_path; + if (try accessPath(arena, test_path, checked_paths, full_path)) return true; } } - return null; + return false; } -fn resolveFramework(arena: Allocator, search_dirs: []const []const u8, name: []const u8) !?[]const u8 { +fn resolveFramework( + arena: Allocator, + test_path: *std.ArrayList(u8), + checked_paths: *std.ArrayList([]const u8), + search_dirs: []const []const u8, + name: []const u8, +) !bool { const prefix = try std.fmt.allocPrint(arena, "{s}.framework", .{name}); const path = try std.fs.path.join(arena, &[_][]const u8{ prefix, name }); for (search_dirs) |dir| { for (&[_][]const u8{ ".tbd", ".dylib" }) |ext| { const with_ext = try std.fmt.allocPrint(arena, "{s}{s}", .{ path, ext }); const full_path = try std.fs.path.join(arena, &[_][]const u8{ dir, with_ext }); - if (try accessPath(full_path)) return full_path; + if (try accessPath(arena, test_path, checked_paths, full_path)) return true; } } - return null; + return false; } fn parseDependentDylibs(self: *MachO) !void { @@ -1204,8 +1226,9 @@ fn parseDependentDylibs(self: *MachO) !void { const lib_dirs = self.lib_dirs; const framework_dirs = self.framework_dirs; - var arena = std.heap.ArenaAllocator.init(gpa); - defer arena.deinit(); + var arena_alloc = std.heap.ArenaAllocator.init(gpa); + defer arena_alloc.deinit(); + const arena = arena_alloc.allocator(); // TODO handle duplicate dylibs - it is not uncommon to have the same dylib loaded multiple times // in which case we should track that and return File.Index immediately instead re-parsing paths. @@ -1215,7 +1238,7 @@ fn parseDependentDylibs(self: *MachO) !void { while (index < self.dylibs.items.len) : (index += 1) { const dylib_index = self.dylibs.items[index]; - var dependents = std.ArrayList(File.Index).init(gpa); + var dependents = std.ArrayList(struct { id: Dylib.Id, file: File.Index }).init(gpa); defer dependents.deinit(); try dependents.ensureTotalCapacityPrecise(self.getFile(dylib_index).?.dylib.dependents.items.len); @@ -1228,6 +1251,9 @@ fn parseDependentDylibs(self: *MachO) !void { // 3. If name is a relative path, substitute @rpath, @loader_path, @executable_path with // dependees list of rpaths, and search there. // 4. Finally, just search the provided relative path directly in CWD. + var test_path = std.ArrayList(u8).init(arena); + var checked_paths = std.ArrayList([]const u8).init(arena); + const full_path = full_path: { fail: { const stem = std.fs.path.stem(id.name); @@ -1239,24 +1265,36 @@ fn parseDependentDylibs(self: *MachO) !void { if (mem.endsWith(u8, id.name, framework_name)) { // Framework - const full_path = (try resolveFramework(arena.allocator(), framework_dirs, stem)) orelse break :fail; - break :full_path full_path; + if (try resolveFramework( + arena, + &test_path, + &checked_paths, + framework_dirs, + stem, + )) break :full_path test_path.items; + break :fail; } // Library const lib_name = eatPrefix(stem, "lib") orelse stem; - const full_path = (try resolveLib(arena.allocator(), lib_dirs, lib_name)) orelse break :fail; - break :full_path full_path; + if (try resolveLib( + arena, + &test_path, + &checked_paths, + lib_dirs, + lib_name, + )) break :full_path test_path.items; + break :fail; } if (std.fs.path.isAbsolute(id.name)) { const path = if (self.base.comp.sysroot) |root| - try std.fs.path.join(arena.allocator(), &.{ root, id.name }) + try std.fs.path.join(arena, &.{ root, id.name }) else id.name; for (&[_][]const u8{ "", ".tbd", ".dylib" }) |ext| { - const full_path = try std.fmt.allocPrint(arena.allocator(), "{s}{s}", .{ path, ext }); - if (try accessPath(full_path)) break :full_path full_path; + const full_path = try std.fmt.allocPrint(arena, "{s}{s}", .{ path, ext }); + if (try accessPath(arena, &test_path, &checked_paths, full_path)) break :full_path test_path.items; } } @@ -1264,7 +1302,7 @@ fn parseDependentDylibs(self: *MachO) !void { const dylib = self.getFile(dylib_index).?.dylib; for (self.getFile(dylib.umbrella).?.dylib.rpaths.keys()) |rpath| { const prefix = eatPrefix(rpath, "@loader_path/") orelse rpath; - const rel_path = try std.fs.path.join(arena.allocator(), &.{ prefix, path }); + const rel_path = try std.fs.path.join(arena, &.{ prefix, path }); var buffer: [std.fs.MAX_PATH_BYTES]u8 = undefined; const full_path = std.fs.realpath(rel_path, &buffer) catch continue; break :full_path full_path; @@ -1279,7 +1317,14 @@ fn parseDependentDylibs(self: *MachO) !void { var buffer: [std.fs.MAX_PATH_BYTES]u8 = undefined; const full_path = std.fs.realpath(id.name, &buffer) catch { - dependents.appendAssumeCapacity(0); + try self.reportMissingDependencyError( + self.getFile(dylib_index).?.dylib.getUmbrella(self).index, + id.name, + checked_paths.items, + "unable to resolve dependency", + .{}, + ); + has_errors = true; continue; }; break :full_path full_path; @@ -1304,11 +1349,13 @@ fn parseDependentDylibs(self: *MachO) !void { break :file_index file_index; } }; - dependents.appendAssumeCapacity(file_index); + dependents.appendAssumeCapacity(.{ .id = id, .file = file_index }); } const dylib = self.getFile(dylib_index).?.dylib; - for (dylib.dependents.items, dependents.items) |id, file_index| { + for (dependents.items) |entry| { + const id = entry.id; + const file_index = entry.file; if (self.getFile(file_index)) |file| { const dep_dylib = file.dylib; dep_dylib.hoisted = self.isHoisted(id.name); @@ -3857,18 +3904,33 @@ fn reportMissingLibraryError( } } +fn reportMissingDependencyError( + self: *MachO, + parent: File.Index, + path: []const u8, + checked_paths: []const []const u8, + comptime format: []const u8, + args: anytype, +) error{OutOfMemory}!void { + var err = try self.addErrorWithNotes(2 + checked_paths.len); + try err.addMsg(self, format, args); + try err.addNote(self, "while resolving {s}", .{path}); + try err.addNote(self, "a dependency of {}", .{self.getFile(parent).?.fmtPath()}); + for (checked_paths) |p| { + try err.addNote(self, "tried {s}", .{p}); + } +} + fn reportDependencyError( self: *MachO, parent: File.Index, - path: ?[]const u8, + path: []const u8, comptime format: []const u8, args: anytype, ) error{OutOfMemory}!void { var err = try self.addErrorWithNotes(2); try err.addMsg(self, format, args); - if (path) |p| { - try err.addNote(self, "while parsing {s}", .{p}); - } + try err.addNote(self, "while parsing {s}", .{path}); try err.addNote(self, "a dependency of {}", .{self.getFile(parent).?.fmtPath()}); } From dc4ef6d5d05f3e8b99ec38628a3f671a9268236f Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Wed, 24 Jan 2024 14:45:54 +0100 Subject: [PATCH 2/5] macho: try frameworks before libs non-exclusive --- src/link/MachO.zig | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/src/link/MachO.zig b/src/link/MachO.zig index 3b945216bb..5574b7b6f0 100644 --- a/src/link/MachO.zig +++ b/src/link/MachO.zig @@ -1257,23 +1257,15 @@ fn parseDependentDylibs(self: *MachO) !void { const full_path = full_path: { fail: { const stem = std.fs.path.stem(id.name); - const framework_name = try std.fmt.allocPrint(gpa, "{s}.framework" ++ std.fs.path.sep_str ++ "{s}", .{ - stem, - stem, - }); - defer gpa.free(framework_name); - if (mem.endsWith(u8, id.name, framework_name)) { - // Framework - if (try resolveFramework( - arena, - &test_path, - &checked_paths, - framework_dirs, - stem, - )) break :full_path test_path.items; - break :fail; - } + // Framework + if (try resolveFramework( + arena, + &test_path, + &checked_paths, + framework_dirs, + stem, + )) break :full_path test_path.items; // Library const lib_name = eatPrefix(stem, "lib") orelse stem; From 51d60d1a606224357d406d1addf4701099f678d6 Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Wed, 24 Jan 2024 14:52:17 +0100 Subject: [PATCH 3/5] macho: dump print search lib and framework dirs in --verbose-link --- src/link/MachO.zig | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/link/MachO.zig b/src/link/MachO.zig index 5574b7b6f0..1829a645cc 100644 --- a/src/link/MachO.zig +++ b/src/link/MachO.zig @@ -844,6 +844,11 @@ fn dumpArgv(self: *MachO, comp: *Compilation) !void { try argv.append(arg); } + for (self.lib_dirs) |lib_dir| { + const arg = try std.fmt.allocPrint(arena, "-L{s}", .{lib_dir}); + try argv.append(arg); + } + for (self.frameworks) |framework| { const name = std.fs.path.stem(framework.path); const arg = if (framework.needed) @@ -855,6 +860,11 @@ fn dumpArgv(self: *MachO, comp: *Compilation) !void { try argv.append(arg); } + for (self.framework_dirs) |f_dir| { + try argv.append("-F"); + try argv.append(f_dir); + } + if (self.base.isDynLib() and self.base.allow_shlib_undefined) { try argv.append("-undefined"); try argv.append("dynamic_lookup"); From dc9b6ecb551d6ae7992c31fb28a0fdc94f0c8e4c Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Thu, 25 Jan 2024 10:30:17 +0100 Subject: [PATCH 4/5] macho: refactor resolving libs and frameworks when searching dependent dylibs --- src/link/MachO.zig | 126 ++++++++++++++++++++++----------------------- 1 file changed, 63 insertions(+), 63 deletions(-) diff --git a/src/link/MachO.zig b/src/link/MachO.zig index 1829a645cc..6a170f9ca0 100644 --- a/src/link/MachO.zig +++ b/src/link/MachO.zig @@ -1173,58 +1173,54 @@ fn isHoisted(self: *MachO, install_name: []const u8) bool { return false; } -fn accessPath( +fn accessLibPath2( arena: Allocator, test_path: *std.ArrayList(u8), checked_paths: *std.ArrayList([]const u8), - path: []const u8, -) !bool { - test_path.clearRetainingCapacity(); - try test_path.appendSlice(path); - std.fs.cwd().access(path, .{}) catch |err| switch (err) { - error.FileNotFound => { - try checked_paths.append(try arena.dupe(u8, test_path.items)); - return false; - }, - else => |e| return e, - }; - return true; -} - -fn resolveLib( - arena: Allocator, - test_path: *std.ArrayList(u8), - checked_paths: *std.ArrayList([]const u8), - search_dirs: []const []const u8, + search_dir: []const u8, name: []const u8, ) !bool { - const path = try std.fmt.allocPrint(arena, "lib{s}", .{name}); - for (search_dirs) |dir| { - for (&[_][]const u8{ ".tbd", ".dylib" }) |ext| { - const with_ext = try std.fmt.allocPrint(arena, "{s}{s}", .{ path, ext }); - const full_path = try std.fs.path.join(arena, &[_][]const u8{ dir, with_ext }); - if (try accessPath(arena, test_path, checked_paths, full_path)) return true; - } + const sep = fs.path.sep_str; + + for (&[_][]const u8{ ".tbd", ".dylib", "" }) |ext| { + test_path.clearRetainingCapacity(); + try test_path.writer().print("{s}" ++ sep ++ "lib{s}{s}", .{ search_dir, name, ext }); + try checked_paths.append(try arena.dupe(u8, test_path.items)); + std.fs.cwd().access(test_path.items, .{}) catch |err| switch (err) { + error.FileNotFound => continue, + else => |e| return e, + }; + return true; } + return false; } -fn resolveFramework( +fn accessFrameworkPath( arena: Allocator, test_path: *std.ArrayList(u8), checked_paths: *std.ArrayList([]const u8), - search_dirs: []const []const u8, + search_dir: []const u8, name: []const u8, ) !bool { - const prefix = try std.fmt.allocPrint(arena, "{s}.framework", .{name}); - const path = try std.fs.path.join(arena, &[_][]const u8{ prefix, name }); - for (search_dirs) |dir| { - for (&[_][]const u8{ ".tbd", ".dylib" }) |ext| { - const with_ext = try std.fmt.allocPrint(arena, "{s}{s}", .{ path, ext }); - const full_path = try std.fs.path.join(arena, &[_][]const u8{ dir, with_ext }); - if (try accessPath(arena, test_path, checked_paths, full_path)) return true; - } + const sep = fs.path.sep_str; + + for (&[_][]const u8{ ".tbd", ".dylib", "" }) |ext| { + test_path.clearRetainingCapacity(); + try test_path.writer().print("{s}" ++ sep ++ "{s}.framework" ++ sep ++ "{s}{s}", .{ + search_dir, + name, + name, + ext, + }); + try checked_paths.append(try arena.dupe(u8, test_path.items)); + std.fs.cwd().access(test_path.items, .{}) catch |err| switch (err) { + error.FileNotFound => continue, + else => |e| return e, + }; + return true; } + return false; } @@ -1265,38 +1261,39 @@ fn parseDependentDylibs(self: *MachO) !void { var checked_paths = std.ArrayList([]const u8).init(arena); const full_path = full_path: { - fail: { + { const stem = std.fs.path.stem(id.name); // Framework - if (try resolveFramework( - arena, - &test_path, - &checked_paths, - framework_dirs, - stem, - )) break :full_path test_path.items; + for (framework_dirs) |dir| { + test_path.clearRetainingCapacity(); + if (try accessFrameworkPath(arena, &test_path, &checked_paths, dir, stem)) break :full_path test_path.items; + } // Library const lib_name = eatPrefix(stem, "lib") orelse stem; - if (try resolveLib( - arena, - &test_path, - &checked_paths, - lib_dirs, - lib_name, - )) break :full_path test_path.items; - break :fail; + for (lib_dirs) |dir| { + test_path.clearRetainingCapacity(); + if (try accessLibPath2(arena, &test_path, &checked_paths, dir, lib_name)) break :full_path test_path.items; + } } if (std.fs.path.isAbsolute(id.name)) { - const path = if (self.base.comp.sysroot) |root| - try std.fs.path.join(arena, &.{ root, id.name }) - else - id.name; - for (&[_][]const u8{ "", ".tbd", ".dylib" }) |ext| { - const full_path = try std.fmt.allocPrint(arena, "{s}{s}", .{ path, ext }); - if (try accessPath(arena, &test_path, &checked_paths, full_path)) break :full_path test_path.items; + const existing_ext = std.fs.path.extension(id.name); + const path = if (existing_ext.len > 0) id.name[0 .. id.name.len - existing_ext.len] else id.name; + for (&[_][]const u8{ ".tbd", ".dylib", "" }) |ext| { + test_path.clearRetainingCapacity(); + if (self.base.comp.sysroot) |root| { + try test_path.writer().print("{s}" ++ std.fs.path.sep_str ++ "{s}{s}", .{ root, path, ext }); + } else { + try test_path.writer().print("{s}{s}", .{ path, ext }); + } + try checked_paths.append(try arena.dupe(u8, test_path.items)); + std.fs.cwd().access(test_path.items, .{}) catch |err| switch (err) { + error.FileNotFound => continue, + else => |e| return e, + }; + break :full_path test_path.items; } } @@ -1305,6 +1302,7 @@ fn parseDependentDylibs(self: *MachO) !void { for (self.getFile(dylib.umbrella).?.dylib.rpaths.keys()) |rpath| { const prefix = eatPrefix(rpath, "@loader_path/") orelse rpath; const rel_path = try std.fs.path.join(arena, &.{ prefix, path }); + try checked_paths.append(rel_path); var buffer: [std.fs.MAX_PATH_BYTES]u8 = undefined; const full_path = std.fs.realpath(rel_path, &buffer) catch continue; break :full_path full_path; @@ -1317,8 +1315,11 @@ fn parseDependentDylibs(self: *MachO) !void { return error.Unhandled; } + try checked_paths.append(try arena.dupe(u8, id.name)); var buffer: [std.fs.MAX_PATH_BYTES]u8 = undefined; - const full_path = std.fs.realpath(id.name, &buffer) catch { + if (std.fs.realpath(id.name, &buffer)) |full_path| { + break :full_path full_path; + } else |_| { try self.reportMissingDependencyError( self.getFile(dylib_index).?.dylib.getUmbrella(self).index, id.name, @@ -1328,8 +1329,7 @@ fn parseDependentDylibs(self: *MachO) !void { ); has_errors = true; continue; - }; - break :full_path full_path; + } }; const lib = SystemLib{ .path = full_path, From d62e7bbefde641065eb726c3b561defd594ddae8 Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Thu, 25 Jan 2024 11:24:28 +0100 Subject: [PATCH 5/5] macho: unify accessLibPath with accessLibPath2 --- src/link/MachO.zig | 53 ++++------------------------------------------ 1 file changed, 4 insertions(+), 49 deletions(-) diff --git a/src/link/MachO.zig b/src/link/MachO.zig index 6a170f9ca0..9ace2e3b82 100644 --- a/src/link/MachO.zig +++ b/src/link/MachO.zig @@ -897,11 +897,11 @@ pub fn resolveLibSystem( if (self.sdk_layout) |sdk_layout| switch (sdk_layout) { .sdk => { const dir = try fs.path.join(arena, &[_][]const u8{ comp.sysroot.?, "usr", "lib" }); - if (try accessLibPath(arena, &test_path, &checked_paths, dir, "libSystem")) break :success; + if (try accessLibPath(arena, &test_path, &checked_paths, dir, "System")) break :success; }, .vendored => { const dir = try comp.zig_lib_directory.join(arena, &[_][]const u8{ "libc", "darwin" }); - if (try accessLibPath(arena, &test_path, &checked_paths, dir, "libSystem")) break :success; + if (try accessLibPath(arena, &test_path, &checked_paths, dir, "System")) break :success; }, }; @@ -916,51 +916,6 @@ pub fn resolveLibSystem( }); } -fn accessLibPath( - gpa: Allocator, - test_path: *std.ArrayList(u8), - checked_paths: *std.ArrayList([]const u8), - search_dir: []const u8, - lib_name: []const u8, -) !bool { - const sep = fs.path.sep_str; - - tbd: { - test_path.clearRetainingCapacity(); - try test_path.writer().print("{s}" ++ sep ++ "{s}.tbd", .{ search_dir, lib_name }); - try checked_paths.append(try gpa.dupe(u8, test_path.items)); - fs.cwd().access(test_path.items, .{}) catch |err| switch (err) { - error.FileNotFound => break :tbd, - else => |e| return e, - }; - return true; - } - - dylib: { - test_path.clearRetainingCapacity(); - try test_path.writer().print("{s}" ++ sep ++ "{s}.dylib", .{ search_dir, lib_name }); - try checked_paths.append(try gpa.dupe(u8, test_path.items)); - fs.cwd().access(test_path.items, .{}) catch |err| switch (err) { - error.FileNotFound => break :dylib, - else => |e| return e, - }; - return true; - } - - noextension: { - test_path.clearRetainingCapacity(); - try test_path.writer().print("{s}" ++ sep ++ "{s}", .{ search_dir, lib_name }); - try checked_paths.append(try gpa.dupe(u8, test_path.items)); - fs.cwd().access(test_path.items, .{}) catch |err| switch (err) { - error.FileNotFound => break :noextension, - else => |e| return e, - }; - return true; - } - - return false; -} - const ParseError = error{ MalformedObject, MalformedArchive, @@ -1173,7 +1128,7 @@ fn isHoisted(self: *MachO, install_name: []const u8) bool { return false; } -fn accessLibPath2( +fn accessLibPath( arena: Allocator, test_path: *std.ArrayList(u8), checked_paths: *std.ArrayList([]const u8), @@ -1274,7 +1229,7 @@ fn parseDependentDylibs(self: *MachO) !void { const lib_name = eatPrefix(stem, "lib") orelse stem; for (lib_dirs) |dir| { test_path.clearRetainingCapacity(); - if (try accessLibPath2(arena, &test_path, &checked_paths, dir, lib_name)) break :full_path test_path.items; + if (try accessLibPath(arena, &test_path, &checked_paths, dir, lib_name)) break :full_path test_path.items; } }