zld: when parsing dylibs, allow multiple return values

2 files changed, 90 insertions, 94 deletions

diff --git a/src/link/MachO/Dylib.zig b/src/link/MachO/Dylib.zig
index 723277c541..515df003b6 100644
--- a/src/link/MachO/Dylib.zig
+++ b/src/link/MachO/Dylib.zig
@@ -37,10 +37,7 @@ id: ?Id = null,
 /// a symbol is referenced by an object file.
 symbols: std.StringArrayHashMapUnmanaged(void) = .{},
 
-// TODO we should keep track of already parsed dylibs so that
-// we don't unnecessarily reparse them again.
-// TODO add dylib dep analysis and extraction for .dylib files.
-dylibs: std.ArrayListUnmanaged(*Dylib) = .{},
+dependent_libs: std.StringArrayHashMapUnmanaged(void) = .{},
 
 pub const Id = struct {
     name: []const u8,
@@ -66,7 +63,7 @@ pub fn createAndParseFromPath(
     path: []const u8,
     syslibroot: ?[]const u8,
     recurse_libs: bool,
-) Error!?*Dylib {
+) Error!?[]*Dylib {
     const file = fs.cwd().openFile(path, .{}) catch |err| switch (err) {
         error.FileNotFound => return null,
         else => |e| return e,
@@ -87,7 +84,7 @@ pub fn createAndParseFromPath(
         .syslibroot = syslibroot,
     };
 
-    dylib.parse(recurse_libs) catch |err| switch (err) {
+    dylib.parse() catch |err| switch (err) {
         error.EndOfStream, error.NotDylib => {
             try file.seekTo(0);
 
@@ -98,12 +95,20 @@ pub fn createAndParseFromPath(
             };
             defer lib_stub.deinit();
 
-            try dylib.parseFromStub(lib_stub, recurse_libs);
+            try dylib.parseFromStub(lib_stub);
         },
         else => |e| return e,
     };
 
-    return dylib;
+    var dylibs = std.ArrayList(*Dylib).init(allocator);
+    defer dylibs.deinit();
+    try dylibs.append(dylib);
+
+    if (recurse_libs) {
+        try dylib.parseDependentLibs(&dylibs);
+    }
+
+    return dylibs.toOwnedSlice();
 }
 
 pub fn deinit(self: *Dylib) void {
@@ -116,7 +121,11 @@ pub fn deinit(self: *Dylib) void {
         self.allocator.free(key);
     }
     self.symbols.deinit(self.allocator);
-    self.dylibs.deinit(self.allocator);
+
+    for (self.dependent_libs.keys()) |key| {
+        self.allocator.free(key);
+    }
+    self.dependent_libs.deinit(self.allocator);
 
     if (self.name) |name| {
         self.allocator.free(name);
@@ -133,7 +142,7 @@ pub fn closeFile(self: Dylib) void {
     }
 }
 
-pub fn parse(self: *Dylib, recurse_libs: bool) !void {
+pub fn parse(self: *Dylib) !void {
     log.debug("parsing shared library '{s}'", .{self.name.?});
 
     var reader = self.file.?.reader();
@@ -235,6 +244,13 @@ fn parseSymbols(self: *Dylib) !void {
     }
 }
 
+fn hasTarget(targets: []const []const u8, target: []const u8) bool {
+    for (targets) |t| {
+        if (mem.eql(u8, t, target)) return true;
+    }
+    return false;
+}
+
 fn addObjCClassSymbols(self: *Dylib, sym_name: []const u8) !void {
     const expanded = &[_][]const u8{
         try std.fmt.allocPrint(self.allocator, "_OBJC_CLASS_$_{s}", .{sym_name}),
@@ -247,7 +263,7 @@ fn addObjCClassSymbols(self: *Dylib, sym_name: []const u8) !void {
     }
 }
 
-pub fn parseFromStub(self: *Dylib, lib_stub: LibStub, recurse_libs: bool) !void {
+pub fn parseFromStub(self: *Dylib, lib_stub: LibStub) !void {
     if (lib_stub.inner.len == 0) return error.EmptyStubFile;
 
     log.debug("parsing shared library from stub '{s}'", .{self.name.?});
@@ -270,6 +286,17 @@ pub fn parseFromStub(self: *Dylib, lib_stub: LibStub, recurse_libs: bool) !void
     for (lib_stub.inner) |stub| {
         if (!hasTarget(stub.targets, target_string)) continue;
 
+        if (stub.reexported_libraries) |reexports| {
+            for (reexports) |reexp| {
+                if (!hasTarget(reexp.targets, target_string)) continue;
+
+                try self.dependent_libs.ensureUnusedCapacity(self.allocator, reexp.libraries.len);
+                for (reexp.libraries) |lib| {
+                    self.dependent_libs.putAssumeCapacity(try self.allocator.dupe(u8, lib), {});
+                }
+            }
+        }
+
         if (stub.exports) |exports| {
             for (exports) |exp| {
                 if (!hasTarget(exp.targets, target_string)) continue;
@@ -314,69 +341,53 @@ pub fn parseFromStub(self: *Dylib, lib_stub: LibStub, recurse_libs: bool) !void
             }
         }
     }
+}
 
-    for (lib_stub.inner) |stub| {
-        if (!hasTarget(stub.targets, target_string)) continue;
-
-        if (stub.reexported_libraries) |reexports| reexports: {
-            if (!recurse_libs) break :reexports;
+pub fn parseDependentLibs(self: *Dylib, out: *std.ArrayList(*Dylib)) !void {
+    outer: for (self.dependent_libs.keys()) |lib| {
+        const dirname = fs.path.dirname(lib) orelse {
+            log.warn("unable to resolve dependency {s}", .{lib});
+            continue;
+        };
+        const filename = fs.path.basename(lib);
+        const without_ext = if (mem.lastIndexOfScalar(u8, filename, '.')) |index|
+            filename[0..index]
+        else
+            filename;
+
+        for (&[_][]const u8{ "dylib", "tbd" }) |ext| {
+            const with_ext = try std.fmt.allocPrint(self.allocator, "{s}.{s}", .{
+                without_ext,
+                ext,
+            });
+            defer self.allocator.free(with_ext);
+
+            const lib_path = if (self.syslibroot) |syslibroot|
+                try fs.path.join(self.allocator, &.{ syslibroot, dirname, with_ext })
+            else
+                try fs.path.join(self.allocator, &.{ dirname, with_ext });
+
+            log.debug("trying dependency at fully resolved path {s}", .{lib_path});
+
+            const dylibs = (try createAndParseFromPath(
+                self.allocator,
+                self.arch.?,
+                lib_path,
+                self.syslibroot,
+                true,
+            )) orelse {
+                continue;
+            };
 
-            for (reexports) |reexp| {
-                if (!hasTarget(reexp.targets, target_string)) continue;
+            try out.appendSlice(dylibs);
 
-                outer: for (reexp.libraries) |lib| {
-                    const dirname = fs.path.dirname(lib) orelse {
-                        log.warn("unable to resolve dependency {s}", .{lib});
-                        continue;
-                    };
-                    const filename = fs.path.basename(lib);
-                    const without_ext = if (mem.lastIndexOfScalar(u8, filename, '.')) |index|
-                        filename[0..index]
-                    else
-                        filename;
-
-                    for (&[_][]const u8{ "dylib", "tbd" }) |ext| {
-                        const with_ext = try std.fmt.allocPrint(self.allocator, "{s}.{s}", .{
-                            without_ext,
-                            ext,
-                        });
-                        defer self.allocator.free(with_ext);
-
-                        const lib_path = if (self.syslibroot) |syslibroot|
-                            try fs.path.join(self.allocator, &.{ syslibroot, dirname, with_ext })
-                        else
-                            try fs.path.join(self.allocator, &.{ dirname, with_ext });
-
-                        log.debug("trying dependency at fully resolved path {s}", .{lib_path});
-
-                        const dylib = (try createAndParseFromPath(
-                            self.allocator,
-                            self.arch.?,
-                            lib_path,
-                            self.syslibroot,
-                            true,
-                        )) orelse {
-                            continue;
-                        };
-
-                        try self.dylibs.append(self.allocator, dylib);
-                        continue :outer;
-                    } else {
-                        log.warn("unable to resolve dependency {s}", .{lib});
-                    }
-                }
-            }
+            continue :outer;
+        } else {
+            log.warn("unable to resolve dependency {s}", .{lib});
         }
     }
 }
 
-fn hasTarget(targets: []const []const u8, target: []const u8) bool {
-    for (targets) |t| {
-        if (mem.eql(u8, t, target)) return true;
-    }
-    return false;
-}
-
 pub fn createProxy(self: *Dylib, sym_name: []const u8) !?*Symbol {
     if (!self.symbols.contains(sym_name)) return null;
 
diff --git a/src/link/MachO/Zld.zig b/src/link/MachO/Zld.zig
index a4ee93cf58..a0c60f1b07 100644
--- a/src/link/MachO/Zld.zig
+++ b/src/link/MachO/Zld.zig
@@ -280,8 +280,9 @@ fn parseInputFiles(self: *Zld, files: []const []const u8) !void {
             full_path,
             self.syslibroot,
             true,
-        )) |dylib| {
-            try self.dylibs.append(self.allocator, dylib);
+        )) |dylibs| {
+            defer self.allocator.free(dylibs);
+            try self.dylibs.appendSlice(self.allocator, dylibs);
             continue;
         }
 
@@ -290,18 +291,6 @@ fn parseInputFiles(self: *Zld, files: []const []const u8) !void {
 }
 
 fn parseLibs(self: *Zld, libs: []const []const u8) !void {
-    const DylibDeps = struct {
-        fn bubbleUp(out: *std.ArrayList(*Dylib), next: *Dylib) error{OutOfMemory}!void {
-            try out.ensureUnusedCapacity(next.dylibs.items.len);
-            for (next.dylibs.items) |dylib| {
-                out.appendAssumeCapacity(dylib);
-            }
-            for (next.dylibs.items) |dylib| {
-                try bubbleUp(out, dylib);
-            }
-        }
-    };
-
     for (libs) |lib| {
         if (try Dylib.createAndParseFromPath(
             self.allocator,
@@ -309,8 +298,9 @@ fn parseLibs(self: *Zld, libs: []const []const u8) !void {
             lib,
             self.syslibroot,
             true,
-        )) |dylib| {
-            try self.dylibs.append(self.allocator, dylib);
+        )) |dylibs| {
+            defer self.allocator.free(dylibs);
+            try self.dylibs.appendSlice(self.allocator, dylibs);
             continue;
         }
 
@@ -321,26 +311,21 @@ fn parseLibs(self: *Zld, libs: []const []const u8) !void {
 
         log.warn("unknown filetype for a library: '{s}'", .{lib});
     }
-
-    // Flatten out any parsed dependencies.
-    var deps = std.ArrayList(*Dylib).init(self.allocator);
-    defer deps.deinit();
-
-    for (self.dylibs.items) |dylib| {
-        try DylibDeps.bubbleUp(&deps, dylib);
-    }
-
-    try self.dylibs.appendSlice(self.allocator, deps.toOwnedSlice());
 }
 
 fn parseLibSystem(self: *Zld, libc_stub_path: []const u8) !void {
-    const dylib = (try Dylib.createAndParseFromPath(
+    const dylibs = (try Dylib.createAndParseFromPath(
         self.allocator,
         self.arch.?,
         libc_stub_path,
         self.syslibroot,
         false,
     )) orelse return error.FailedToParseLibSystem;
+    defer self.allocator.free(dylibs);
+
+    assert(dylibs.len == 1); // More than one dylib output from parsing libSystem!
+    const dylib = dylibs[0];
+
     self.libsystem_dylib_index = @intCast(u16, self.dylibs.items.len);
     try self.dylibs.append(self.allocator, dylib);