Merge pull request #18677 from ziglang/post-new-macho

macho: fix logic for parsing dependent dylibs aka re-exports

Author: kubkon

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");
@@ -887,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;
             },
         };
 
@@ -906,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,
@@ -1163,37 +1128,55 @@ fn isHoisted(self: *MachO, install_name: []const u8) bool {
     return false;
 }
 
-fn accessPath(path: []const u8) !bool {
-    std.fs.cwd().access(path, .{}) catch |err| switch (err) {
-        error.FileNotFound => return false,
-        else => |e| return e,
-    };
-    return true;
-}
+fn accessLibPath(
+    arena: Allocator,
+    test_path: *std.ArrayList(u8),
+    checked_paths: *std.ArrayList([]const u8),
+    search_dir: []const u8,
+    name: []const u8,
+) !bool {
+    const sep = fs.path.sep_str;
 
-fn resolveLib(arena: Allocator, search_dirs: []const []const u8, name: []const u8) !?[]const u8 {
-    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;
-        }
+    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 null;
+
+    return false;
 }
 
-fn resolveFramework(arena: Allocator, search_dirs: []const []const u8, name: []const u8) !?[]const u8 {
-    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;
-        }
+fn accessFrameworkPath(
+    arena: Allocator,
+    test_path: *std.ArrayList(u8),
+    checked_paths: *std.ArrayList([]const u8),
+    search_dir: []const u8,
+    name: []const u8,
+) !bool {
+    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 null;
+
+    return false;
 }
 
 fn parseDependentDylibs(self: *MachO) !void {
@@ -1204,8 +1187,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 +1199,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,43 +1212,52 @@ 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);
-                    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
-                        const full_path = (try resolveFramework(arena.allocator(), framework_dirs, stem)) orelse break :fail;
-                        break :full_path full_path;
+                    // Framework
+                    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;
-                    const full_path = (try resolveLib(arena.allocator(), lib_dirs, lib_name)) orelse break :fail;
-                    break :full_path full_path;
+                    for (lib_dirs) |dir| {
+                        test_path.clearRetainingCapacity();
+                        if (try accessLibPath(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.allocator(), &.{ 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 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;
                     }
                 }
 
                 if (eatPrefix(id.name, "@rpath/")) |path| {
                     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 });
+                        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;
@@ -1277,12 +1270,21 @@ 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 {
-                    dependents.appendAssumeCapacity(0);
+                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,
+                        checked_paths.items,
+                        "unable to resolve dependency",
+                        .{},
+                    );
+                    has_errors = true;
                     continue;
-                };
-                break :full_path full_path;
+                }
             };
             const lib = SystemLib{
                 .path = full_path,
@@ -1304,11 +1306,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 +3861,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()});
 }