Commit e73777333d

Jakub Konka <kubkon@jakubkonka.com>
2021-07-31 10:59:22
macho: don't store allocator in Dylib instance
instead pass it in as an arg to a function that requires it.
1 parent 586a19e
Changed files (2)
src
src/link/MachO/Dylib.zig
@@ -9,20 +9,18 @@ const macho = std.macho;
 const math = std.math;
 const mem = std.mem;
 const fat = @import("fat.zig");
+const commands = @import("commands.zig");
 
 const Allocator = mem.Allocator;
 const Arch = std.Target.Cpu.Arch;
 const LibStub = @import("../tapi.zig").LibStub;
+const LoadCommand = commands.LoadCommand;
 const MachO = @import("../MachO.zig");
 
-usingnamespace @import("commands.zig");
+file: fs.File,
+name: []const u8,
 
-allocator: *Allocator,
-arch: ?Arch = null,
 header: ?macho.mach_header_64 = null,
-file: ?fs.File = null,
-name: ?[]const u8 = null,
-syslibroot: ?[]const u8 = null,
 
 ordinal: ?u16 = null,
 
@@ -61,7 +59,7 @@ pub const Id = struct {
         };
     }
 
-    pub fn fromLoadCommand(allocator: *Allocator, lc: GenericCommandWithData(macho.dylib_command)) !Id {
+    pub fn fromLoadCommand(allocator: *Allocator, lc: commands.GenericCommandWithData(macho.dylib_command)) !Id {
         const dylib = lc.inner.dylib;
         const dylib_name = @ptrCast([*:0]const u8, lc.data[dylib.name - @sizeOf(macho.dylib_command) ..]);
         const name = try allocator.dupe(u8, mem.spanZ(dylib_name));
@@ -164,25 +162,22 @@ pub fn createAndParseFromPath(
     errdefer allocator.free(name);
 
     dylib.* = .{
-        .allocator = allocator,
-        .arch = arch,
         .name = name,
         .file = file,
-        .syslibroot = opts.syslibroot,
     };
 
-    dylib.parse() catch |err| switch (err) {
+    dylib.parse(allocator, arch) catch |err| switch (err) {
         error.EndOfStream, error.NotDylib => {
             try file.seekTo(0);
 
             var lib_stub = LibStub.loadFromFile(allocator, file) catch {
-                dylib.deinit();
+                dylib.deinit(allocator);
                 allocator.destroy(dylib);
                 return null;
             };
             defer lib_stub.deinit();
 
-            try dylib.parseFromStub(lib_stub);
+            try dylib.parseFromStub(allocator, arch, lib_stub);
         },
         else => |e| return e,
     };
@@ -195,7 +190,7 @@ pub fn createAndParseFromPath(
             log.warn("  | dylib version: {}", .{dylib.id.?.current_version});
 
             // TODO maybe this should be an error and facilitate auto-cleanup?
-            dylib.deinit();
+            dylib.deinit(allocator);
             allocator.destroy(dylib);
             return null;
         }
@@ -205,50 +200,41 @@ pub fn createAndParseFromPath(
     defer dylibs.deinit();
 
     try dylibs.append(dylib);
-    try dylib.parseDependentLibs(&dylibs);
+    try dylib.parseDependentLibs(allocator, arch, &dylibs, opts.syslibroot);
 
     return dylibs.toOwnedSlice();
 }
 
-pub fn deinit(self: *Dylib) void {
+pub fn deinit(self: *Dylib, allocator: *Allocator) void {
     for (self.load_commands.items) |*lc| {
-        lc.deinit(self.allocator);
+        lc.deinit(allocator);
     }
-    self.load_commands.deinit(self.allocator);
+    self.load_commands.deinit(allocator);
 
     for (self.symbols.keys()) |key| {
-        self.allocator.free(key);
+        allocator.free(key);
     }
-    self.symbols.deinit(self.allocator);
+    self.symbols.deinit(allocator);
 
     for (self.dependent_libs.items) |*id| {
-        id.deinit(self.allocator);
-    }
-    self.dependent_libs.deinit(self.allocator);
-
-    if (self.name) |name| {
-        self.allocator.free(name);
+        id.deinit(allocator);
     }
+    self.dependent_libs.deinit(allocator);
+    allocator.free(self.name);
 
     if (self.id) |*id| {
-        id.deinit(self.allocator);
+        id.deinit(allocator);
     }
 }
 
-pub fn closeFile(self: Dylib) void {
-    if (self.file) |file| {
-        file.close();
-    }
-}
+pub fn parse(self: *Dylib, allocator: *Allocator, arch: Arch) !void {
+    log.debug("parsing shared library '{s}'", .{self.name});
 
-pub fn parse(self: *Dylib) !void {
-    log.debug("parsing shared library '{s}'", .{self.name.?});
+    self.library_offset = try fat.getLibraryOffset(self.file.reader(), arch);
 
-    self.library_offset = try fat.getLibraryOffset(self.file.?.reader(), self.arch.?);
+    try self.file.seekTo(self.library_offset);
 
-    try self.file.?.seekTo(self.library_offset);
-
-    var reader = self.file.?.reader();
+    var reader = self.file.reader();
     self.header = try reader.readStruct(macho.mach_header_64);
 
     if (self.header.?.filetype != macho.MH_DYLIB) {
@@ -258,24 +244,24 @@ pub fn parse(self: *Dylib) !void {
 
     const this_arch: Arch = try fat.decodeArch(self.header.?.cputype, true);
 
-    if (this_arch != self.arch.?) {
-        log.err("mismatched cpu architecture: expected {s}, found {s}", .{ self.arch.?, this_arch });
+    if (this_arch != arch) {
+        log.err("mismatched cpu architecture: expected {s}, found {s}", .{ arch, this_arch });
         return error.MismatchedCpuArchitecture;
     }
 
-    try self.readLoadCommands(reader);
-    try self.parseId();
-    try self.parseSymbols();
+    try self.readLoadCommands(allocator, reader);
+    try self.parseId(allocator);
+    try self.parseSymbols(allocator);
 }
 
-fn readLoadCommands(self: *Dylib, reader: anytype) !void {
+fn readLoadCommands(self: *Dylib, allocator: *Allocator, reader: anytype) !void {
     const should_lookup_reexports = self.header.?.flags & macho.MH_NO_REEXPORTED_DYLIBS == 0;
 
-    try self.load_commands.ensureCapacity(self.allocator, self.header.?.ncmds);
+    try self.load_commands.ensureCapacity(allocator, self.header.?.ncmds);
 
     var i: u16 = 0;
     while (i < self.header.?.ncmds) : (i += 1) {
-        var cmd = try LoadCommand.read(self.allocator, reader);
+        var cmd = try LoadCommand.read(allocator, reader);
         switch (cmd.cmd()) {
             macho.LC_SYMTAB => {
                 self.symtab_cmd_index = i;
@@ -289,8 +275,8 @@ fn readLoadCommands(self: *Dylib, reader: anytype) !void {
             macho.LC_REEXPORT_DYLIB => {
                 if (should_lookup_reexports) {
                     // Parse install_name to dependent dylib.
-                    const id = try Id.fromLoadCommand(self.allocator, cmd.Dylib);
-                    try self.dependent_libs.append(self.allocator, id);
+                    const id = try Id.fromLoadCommand(allocator, cmd.Dylib);
+                    try self.dependent_libs.append(allocator, id);
                 }
             },
             else => {
@@ -301,27 +287,27 @@ fn readLoadCommands(self: *Dylib, reader: anytype) !void {
     }
 }
 
-fn parseId(self: *Dylib) !void {
+fn parseId(self: *Dylib, allocator: *Allocator) !void {
     const index = self.id_cmd_index orelse {
         log.debug("no LC_ID_DYLIB load command found; using hard-coded defaults...", .{});
-        self.id = try Id.default(self.allocator, self.name.?);
+        self.id = try Id.default(allocator, self.name);
         return;
     };
-    self.id = try Id.fromLoadCommand(self.allocator, self.load_commands.items[index].Dylib);
+    self.id = try Id.fromLoadCommand(allocator, self.load_commands.items[index].Dylib);
 }
 
-fn parseSymbols(self: *Dylib) !void {
+fn parseSymbols(self: *Dylib, allocator: *Allocator) !void {
     const index = self.symtab_cmd_index orelse return;
     const symtab_cmd = self.load_commands.items[index].Symtab;
 
-    var symtab = try self.allocator.alloc(u8, @sizeOf(macho.nlist_64) * symtab_cmd.nsyms);
-    defer self.allocator.free(symtab);
-    _ = try self.file.?.preadAll(symtab, symtab_cmd.symoff + self.library_offset);
+    var symtab = try allocator.alloc(u8, @sizeOf(macho.nlist_64) * symtab_cmd.nsyms);
+    defer allocator.free(symtab);
+    _ = try self.file.preadAll(symtab, symtab_cmd.symoff + self.library_offset);
     const slice = @alignCast(@alignOf(macho.nlist_64), mem.bytesAsSlice(macho.nlist_64, symtab));
 
-    var strtab = try self.allocator.alloc(u8, symtab_cmd.strsize);
-    defer self.allocator.free(strtab);
-    _ = try self.file.?.preadAll(strtab, symtab_cmd.stroff + self.library_offset);
+    var strtab = try allocator.alloc(u8, symtab_cmd.strsize);
+    defer allocator.free(strtab);
+    _ = try self.file.preadAll(strtab, symtab_cmd.stroff + self.library_offset);
 
     for (slice) |sym| {
         const add_to_symtab = MachO.symbolIsExt(sym) and (MachO.symbolIsSect(sym) or MachO.symbolIsIndr(sym));
@@ -329,8 +315,8 @@ fn parseSymbols(self: *Dylib) !void {
         if (!add_to_symtab) continue;
 
         const sym_name = mem.spanZ(@ptrCast([*:0]const u8, strtab.ptr + sym.n_strx));
-        const name = try self.allocator.dupe(u8, sym_name);
-        try self.symbols.putNoClobber(self.allocator, name, {});
+        const name = try allocator.dupe(u8, sym_name);
+        try self.symbols.putNoClobber(allocator, name, {});
     }
 }
 
@@ -341,26 +327,26 @@ fn hasTarget(targets: []const []const u8, target: []const u8) bool {
     return false;
 }
 
-fn addObjCClassSymbols(self: *Dylib, sym_name: []const u8) !void {
+fn addObjCClassSymbols(self: *Dylib, allocator: *Allocator, sym_name: []const u8) !void {
     const expanded = &[_][]const u8{
-        try std.fmt.allocPrint(self.allocator, "_OBJC_CLASS_$_{s}", .{sym_name}),
-        try std.fmt.allocPrint(self.allocator, "_OBJC_METACLASS_$_{s}", .{sym_name}),
+        try std.fmt.allocPrint(allocator, "_OBJC_CLASS_$_{s}", .{sym_name}),
+        try std.fmt.allocPrint(allocator, "_OBJC_METACLASS_$_{s}", .{sym_name}),
     };
 
     for (expanded) |sym| {
         if (self.symbols.contains(sym)) continue;
-        try self.symbols.putNoClobber(self.allocator, sym, .{});
+        try self.symbols.putNoClobber(allocator, sym, .{});
     }
 }
 
-pub fn parseFromStub(self: *Dylib, lib_stub: LibStub) !void {
+pub fn parseFromStub(self: *Dylib, allocator: *Allocator, arch: Arch, lib_stub: LibStub) !void {
     if (lib_stub.inner.len == 0) return error.EmptyStubFile;
 
-    log.debug("parsing shared library from stub '{s}'", .{self.name.?});
+    log.debug("parsing shared library from stub '{s}'", .{self.name});
 
     const umbrella_lib = lib_stub.inner[0];
 
-    var id = try Id.default(self.allocator, umbrella_lib.install_name);
+    var id = try Id.default(allocator, umbrella_lib.install_name);
     if (umbrella_lib.current_version) |version| {
         try id.parseCurrentVersion(version);
     }
@@ -369,13 +355,13 @@ pub fn parseFromStub(self: *Dylib, lib_stub: LibStub) !void {
     }
     self.id = id;
 
-    const target_string: []const u8 = switch (self.arch.?) {
+    const target_string: []const u8 = switch (arch) {
         .aarch64 => "arm64-macos",
         .x86_64 => "x86_64-macos",
         else => unreachable,
     };
 
-    var umbrella_libs = std.StringHashMap(void).init(self.allocator);
+    var umbrella_libs = std.StringHashMap(void).init(allocator);
     defer umbrella_libs.deinit();
 
     for (lib_stub.inner) |stub, stub_index| {
@@ -395,13 +381,13 @@ pub fn parseFromStub(self: *Dylib, lib_stub: LibStub) !void {
                 if (exp.symbols) |symbols| {
                     for (symbols) |sym_name| {
                         if (self.symbols.contains(sym_name)) continue;
-                        try self.symbols.putNoClobber(self.allocator, try self.allocator.dupe(u8, sym_name), {});
+                        try self.symbols.putNoClobber(allocator, try allocator.dupe(u8, sym_name), {});
                     }
                 }
 
                 if (exp.objc_classes) |classes| {
                     for (classes) |sym_name| {
-                        try self.addObjCClassSymbols(sym_name);
+                        try self.addObjCClassSymbols(allocator, sym_name);
                     }
                 }
             }
@@ -414,13 +400,13 @@ pub fn parseFromStub(self: *Dylib, lib_stub: LibStub) !void {
                 if (reexp.symbols) |symbols| {
                     for (symbols) |sym_name| {
                         if (self.symbols.contains(sym_name)) continue;
-                        try self.symbols.putNoClobber(self.allocator, try self.allocator.dupe(u8, sym_name), {});
+                        try self.symbols.putNoClobber(allocator, try allocator.dupe(u8, sym_name), {});
                     }
                 }
 
                 if (reexp.objc_classes) |classes| {
                     for (classes) |sym_name| {
-                        try self.addObjCClassSymbols(sym_name);
+                        try self.addObjCClassSymbols(allocator, sym_name);
                     }
                 }
             }
@@ -428,7 +414,7 @@ pub fn parseFromStub(self: *Dylib, lib_stub: LibStub) !void {
 
         if (stub.objc_classes) |classes| {
             for (classes) |sym_name| {
-                try self.addObjCClassSymbols(sym_name);
+                try self.addObjCClassSymbols(allocator, sym_name);
             }
         }
     }
@@ -451,15 +437,21 @@ pub fn parseFromStub(self: *Dylib, lib_stub: LibStub) !void {
 
                     log.debug("  | {s}", .{lib});
 
-                    const dep_id = try Id.default(self.allocator, lib);
-                    try self.dependent_libs.append(self.allocator, dep_id);
+                    const dep_id = try Id.default(allocator, lib);
+                    try self.dependent_libs.append(allocator, dep_id);
                 }
             }
         }
     }
 }
 
-pub fn parseDependentLibs(self: *Dylib, out: *std.ArrayList(*Dylib)) !void {
+pub fn parseDependentLibs(
+    self: *Dylib,
+    allocator: *Allocator,
+    arch: Arch,
+    out: *std.ArrayList(*Dylib),
+    syslibroot: ?[]const u8,
+) !void {
     outer: for (self.dependent_libs.items) |id| {
         const has_ext = blk: {
             const basename = fs.path.basename(id.name);
@@ -472,27 +464,27 @@ pub fn parseDependentLibs(self: *Dylib, out: *std.ArrayList(*Dylib)) !void {
         } else id.name;
 
         for (&[_][]const u8{ extension, ".tbd" }) |ext| {
-            const with_ext = try std.fmt.allocPrint(self.allocator, "{s}{s}", .{
+            const with_ext = try std.fmt.allocPrint(allocator, "{s}{s}", .{
                 without_ext,
                 ext,
             });
-            defer self.allocator.free(with_ext);
+            defer allocator.free(with_ext);
 
-            const full_path = if (self.syslibroot) |syslibroot|
-                try fs.path.join(self.allocator, &.{ syslibroot, with_ext })
+            const full_path = if (syslibroot) |root|
+                try fs.path.join(allocator, &.{ root, with_ext })
             else
                 with_ext;
-            defer if (self.syslibroot) |_| self.allocator.free(full_path);
+            defer if (syslibroot) |_| allocator.free(full_path);
 
             log.debug("trying dependency at fully resolved path {s}", .{full_path});
 
             const dylibs = (try createAndParseFromPath(
-                self.allocator,
-                self.arch.?,
+                allocator,
+                arch,
                 full_path,
                 .{
                     .id = id,
-                    .syslibroot = self.syslibroot,
+                    .syslibroot = syslibroot,
                 },
             )) orelse {
                 continue;
src/link/MachO.zig
@@ -3350,7 +3350,7 @@ pub fn deinit(self: *MachO) void {
     self.archives.deinit(self.base.allocator);
 
     for (self.dylibs.items) |dylib| {
-        dylib.deinit();
+        dylib.deinit(self.base.allocator);
         self.base.allocator.destroy(dylib);
     }
     self.dylibs.deinit(self.base.allocator);
@@ -3381,6 +3381,9 @@ pub fn closeFiles(self: MachO) void {
     for (self.archives.items) |archive| {
         archive.file.close();
     }
+    for (self.dylibs.items) |dylib| {
+        dylib.file.close();
+    }
 }
 
 fn freeTextBlock(self: *MachO, text_block: *TextBlock) void {