From 758c069a876388ffed717333f50991225cf1d890 Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Mon, 1 Jul 2024 10:40:49 +0200 Subject: [PATCH 1/4] macho: fix memory oopsies --- src/MachO/Object.zig | 121 +++++++++++++++++++++++++++---------------- src/tracy.zig | 3 +- 2 files changed, 76 insertions(+), 48 deletions(-) diff --git a/src/MachO/Object.zig b/src/MachO/Object.zig index e8126fd5..fbef069a 100644 --- a/src/MachO/Object.zig +++ b/src/MachO/Object.zig @@ -74,8 +74,7 @@ pub fn parse(self: *Object, macho_file: *MachO) !void { const file = macho_file.getFileHandle(self.file_handle); // Atom at index 0 is reserved as null atom - try self.atoms.append(gpa, .{}); - try self.atoms_extra.append(gpa, 0); + try self.atoms.append(gpa, .{ .extra = try self.addAtomExtra(gpa, .{}) }); var header_buffer: [@sizeOf(macho.mach_header_64)]u8 = undefined; { @@ -117,7 +116,7 @@ pub fn parse(self: *Object, macho_file: *MachO) !void { const sections = lc.getSections(); try self.sections.ensureUnusedCapacity(gpa, sections.len); for (sections) |sect| { - const index = try self.sections.addOne(gpa); + const index = self.sections.addOneAssumeCapacity(); self.sections.set(index, .{ .header = sect }); if (mem.eql(u8, sect.sectName(), "__eh_frame")) { @@ -430,8 +429,10 @@ fn initCstringLiterals(self: *Object, allocator: Allocator, file: File.Handle, m for (slice.items(.header), 0..) |sect, n_sect| { if (!isCstringLiteral(sect)) continue; - const data = try self.getSectionData(allocator, @intCast(n_sect), file); + const data = try allocator.alloc(u8, sect.size); defer allocator.free(data); + const amt = try file.preadAll(data, sect.offset + self.offset); + if (amt != data.len) return error.InputOutput; var count: u32 = 0; var start: u32 = 0; @@ -619,11 +620,23 @@ pub fn resolveLiterals(self: *Object, lp: *MachO.LiteralPool, macho_file: *MachO var buffer = std.ArrayList(u8).init(gpa); defer buffer.deinit(); + var sections_data = std.AutoHashMap(u32, []const u8).init(gpa); + try sections_data.ensureTotalCapacity(@intCast(self.sections.items(.header).len)); + defer { + var it = sections_data.iterator(); + while (it.next()) |entry| { + gpa.free(entry.value_ptr.*); + } + sections_data.deinit(); + } + const slice = self.sections.slice(); - for (slice.items(.header), slice.items(.subsections), 0..) |header, subs, n_sect| { + for (slice.items(.header), slice.items(.subsections)) |header, subs| { if (isCstringLiteral(header) or isFixedSizeLiteral(header)) { - const data = try self.getSectionData(gpa, @intCast(n_sect), file); + const data = try gpa.alloc(u8, header.size); defer gpa.free(data); + const amt = try file.preadAll(data, header.offset + self.offset); + if (amt != data.len) return error.InputOutput; for (subs.items) |sub| { const atom = self.getAtom(sub.atom).?; @@ -640,15 +653,6 @@ pub fn resolveLiterals(self: *Object, lp: *MachO.LiteralPool, macho_file: *MachO atom.addExtra(.{ .literal_pool_index = res.index }, macho_file); } } else if (isPtrLiteral(header)) { - var sections_data = std.AutoHashMap(u32, []const u8).init(gpa); - try sections_data.ensureUnusedCapacity(@intCast(self.sections.items(.header).len)); - defer { - var it = sections_data.iterator(); - while (it.next()) |entry| { - gpa.free(entry.value_ptr.*); - } - sections_data.deinit(); - } for (subs.items) |sub| { const atom = self.getAtom(sub.atom).?; const relocs = atom.getRelocs(macho_file); @@ -663,7 +667,11 @@ pub fn resolveLiterals(self: *Object, lp: *MachO.LiteralPool, macho_file: *MachO buffer.resize(target.size) catch unreachable; const gop = try sections_data.getOrPut(target.n_sect); if (!gop.found_existing) { - gop.value_ptr.* = try self.getSectionData(gpa, target.n_sect, file); + const target_sect = slice.items(.header)[target.n_sect]; + const data = try gpa.alloc(u8, target_sect.size); + const amt = try file.preadAll(data, target_sect.offset + self.offset); + if (amt != data.len) return error.InputOutput; + gop.value_ptr.* = data; } const data = gop.value_ptr.*; @memcpy(buffer.items, data[target.off..][0..target.size]); @@ -959,7 +967,7 @@ fn initRelocs(self: *Object, file: File.Handle, cpu_arch: std.Target.Cpu.Arch, m defer tracy.end(); const slice = self.sections.slice(); - for (slice.items(.header), slice.items(.relocs), 0..) |sect, *out, n_sect| { + for (slice.items(.header), slice.items(.relocs)) |sect, *out| { if (sect.nreloc == 0) continue; // We skip relocs for __DWARF since even in -r mode, the linker is expected to emit // debug symbol stabs in the relocatable. This made me curious why that is. For now, @@ -968,8 +976,8 @@ fn initRelocs(self: *Object, file: File.Handle, cpu_arch: std.Target.Cpu.Arch, m !mem.eql(u8, sect.sectName(), "__compact_unwind")) continue; switch (cpu_arch) { - .x86_64 => try x86_64.parseRelocs(self, @intCast(n_sect), sect, out, file, macho_file), - .aarch64 => try aarch64.parseRelocs(self, @intCast(n_sect), sect, out, file, macho_file), + .x86_64 => try x86_64.parseRelocs(self, sect, out, file, macho_file), + .aarch64 => try aarch64.parseRelocs(self, sect, out, file, macho_file), else => unreachable, } @@ -1003,12 +1011,9 @@ fn initEhFrameRecords(self: *Object, allocator: Allocator, sect_id: u8, file: Fi const sect = slice.items(.header)[sect_id]; const relocs = slice.items(.relocs)[sect_id]; - // TODO: read into buffer directly - const data = try self.getSectionData(allocator, sect_id, file); - defer allocator.free(data); - - try self.eh_frame_data.ensureTotalCapacityPrecise(allocator, data.len); - self.eh_frame_data.appendSliceAssumeCapacity(data); + try self.eh_frame_data.resize(allocator, sect.size); + const amt = try file.preadAll(self.eh_frame_data.items, sect.offset + self.offset); + if (amt != self.eh_frame_data.items.len) return error.InputOutput; // Check for non-personality relocs in FDEs and apply them for (relocs.items, 0..) |rel, i| { @@ -1106,8 +1111,12 @@ fn initUnwindRecords(self: *Object, allocator: Allocator, sect_id: u8, file: Fil } }; - const data = try self.getSectionData(allocator, sect_id, file); + const sect = self.sections.items(.header)[sect_id]; + const data = try allocator.alloc(u8, sect.size); defer allocator.free(data); + const amt = try file.preadAll(data, sect.offset + self.offset); + if (amt != data.len) return error.InputOutput; + const nrecs = @divExact(data.len, @sizeOf(macho.compact_unwind_entry)); const recs = @as([*]align(1) const macho.compact_unwind_entry, @ptrCast(data.ptr))[0..nrecs]; const sym_lookup = SymbolLookup{ .ctx = self }; @@ -1329,12 +1338,31 @@ fn parseDebugInfo(self: *Object, macho_file: *MachO) !void { if (debug_info_index == null or debug_abbrev_index == null) return; + const slice = self.sections.slice(); const file = macho_file.getFileHandle(self.file_handle); - const debug_info = try self.getSectionData(gpa, @intCast(debug_info_index.?), file); + const debug_info = blk: { + const sect = slice.items(.header)[debug_info_index.?]; + const data = try gpa.alloc(u8, sect.size); + const amt = try file.preadAll(data, sect.offset + self.offset); + if (amt != data.len) return error.InputOutput; + break :blk data; + }; defer gpa.free(debug_info); - const debug_abbrev = try self.getSectionData(gpa, @intCast(debug_abbrev_index.?), file); + const debug_abbrev = blk: { + const sect = slice.items(.header)[debug_abbrev_index.?]; + const data = try gpa.alloc(u8, sect.size); + const amt = try file.preadAll(data, sect.offset + self.offset); + if (amt != data.len) return error.InputOutput; + break :blk data; + }; defer gpa.free(debug_abbrev); - const debug_str = if (debug_str_index) |index| try self.getSectionData(gpa, @intCast(index), file) else &[0]u8{}; + const debug_str = if (debug_str_index) |sid| blk: { + const sect = slice.items(.header)[sid]; + const data = try gpa.alloc(u8, sect.size); + const amt = try file.preadAll(data, sect.offset + self.offset); + if (amt != data.len) return error.InputOutput; + break :blk data; + } else &[0]u8{}; defer gpa.free(debug_str); self.compile_unit = self.findCompileUnit(.{ @@ -1757,7 +1785,10 @@ pub fn writeAtoms(self: *Object, macho_file: *MachO) !void { for (headers, 0..) |header, n_sect| { if (header.isZerofill()) continue; - sections_data[n_sect] = try self.getSectionData(gpa, @intCast(n_sect), file); + const data = try gpa.alloc(u8, header.size); + const amt = try file.preadAll(data, header.offset + self.offset); + if (amt != data.len) return error.InputOutput; + sections_data[n_sect] = data; } for (self.getAtoms()) |atom_index| { const atom = self.getAtom(atom_index) orelse continue; @@ -1790,7 +1821,10 @@ pub fn writeAtomsRelocatable(self: *Object, macho_file: *MachO) !void { for (headers, 0..) |header, n_sect| { if (header.isZerofill()) continue; - sections_data[n_sect] = try self.getSectionData(gpa, @intCast(n_sect), file); + const data = try gpa.alloc(u8, header.size); + const amt = try file.preadAll(data, header.offset + self.offset); + if (amt != data.len) return error.InputOutput; + sections_data[n_sect] = data; } for (self.getAtoms()) |atom_index| { const atom = self.getAtom(atom_index) orelse continue; @@ -2183,17 +2217,6 @@ pub fn writeStabs(self: *const Object, stroff: u32, macho_file: *MachO) void { } } -pub fn getSectionData(self: *const Object, allocator: Allocator, index: u32, file: File.Handle) ![]u8 { - const slice = self.sections.slice(); - assert(index < slice.items(.header).len); - const sect = slice.items(.header)[index]; - const buffer = try allocator.alloc(u8, sect.size); - errdefer allocator.free(buffer); - const amt = try file.preadAll(buffer, sect.offset + self.offset); - if (amt != buffer.len) return error.InputOutput; - return buffer; -} - fn addString(self: *Object, allocator: Allocator, name: [:0]const u8) error{OutOfMemory}!MachO.String { const off: u32 = @intCast(self.strtab.items.len); try self.strtab.ensureUnusedCapacity(allocator, name.len + 1); @@ -2689,7 +2712,6 @@ const CompactUnwindCtx = struct { const x86_64 = struct { fn parseRelocs( self: *Object, - n_sect: u8, sect: macho.section_64, out: *std.ArrayListUnmanaged(Relocation), file: File.Handle, @@ -2705,8 +2727,12 @@ const x86_64 = struct { } const relocs = @as([*]align(1) const macho.relocation_info, @ptrCast(relocs_buffer.ptr))[0..sect.nreloc]; - const code = try self.getSectionData(gpa, @intCast(n_sect), file); + const code = try gpa.alloc(u8, sect.size); defer gpa.free(code); + { + const amt = try file.preadAll(code, sect.offset + self.offset); + if (amt != code.len) return error.InputOutput; + } try out.ensureTotalCapacityPrecise(gpa, relocs.len); @@ -2854,7 +2880,6 @@ const x86_64 = struct { const aarch64 = struct { fn parseRelocs( self: *Object, - n_sect: u8, sect: macho.section_64, out: *std.ArrayListUnmanaged(Relocation), file: File.Handle, @@ -2870,8 +2895,12 @@ const aarch64 = struct { } const relocs = @as([*]align(1) const macho.relocation_info, @ptrCast(relocs_buffer.ptr))[0..sect.nreloc]; - const code = try self.getSectionData(gpa, @intCast(n_sect), file); + const code = try gpa.alloc(u8, sect.size); defer gpa.free(code); + { + const amt = try file.preadAll(code, sect.offset + self.offset); + if (amt != code.len) return error.InputOutput; + } try out.ensureTotalCapacityPrecise(gpa, relocs.len); diff --git a/src/tracy.zig b/src/tracy.zig index 15ad6970..3e223a88 100644 --- a/src/tracy.zig +++ b/src/tracy.zig @@ -3,8 +3,7 @@ const builtin = @import("builtin"); const build_options = @import("build_options"); pub const enable = if (builtin.is_test) false else build_options.enable_tracy; -// pub const enable_allocation = enable; -pub const enable_allocation = false; +pub const enable_allocation = enable; pub const enable_callstack = enable; // TODO: make this configurable From 263c0ed9b9312b1115430d967aa85766999a7d44 Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Wed, 3 Jul 2024 08:04:09 +0200 Subject: [PATCH 2/4] macho: dylinker command is redundant for dylibs --- src/MachO.zig | 7 +++++-- src/MachO/load_commands.zig | 12 +++++++----- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/MachO.zig b/src/MachO.zig index 602fda4f..a726d0a4 100644 --- a/src/MachO.zig +++ b/src/MachO.zig @@ -2299,8 +2299,11 @@ fn writeLoadCommands(self: *MachO) !struct { usize, usize, usize } { ncmds += 1; try writer.writeStruct(self.dysymtab_cmd); ncmds += 1; - try load_commands.writeDylinkerLC(writer); - ncmds += 1; + + if (!self.options.dylib) { + try load_commands.writeDylinkerLC(writer); + ncmds += 1; + } if (self.getInternalObject()) |obj| { if (obj.getEntryRef(self)) |ref| { diff --git a/src/MachO/load_commands.zig b/src/MachO/load_commands.zig index 725bd429..80f1c923 100644 --- a/src/MachO/load_commands.zig +++ b/src/MachO/load_commands.zig @@ -38,11 +38,13 @@ pub fn calcLoadCommandsSize(macho_file: *MachO, assume_max_path_len: bool) u32 { // LC_DYSYMTAB sizeofcmds += @sizeOf(macho.dysymtab_command); // LC_LOAD_DYLINKER - sizeofcmds += calcInstallNameLen( - @sizeOf(macho.dylinker_command), - mem.sliceTo(default_dyld_path, 0), - false, - ); + if (!options.dylib) { + sizeofcmds += calcInstallNameLen( + @sizeOf(macho.dylinker_command), + mem.sliceTo(default_dyld_path, 0), + false, + ); + } // LC_MAIN if (!options.dylib) { sizeofcmds += @sizeOf(macho.entry_point_command); From 4e056851aed1044f89bf36376d983d14c3f8f22d Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Wed, 3 Jul 2024 11:44:23 +0200 Subject: [PATCH 3/4] macho: set minimum default headerpad to 32 bytes This matches Apple's ld and LLVM's lld. --- src/MachO/load_commands.zig | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/MachO/load_commands.zig b/src/MachO/load_commands.zig index 80f1c923..132ff469 100644 --- a/src/MachO/load_commands.zig +++ b/src/MachO/load_commands.zig @@ -137,7 +137,9 @@ pub fn calcLoadCommandsSizeObject(macho_file: *MachO) u32 { pub fn calcMinHeaderPadSize(macho_file: *MachO) u32 { const options = &macho_file.options; - var padding: u32 = calcLoadCommandsSize(macho_file, false) + (options.headerpad orelse 0); + // We match Apple's ld and LLVM's lld here. + const default_headerpad_size = options.headerpad orelse 32; + var padding: u32 = calcLoadCommandsSize(macho_file, false) + default_headerpad_size; log.debug("minimum requested headerpad size 0x{x}", .{padding + @sizeOf(macho.mach_header_64)}); if (options.headerpad_max_install_names) { From 18c0d897af77cf4123433f744794526262fcd39a Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Wed, 3 Jul 2024 14:14:53 +0200 Subject: [PATCH 4/4] ci: update to latest setup-zig to properly support arm64 runners --- .github/workflows/main.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index a092b4aa..c06dd05f 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -13,7 +13,7 @@ jobs: steps: - uses: actions/checkout@v3 - - uses: goto-bus-stop/setup-zig@v2 + - uses: goto-bus-stop/setup-zig@v2.2.0 with: version: master - run: zig version