Commit 20ea44ef10

Jakub Konka <kubkon@jakubkonka.com>
2022-07-25 18:26:06
macho: fix memory leak and refactor Target usage
1 parent cff5d9c
Changed files (5)
src/link/MachO/Archive.zig
@@ -103,9 +103,9 @@ pub fn deinit(self: *Archive, allocator: Allocator) void {
     allocator.free(self.name);
 }
 
-pub fn parse(self: *Archive, allocator: Allocator, target: std.Target) !void {
+pub fn parse(self: *Archive, allocator: Allocator, cpu_arch: std.Target.Cpu.Arch) !void {
     const reader = self.file.reader();
-    self.library_offset = try fat.getLibraryOffset(reader, target);
+    self.library_offset = try fat.getLibraryOffset(reader, cpu_arch);
     try self.file.seekTo(self.library_offset);
 
     const magic = try reader.readBytesNoEof(SARMAG);
@@ -187,7 +187,7 @@ fn parseTableOfContents(self: *Archive, allocator: Allocator, reader: anytype) !
     }
 }
 
-pub fn parseObject(self: Archive, allocator: Allocator, target: std.Target, offset: u32) !Object {
+pub fn parseObject(self: Archive, allocator: Allocator, cpu_arch: std.Target.Cpu.Arch, offset: u32) !Object {
     const reader = self.file.reader();
     try reader.context.seekTo(offset + self.library_offset);
 
@@ -216,7 +216,7 @@ pub fn parseObject(self: Archive, allocator: Allocator, target: std.Target, offs
         .mtime = try self.header.?.date(),
     };
 
-    try object.parse(allocator, target);
+    try object.parse(allocator, cpu_arch);
     try reader.context.seekTo(0);
 
     return object;
src/link/MachO/Dylib.zig
@@ -11,6 +11,7 @@ const mem = std.mem;
 const fat = @import("fat.zig");
 
 const Allocator = mem.Allocator;
+const CrossTarget = std.zig.CrossTarget;
 const LibStub = @import("../tapi.zig").LibStub;
 const MachO = @import("../MachO.zig");
 
@@ -145,13 +146,13 @@ pub fn deinit(self: *Dylib, allocator: Allocator) void {
 pub fn parse(
     self: *Dylib,
     allocator: Allocator,
-    target: std.Target,
+    cpu_arch: std.Target.Cpu.Arch,
     dylib_id: u16,
     dependent_libs: anytype,
 ) !void {
     log.debug("parsing shared library '{s}'", .{self.name});
 
-    self.library_offset = try fat.getLibraryOffset(self.file.reader(), target);
+    self.library_offset = try fat.getLibraryOffset(self.file.reader(), cpu_arch);
 
     try self.file.seekTo(self.library_offset);
 
@@ -165,8 +166,8 @@ pub fn parse(
 
     const this_arch: std.Target.Cpu.Arch = try fat.decodeArch(self.header.?.cputype, true);
 
-    if (this_arch != target.cpu.arch) {
-        log.err("mismatched cpu architecture: expected {s}, found {s}", .{ target.cpu.arch, this_arch });
+    if (this_arch != cpu_arch) {
+        log.err("mismatched cpu architecture: expected {s}, found {s}", .{ cpu_arch, this_arch });
         return error.MismatchedCpuArchitecture;
     }
 
@@ -278,23 +279,24 @@ fn addSymbol(self: *Dylib, allocator: Allocator, sym_name: []const u8) !void {
 
 const TargetMatcher = struct {
     allocator: Allocator,
-    target: std.Target,
+    target: CrossTarget,
     target_strings: std.ArrayListUnmanaged([]const u8) = .{},
 
-    fn init(allocator: Allocator, target: std.Target) !TargetMatcher {
+    fn init(allocator: Allocator, target: CrossTarget) !TargetMatcher {
         var self = TargetMatcher{
             .allocator = allocator,
             .target = target,
         };
         try self.target_strings.append(allocator, try targetToAppleString(allocator, target));
 
-        if (target.abi == .simulator) {
+        const abi = target.abi orelse .none;
+        if (abi == .simulator) {
             // For Apple simulator targets, linking gets tricky as we need to link against the simulator
             // hosts dylibs too.
-            const host_target = try targetToAppleString(allocator, (std.zig.CrossTarget{
-                .cpu_arch = target.cpu.arch,
+            const host_target = try targetToAppleString(allocator, .{
+                .cpu_arch = target.cpu_arch.?,
                 .os_tag = .macos,
-            }).toTarget());
+            });
             try self.target_strings.append(allocator, host_target);
         }
 
@@ -308,23 +310,24 @@ const TargetMatcher = struct {
         self.target_strings.deinit(self.allocator);
     }
 
-    fn targetToAppleString(allocator: Allocator, target: std.Target) ![]const u8 {
-        const arch = switch (target.cpu.arch) {
+    fn targetToAppleString(allocator: Allocator, target: CrossTarget) ![]const u8 {
+        const cpu_arch = switch (target.cpu_arch.?) {
             .aarch64 => "arm64",
             .x86_64 => "x86_64",
             else => unreachable,
         };
-        const os = @tagName(target.os.tag);
-        const abi: ?[]const u8 = switch (target.abi) {
+        const os_tag = @tagName(target.os_tag.?);
+        const target_abi = target.abi orelse .none;
+        const abi: ?[]const u8 = switch (target_abi) {
             .none => null,
             .simulator => "simulator",
             .macabi => "maccatalyst",
             else => unreachable,
         };
         if (abi) |x| {
-            return std.fmt.allocPrint(allocator, "{s}-{s}-{s}", .{ arch, os, x });
+            return std.fmt.allocPrint(allocator, "{s}-{s}-{s}", .{ cpu_arch, os_tag, x });
         }
-        return std.fmt.allocPrint(allocator, "{s}-{s}", .{ arch, os });
+        return std.fmt.allocPrint(allocator, "{s}-{s}", .{ cpu_arch, os_tag });
     }
 
     fn hasValue(stack: []const []const u8, needle: []const u8) bool {
@@ -342,7 +345,7 @@ const TargetMatcher = struct {
     }
 
     fn matchesArch(self: TargetMatcher, archs: []const []const u8) bool {
-        return hasValue(archs, @tagName(self.target.cpu.arch));
+        return hasValue(archs, @tagName(self.target.cpu_arch.?));
     }
 };
 
@@ -376,7 +379,11 @@ pub fn parseFromStub(
 
     log.debug("  (install_name '{s}')", .{umbrella_lib.installName()});
 
-    var matcher = try TargetMatcher.init(allocator, target);
+    var matcher = try TargetMatcher.init(allocator, .{
+        .cpu_arch = target.cpu.arch,
+        .os_tag = target.os.tag,
+        .abi = target.abi,
+    });
     defer matcher.deinit();
 
     for (lib_stub.inner) |elem, stub_index| {
src/link/MachO/fat.zig
@@ -6,7 +6,7 @@ const mem = std.mem;
 const native_endian = builtin.target.cpu.arch.endian();
 
 pub fn decodeArch(cputype: macho.cpu_type_t, comptime logError: bool) !std.Target.Cpu.Arch {
-    const arch: std.Target.Cpu.Arch = switch (cputype) {
+    const cpu_arch: std.Target.Cpu.Arch = switch (cputype) {
         macho.CPU_TYPE_ARM64 => .aarch64,
         macho.CPU_TYPE_X86_64 => .x86_64,
         else => {
@@ -16,7 +16,7 @@ pub fn decodeArch(cputype: macho.cpu_type_t, comptime logError: bool) !std.Targe
             return error.UnsupportedCpuArchitecture;
         },
     };
-    return arch;
+    return cpu_arch;
 }
 
 fn readFatStruct(reader: anytype, comptime T: type) !T {
@@ -29,7 +29,7 @@ fn readFatStruct(reader: anytype, comptime T: type) !T {
     return res;
 }
 
-pub fn getLibraryOffset(reader: anytype, target: std.Target) !u64 {
+pub fn getLibraryOffset(reader: anytype, cpu_arch: std.Target.Cpu.Arch) !u64 {
     const fat_header = try readFatStruct(reader, macho.fat_header);
     if (fat_header.magic != macho.FAT_MAGIC) return 0;
 
@@ -41,12 +41,12 @@ pub fn getLibraryOffset(reader: anytype, target: std.Target) !u64 {
         const lib_arch = decodeArch(fat_arch.cputype, false) catch |err| switch (err) {
             error.UnsupportedCpuArchitecture => continue,
         };
-        if (lib_arch == target.cpu.arch) {
+        if (lib_arch == cpu_arch) {
             // We have found a matching architecture!
             return fat_arch.offset;
         }
     } else {
-        log.err("Could not find matching cpu architecture in fat library: expected {s}", .{target.cpu.arch});
+        log.err("Could not find matching cpu architecture in fat library: expected {s}", .{cpu_arch});
         return error.MismatchedCpuArchitecture;
     }
 }
src/link/MachO/Object.zig
@@ -66,6 +66,7 @@ pub fn deinit(self: *Object, gpa: Allocator) void {
     }
     self.load_commands.deinit(gpa);
     gpa.free(self.contents);
+    self.symtab.deinit(gpa);
     self.sections_as_symbols.deinit(gpa);
     self.atom_by_index_table.deinit(gpa);
 
@@ -78,7 +79,7 @@ pub fn deinit(self: *Object, gpa: Allocator) void {
     gpa.free(self.name);
 }
 
-pub fn parse(self: *Object, allocator: Allocator, target: std.Target) !void {
+pub fn parse(self: *Object, allocator: Allocator, cpu_arch: std.Target.Cpu.Arch) !void {
     const file_stat = try self.file.stat();
     const file_size = math.cast(usize, file_stat.size) orelse return error.Overflow;
     self.contents = try self.file.readToEndAlloc(allocator, file_size);
@@ -108,8 +109,8 @@ pub fn parse(self: *Object, allocator: Allocator, target: std.Target) !void {
             return error.UnsupportedCpuArchitecture;
         },
     };
-    if (this_arch != target.cpu.arch) {
-        log.err("mismatched cpu architecture: expected {s}, found {s}", .{ target.cpu.arch, this_arch });
+    if (this_arch != cpu_arch) {
+        log.err("mismatched cpu architecture: expected {s}, found {s}", .{ cpu_arch, this_arch });
         return error.MismatchedCpuArchitecture;
     }
 
@@ -351,7 +352,7 @@ pub fn splitIntoAtomsOneShot(self: *Object, macho_file: *MachO, object_id: u32)
             macho_file.getSection(match).sectName(),
         });
 
-        const arch = macho_file.base.options.target.cpu.arch;
+        const cpu_arch = macho_file.base.options.target.cpu.arch;
         const is_zerofill = blk: {
             const section_type = sect.type_();
             break :blk section_type == macho.S_ZEROFILL or section_type == macho.S_THREAD_LOCAL_ZEROFILL;
@@ -466,7 +467,7 @@ pub fn splitIntoAtomsOneShot(self: *Object, macho_file: *MachO, object_id: u32)
                     sect,
                 );
 
-                if (arch == .x86_64 and addr == sect.addr) {
+                if (cpu_arch == .x86_64 and addr == sect.addr) {
                     // In x86_64 relocs, it can so happen that the compiler refers to the same
                     // atom by both the actual assigned symbol and the start of the section. In this
                     // case, we need to link the two together so add an alias.
src/link/MachO.zig
@@ -1415,7 +1415,7 @@ fn parseObject(self: *MachO, path: []const u8) !bool {
         .mtime = mtime,
     };
 
-    object.parse(self.base.allocator, self.base.options.target) catch |err| switch (err) {
+    object.parse(self.base.allocator, self.base.options.target.cpu.arch) catch |err| switch (err) {
         error.EndOfStream, error.NotObject => {
             object.deinit(self.base.allocator);
             return false;
@@ -1443,7 +1443,7 @@ fn parseArchive(self: *MachO, path: []const u8, force_load: bool) !bool {
         .file = file,
     };
 
-    archive.parse(self.base.allocator, self.base.options.target) catch |err| switch (err) {
+    archive.parse(self.base.allocator, self.base.options.target.cpu.arch) catch |err| switch (err) {
         error.EndOfStream, error.NotArchive => {
             archive.deinit(self.base.allocator);
             return false;
@@ -1463,7 +1463,11 @@ fn parseArchive(self: *MachO, path: []const u8, force_load: bool) !bool {
         }
         for (offsets.keys()) |off| {
             const object = try self.objects.addOne(self.base.allocator);
-            object.* = try archive.parseObject(self.base.allocator, self.base.options.target, off);
+            object.* = try archive.parseObject(
+                self.base.allocator,
+                self.base.options.target.cpu.arch,
+                off,
+            );
         }
     } else {
         try self.archives.append(self.base.allocator, archive);
@@ -1511,7 +1515,7 @@ pub fn parseDylib(
 
     dylib.parse(
         self.base.allocator,
-        self.base.options.target,
+        self.base.options.target.cpu.arch,
         dylib_id,
         dependent_libs,
     ) catch |err| switch (err) {
@@ -3126,7 +3130,11 @@ fn resolveSymbolsInArchives(self: *MachO) !void {
 
             const object_id = @intCast(u16, self.objects.items.len);
             const object = try self.objects.addOne(self.base.allocator);
-            object.* = try archive.parseObject(self.base.allocator, self.base.options.target, offsets.items[0]);
+            object.* = try archive.parseObject(
+                self.base.allocator,
+                self.base.options.target.cpu.arch,
+                offsets.items[0],
+            );
             try self.resolveSymbolsInObject(object, object_id);
 
             continue :loop;