Commit 06396ddd7d

Jakub Konka <kubkon@jakubkonka.com>
2021-07-31 15:01:37
macho: don't allocate Objects on the heap
instead, ownership is transferred to MachO. This makes Object management align closer with data-oriented design.
1 parent e737773
Changed files (3)
src/link/MachO/Archive.zig
@@ -223,8 +223,7 @@ fn parseTableOfContents(self: *Archive, allocator: *Allocator, reader: anytype)
     }
 }
 
-/// Caller owns the Object instance.
-pub fn parseObject(self: Archive, allocator: *Allocator, arch: Arch, offset: u32) !*Object {
+pub fn extractObject(self: Archive, allocator: *Allocator, offset: u32) !Object {
     var reader = self.file.reader();
     try reader.context.seekTo(offset + self.library_offset);
 
@@ -246,16 +245,13 @@ pub fn parseObject(self: Archive, allocator: *Allocator, arch: Arch, offset: u32
         break :name try std.fmt.allocPrint(allocator, "{s}({s})", .{ path, object_name });
     };
 
-    var object = try allocator.create(Object);
-    errdefer allocator.destroy(object);
-
-    object.* = .{
+    var object = Object{
         .file = try fs.cwd().openFile(self.name, .{}),
         .name = name,
         .file_offset = @intCast(u32, try reader.context.getPos()),
         .mtime = try self.header.?.date(),
     };
-    try object.parse(allocator, arch);
+
     try reader.context.seekTo(0);
 
     return object;
src/link/MachO/Object.zig
@@ -127,36 +127,6 @@ const DebugInfo = struct {
     }
 };
 
-pub fn createAndParseFromPath(allocator: *Allocator, arch: Arch, path: []const u8) !?*Object {
-    const file = fs.cwd().openFile(path, .{}) catch |err| switch (err) {
-        error.FileNotFound => return null,
-        else => |e| return e,
-    };
-    errdefer file.close();
-
-    const object = try allocator.create(Object);
-    errdefer allocator.destroy(object);
-
-    const name = try allocator.dupe(u8, path);
-    errdefer allocator.free(name);
-
-    object.* = .{
-        .name = name,
-        .file = file,
-    };
-
-    object.parse(allocator, arch) catch |err| switch (err) {
-        error.EndOfStream, error.NotObject => {
-            object.deinit(allocator);
-            allocator.destroy(object);
-            return null;
-        },
-        else => |e| return e,
-    };
-
-    return object;
-}
-
 pub fn deinit(self: *Object, allocator: *Allocator) void {
     for (self.load_commands.items) |*lc| {
         lc.deinit(allocator);
@@ -184,6 +154,22 @@ 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,
+            }
+        }
+    };
+    try file.seekTo(0);
+    return is_object;
+}
+
 pub fn parse(self: *Object, allocator: *Allocator, arch: Arch) !void {
     var reader = self.file.reader();
     if (self.file_offset) |offset| {
@@ -481,7 +467,12 @@ const TextBlockParser = struct {
     }
 };
 
-pub fn parseTextBlocks(self: *Object, allocator: *Allocator, macho_file: *MachO) !void {
+pub fn parseTextBlocks(
+    self: *Object,
+    allocator: *Allocator,
+    object_id: u16,
+    macho_file: *MachO,
+) !void {
     const seg = self.load_commands.items[self.segment_cmd_index.?].Segment;
 
     log.debug("analysing {s}", .{self.name});
@@ -668,13 +659,14 @@ pub fn parseTextBlocks(self: *Object, allocator: *Allocator, macho_file: *MachO)
                     if (is_ext) {
                         if (macho_file.symbol_resolver.get(sym.n_strx)) |resolv| {
                             assert(resolv.where == .global);
-                            const global_object = macho_file.objects.items[resolv.file];
-                            if (global_object != self) {
+                            if (resolv.file != object_id) {
                                 log.debug("deduping definition of {s} in {s}", .{
                                     macho_file.getString(sym.n_strx),
                                     self.name,
                                 });
-                                log.debug("  already defined in {s}", .{global_object.name});
+                                log.debug("  already defined in {s}", .{
+                                    macho_file.objects.items[resolv.file].name,
+                                });
                                 continue;
                             }
                         }
src/link/MachO.zig
@@ -61,7 +61,7 @@ header_pad: u16 = 0x1000,
 /// The absolute address of the entry point.
 entry_addr: ?u64 = null,
 
-objects: std.ArrayListUnmanaged(*Object) = .{},
+objects: std.ArrayListUnmanaged(Object) = .{},
 archives: std.ArrayListUnmanaged(*Archive) = .{},
 dylibs: std.ArrayListUnmanaged(*Dylib) = .{},
 
@@ -990,13 +990,19 @@ fn parseInputFiles(self: *MachO, files: []const []const u8, syslibroot: ?[]const
     const arch = self.base.options.target.cpu.arch;
     for (files) |file_name| {
         const full_path = full_path: {
-            var buffer: [std.fs.MAX_PATH_BYTES]u8 = undefined;
+            var buffer: [fs.MAX_PATH_BYTES]u8 = undefined;
             const path = try std.fs.realpath(file_name, &buffer);
             break :full_path try self.base.allocator.dupe(u8, path);
         };
+        const file = try fs.cwd().openFile(full_path, .{});
 
-        if (try Object.createAndParseFromPath(self.base.allocator, arch, full_path)) |object| {
-            try self.objects.append(self.base.allocator, object);
+        if (try Object.isObject(file)) {
+            const object = try self.objects.addOne(self.base.allocator);
+            object.* = .{
+                .name = full_path,
+                .file = file,
+            };
+            try object.parse(self.base.allocator, arch);
             continue;
         }
 
@@ -1993,7 +1999,7 @@ fn writeStubHelperCommon(self: *MachO) !void {
 }
 
 fn resolveSymbolsInObject(self: *MachO, object_id: u16) !void {
-    const object = self.objects.items[object_id];
+    const object = &self.objects.items[object_id];
 
     log.debug("resolving symbols in '{s}'", .{object.name});
 
@@ -2228,13 +2234,10 @@ fn resolveSymbols(self: *MachO) !void {
             };
             assert(offsets.items.len > 0);
 
-            const object = try archive.parseObject(
-                self.base.allocator,
-                self.base.options.target.cpu.arch,
-                offsets.items[0],
-            );
             const object_id = @intCast(u16, self.objects.items.len);
-            try self.objects.append(self.base.allocator, object);
+            const object = try self.objects.addOne(self.base.allocator);
+            object.* = try archive.extractObject(self.base.allocator, offsets.items[0]);
+            try object.parse(self.base.allocator, self.base.options.target.cpu.arch);
             try self.resolveSymbolsInObject(object_id);
 
             continue :loop;
@@ -2460,8 +2463,8 @@ fn resolveSymbols(self: *MachO) !void {
 }
 
 fn parseTextBlocks(self: *MachO) !void {
-    for (self.objects.items) |object| {
-        try object.parseTextBlocks(self.base.allocator, self);
+    for (self.objects.items) |*object, object_id| {
+        try object.parseTextBlocks(self.base.allocator, @intCast(u16, object_id), self);
     }
 }
 
@@ -3337,9 +3340,8 @@ pub fn deinit(self: *MachO) void {
     self.locals_free_list.deinit(self.base.allocator);
     self.symbol_resolver.deinit(self.base.allocator);
 
-    for (self.objects.items) |object| {
+    for (self.objects.items) |*object| {
         object.deinit(self.base.allocator);
-        self.base.allocator.destroy(object);
     }
     self.objects.deinit(self.base.allocator);