Commit f023cdad7c

Jakub Konka <kubkon@jakubkonka.com>
2021-07-31 15:33:39
macho: don't allocate Archives on the heap
instead, transfer ownership directly to MachO struct.
1 parent 06396dd
Changed files (3)
src/link/MachO/Archive.zig
@@ -93,36 +93,6 @@ const ar_hdr = extern struct {
     }
 };
 
-pub fn createAndParseFromPath(allocator: *Allocator, arch: Arch, path: []const u8) !?*Archive {
-    const file = fs.cwd().openFile(path, .{}) catch |err| switch (err) {
-        error.FileNotFound => return null,
-        else => |e| return e,
-    };
-    errdefer file.close();
-
-    const archive = try allocator.create(Archive);
-    errdefer allocator.destroy(archive);
-
-    const name = try allocator.dupe(u8, path);
-    errdefer allocator.free(name);
-
-    archive.* = .{
-        .name = name,
-        .file = file,
-    };
-
-    archive.parse(allocator, arch) catch |err| switch (err) {
-        error.EndOfStream, error.NotArchive => {
-            archive.deinit(allocator);
-            allocator.destroy(archive);
-            return null;
-        },
-        else => |e| return e,
-    };
-
-    return archive;
-}
-
 pub fn deinit(self: *Archive, allocator: *Allocator) void {
     for (self.toc.keys()) |*key| {
         allocator.free(key.*);
@@ -134,32 +104,40 @@ pub fn deinit(self: *Archive, allocator: *Allocator) void {
     allocator.free(self.name);
 }
 
+pub fn isArchive(file: fs.File, arch: Arch) !bool {
+    const Internal = struct {
+        fn isArchive(reader: anytype, a: Arch) !bool {
+            const offset = try fat.getLibraryOffset(reader, a);
+            try reader.context.seekTo(offset);
+            const magic = try reader.readBytesNoEof(SARMAG);
+            if (!mem.eql(u8, &magic, ARMAG)) return false;
+            const header = try reader.readStruct(ar_hdr);
+            return mem.eql(u8, &header.ar_fmag, ARFMAG);
+        }
+    };
+    const is_archive = if (Internal.isArchive(file.reader(), arch)) |res|
+        res
+    else |err| switch (err) {
+        error.EndOfStream => false,
+        error.MismatchedCpuArchitecture => true, // TODO maybe this check should be done differently?
+        else => |e| return e,
+    };
+    try file.seekTo(0);
+    return is_archive;
+}
+
 pub fn parse(self: *Archive, allocator: *Allocator, arch: Arch) !void {
     self.library_offset = try fat.getLibraryOffset(self.file.reader(), arch);
-
     try self.file.seekTo(self.library_offset);
-
-    var reader = self.file.reader();
+    const reader = self.file.reader();
     const magic = try reader.readBytesNoEof(SARMAG);
-
-    if (!mem.eql(u8, &magic, ARMAG)) {
-        log.debug("invalid magic: expected '{s}', found '{s}'", .{ ARMAG, magic });
-        return error.NotArchive;
-    }
-
     self.header = try reader.readStruct(ar_hdr);
-
-    if (!mem.eql(u8, &self.header.?.ar_fmag, ARFMAG)) {
-        log.debug("invalid header delimiter: expected '{s}', found '{s}'", .{ ARFMAG, self.header.?.ar_fmag });
-        return error.NotArchive;
-    }
-
     var embedded_name = try parseName(allocator, self.header.?, reader);
-    log.debug("parsing archive '{s}' at '{s}'", .{ embedded_name, self.name });
     defer allocator.free(embedded_name);
 
-    try self.parseTableOfContents(allocator, reader);
+    log.debug("parsing archive '{s}' at '{s}'", .{ embedded_name, self.name });
 
+    try self.parseTableOfContents(allocator, reader);
     try reader.context.seekTo(0);
 }
 
src/link/MachO/Object.zig
@@ -155,17 +155,18 @@ pub fn deinit(self: *Object, allocator: *Allocator) void {
 }
 
 pub fn isObject(file: fs.File) !bool {
-    const reader = file.reader();
-    const is_object = blk: {
-        if (reader.readStruct(macho.mach_header_64)) |header| {
-            break :blk header.filetype == macho.MH_OBJECT;
-        } else |err| {
-            switch (err) {
-                error.EndOfStream => break :blk false,
-                else => |e| return e,
-            }
+    const Internal = struct {
+        fn isObject(reader: anytype) !bool {
+            const header = try reader.readStruct(macho.mach_header_64);
+            return header.filetype == macho.MH_OBJECT;
         }
     };
+    const is_object = if (Internal.isObject(file.reader())) |res|
+        res
+    else |err| switch (err) {
+        error.EndOfStream => false,
+        else => |e| return e,
+    };
     try file.seekTo(0);
     return is_object;
 }
@@ -175,14 +176,7 @@ pub fn parse(self: *Object, allocator: *Allocator, arch: Arch) !void {
     if (self.file_offset) |offset| {
         try reader.context.seekTo(offset);
     }
-
     const header = try reader.readStruct(macho.mach_header_64);
-
-    if (header.filetype != macho.MH_OBJECT) {
-        log.debug("invalid filetype: expected 0x{x}, found 0x{x}", .{ macho.MH_OBJECT, header.filetype });
-        return error.NotObject;
-    }
-
     const this_arch: Arch = switch (header.cputype) {
         macho.CPU_TYPE_ARM64 => .aarch64,
         macho.CPU_TYPE_X86_64 => .x86_64,
@@ -195,7 +189,6 @@ pub fn parse(self: *Object, allocator: *Allocator, arch: Arch) !void {
         log.err("mismatched cpu architecture: expected {s}, found {s}", .{ arch, this_arch });
         return error.MismatchedCpuArchitecture;
     }
-
     self.header = header;
 
     try self.readLoadCommands(allocator, reader);
src/link/MachO.zig
@@ -62,7 +62,7 @@ header_pad: u16 = 0x1000,
 entry_addr: ?u64 = null,
 
 objects: std.ArrayListUnmanaged(Object) = .{},
-archives: std.ArrayListUnmanaged(*Archive) = .{},
+archives: std.ArrayListUnmanaged(Archive) = .{},
 dylibs: std.ArrayListUnmanaged(*Dylib) = .{},
 
 next_dylib_ordinal: u16 = 1,
@@ -1006,8 +1006,13 @@ fn parseInputFiles(self: *MachO, files: []const []const u8, syslibroot: ?[]const
             continue;
         }
 
-        if (try Archive.createAndParseFromPath(self.base.allocator, arch, full_path)) |archive| {
-            try self.archives.append(self.base.allocator, archive);
+        if (try Archive.isArchive(file, arch)) {
+            const archive = try self.archives.addOne(self.base.allocator);
+            archive.* = .{
+                .name = full_path,
+                .file = file,
+            };
+            try archive.parse(self.base.allocator, arch);
             continue;
         }
 
@@ -1019,6 +1024,8 @@ fn parseInputFiles(self: *MachO, files: []const []const u8, syslibroot: ?[]const
             continue;
         }
 
+        self.base.allocator.free(full_path);
+        file.close();
         log.warn("unknown filetype for positional input file: '{s}'", .{file_name});
     }
 }
@@ -1026,6 +1033,8 @@ fn parseInputFiles(self: *MachO, files: []const []const u8, syslibroot: ?[]const
 fn parseLibs(self: *MachO, libs: []const []const u8, syslibroot: ?[]const u8) !void {
     const arch = self.base.options.target.cpu.arch;
     for (libs) |lib| {
+        const file = try fs.cwd().openFile(lib, .{});
+
         if (try Dylib.createAndParseFromPath(self.base.allocator, arch, lib, .{
             .syslibroot = syslibroot,
         })) |dylibs| {
@@ -1034,11 +1043,17 @@ fn parseLibs(self: *MachO, libs: []const []const u8, syslibroot: ?[]const u8) !v
             continue;
         }
 
-        if (try Archive.createAndParseFromPath(self.base.allocator, arch, lib)) |archive| {
-            try self.archives.append(self.base.allocator, archive);
+        if (try Archive.isArchive(file, arch)) {
+            const archive = try self.archives.addOne(self.base.allocator);
+            archive.* = .{
+                .name = try self.base.allocator.dupe(u8, lib),
+                .file = file,
+            };
+            try archive.parse(self.base.allocator, arch);
             continue;
         }
 
+        file.close();
         log.warn("unknown filetype for a library: '{s}'", .{lib});
     }
 }
@@ -3345,9 +3360,8 @@ pub fn deinit(self: *MachO) void {
     }
     self.objects.deinit(self.base.allocator);
 
-    for (self.archives.items) |archive| {
+    for (self.archives.items) |*archive| {
         archive.deinit(self.base.allocator);
-        self.base.allocator.destroy(archive);
     }
     self.archives.deinit(self.base.allocator);