Commit 1e710396d4

Jakub Konka <kubkon@jakubkonka.com>
2022-08-02 00:03:31
macho: fix linking in incremental context
Fix incorrect writing of symtab and strtab in dSYM bundle in incremental context. Fix incorrectly navigating unnamed consts (freeing) in incremental context. This is currently hard-coded to require all consts to land in `__TEXT,__const`, which is wrong and needs a rewrite.
1 parent 7bba3d3
Changed files (2)
src/link/MachO/DebugSymbols.zig
@@ -63,6 +63,10 @@ pub const Reloc = struct {
 pub fn populateMissingMetadata(self: *DebugSymbols, allocator: Allocator) !void {
     if (self.linkedit_segment_cmd_index == null) {
         self.linkedit_segment_cmd_index = @intCast(u8, self.segments.items.len);
+        log.debug("found __LINKEDIT segment free space 0x{x} to 0x{x}", .{
+            self.base.page_size,
+            self.base.page_size * 2,
+        });
         // TODO this needs reworking
         try self.segments.append(allocator, .{
             .segname = makeStaticString("__LINKEDIT"),
@@ -79,7 +83,7 @@ pub fn populateMissingMetadata(self: *DebugSymbols, allocator: Allocator) !void
     if (self.dwarf_segment_cmd_index == null) {
         self.dwarf_segment_cmd_index = @intCast(u8, self.segments.items.len);
 
-        const linkedit = self.segments.items[self.base.linkedit_segment_cmd_index.?];
+        const linkedit = self.segments.items[self.linkedit_segment_cmd_index.?];
         const ideal_size: u16 = 200 + 128 + 160 + 250;
         const needed_size = mem.alignForwardGeneric(u64, padToIdeal(ideal_size), self.base.page_size);
         const fileoff = linkedit.fileoff + linkedit.filesize;
@@ -290,20 +294,7 @@ pub fn flushModule(self: *DebugSymbols, allocator: Allocator, options: link.Opti
 
     var headers_buf = std.ArrayList(u8).init(allocator);
     defer headers_buf.deinit();
-    try self.base.writeSegmentHeaders(
-        0,
-        self.base.linkedit_segment_cmd_index.?,
-        &ncmds,
-        headers_buf.writer(),
-    );
-
-    for (self.segments.items) |seg| {
-        try headers_buf.writer().writeStruct(seg);
-        ncmds += 2;
-    }
-    for (self.sections.items) |header| {
-        try headers_buf.writer().writeStruct(header);
-    }
+    try self.writeSegmentHeaders(&ncmds, headers_buf.writer());
 
     try self.file.pwriteAll(headers_buf.items, @sizeOf(macho.mach_header_64));
     try self.file.pwriteAll(lc_buffer.items, @sizeOf(macho.mach_header_64) + headers_buf.items.len);
@@ -349,6 +340,7 @@ fn updateDwarfSegment(self: *DebugSymbols) void {
 
     var max_offset: u64 = 0;
     for (self.sections.items) |*sect| {
+        sect.addr += diff;
         log.debug("  {s},{s} - 0x{x}-0x{x} - 0x{x}-0x{x}", .{
             sect.segName(),
             sect.sectName(),
@@ -360,7 +352,6 @@ fn updateDwarfSegment(self: *DebugSymbols) void {
         if (sect.offset + sect.size > max_offset) {
             max_offset = sect.offset + sect.size;
         }
-        sect.addr += diff;
     }
 
     const file_size = max_offset - dwarf_segment.fileoff;
@@ -372,6 +363,37 @@ fn updateDwarfSegment(self: *DebugSymbols) void {
     }
 }
 
+fn writeSegmentHeaders(self: *DebugSymbols, ncmds: *u32, writer: anytype) !void {
+    // Write segment/section headers from the binary file first.
+    const end = self.base.linkedit_segment_cmd_index.?;
+    for (self.base.segments.items[0..end]) |seg, i| {
+        if (seg.nsects == 0 and
+            (mem.eql(u8, seg.segName(), "__DATA_CONST") or
+            mem.eql(u8, seg.segName(), "__DATA"))) continue;
+        var out_seg = seg;
+        out_seg.fileoff = 0;
+        out_seg.filesize = 0;
+        try writer.writeStruct(out_seg);
+
+        const indexes = self.base.getSectionIndexes(@intCast(u8, i));
+        for (self.base.sections.items(.header)[indexes.start..indexes.end]) |header| {
+            var out_header = header;
+            out_header.offset = 0;
+            try writer.writeStruct(out_header);
+        }
+
+        ncmds.* += 1;
+    }
+    // Next, commit DSYM's __LINKEDIT and __DWARF segments headers.
+    for (self.segments.items) |seg| {
+        try writer.writeStruct(seg);
+        ncmds.* += 1;
+    }
+    for (self.sections.items) |header| {
+        try writer.writeStruct(header);
+    }
+}
+
 fn writeHeader(self: *DebugSymbols, ncmds: u32, sizeofcmds: u32) !void {
     var header: macho.mach_header_64 = .{};
     header.filetype = macho.MH_DSYM;
@@ -469,11 +491,7 @@ fn writeSymtab(self: *DebugSymbols, lc: *macho.symtab_command) !void {
     const nsyms = nlocals + nexports;
 
     const seg = &self.segments.items[self.linkedit_segment_cmd_index.?];
-    const offset = mem.alignForwardGeneric(
-        u64,
-        seg.fileoff + seg.filesize,
-        @alignOf(macho.nlist_64),
-    );
+    const offset = mem.alignForwardGeneric(u64, seg.fileoff, @alignOf(macho.nlist_64));
     const needed_size = nsyms * @sizeOf(macho.nlist_64);
 
     if (needed_size > seg.filesize) {
@@ -535,7 +553,7 @@ fn writeStrtab(self: *DebugSymbols, lc: *macho.symtab_command) !void {
     const needed_size = mem.alignForwardGeneric(u64, self.strtab.buffer.items.len, @alignOf(u64));
     lc.strsize = @intCast(u32, needed_size);
 
-    if (offset + needed_size > seg.filesize) {
+    if (symtab_size + needed_size > seg.filesize) {
         const aligned_size = mem.alignForwardGeneric(u64, offset + needed_size, self.base.page_size);
         const diff = @intCast(u32, aligned_size - seg.filesize);
         const dwarf_seg = &self.segments.items[self.dwarf_segment_cmd_index.?];
src/link/MachO.zig
@@ -511,15 +511,14 @@ pub fn flushModule(self: *MachO, comp: *Compilation, prog_node: *std.Progress.No
 
     try self.createMhExecuteHeaderSymbol();
     try self.resolveDyldStubBinder();
+    try self.createDyldPrivateAtom();
+    try self.createStubHelperPreambleAtom();
     try self.resolveSymbolsInDylibs();
 
     if (self.unresolved.count() > 0) {
         return error.UndefinedSymbolReference;
     }
 
-    try self.createDyldPrivateAtom();
-    try self.createStubHelperPreambleAtom();
-
     try self.allocateSpecialSymbols();
 
     if (build_options.enable_logging) {
@@ -589,7 +588,7 @@ pub fn flushModule(self: *MachO, comp: *Compilation, prog_node: *std.Progress.No
     } else null;
 
     var headers_buf = std.ArrayList(u8).init(arena);
-    try self.writeSegmentHeaders(0, self.segments.items.len, &ncmds, headers_buf.writer());
+    try self.writeSegmentHeaders(&ncmds, headers_buf.writer());
 
     try self.base.file.?.pwriteAll(headers_buf.items, @sizeOf(macho.mach_header_64));
     try self.base.file.?.pwriteAll(lc_buffer.items, @sizeOf(macho.mach_header_64) + headers_buf.items.len);
@@ -1203,7 +1202,7 @@ fn linkOneShot(self: *MachO, comp: *Compilation, prog_node: *std.Progress.Node)
         } else null;
 
         var headers_buf = std.ArrayList(u8).init(arena);
-        try self.writeSegmentHeaders(0, self.segments.items.len, &ncmds, headers_buf.writer());
+        try self.writeSegmentHeaders(&ncmds, headers_buf.writer());
 
         try self.base.file.?.pwriteAll(headers_buf.items, @sizeOf(macho.mach_header_64));
         try self.base.file.?.pwriteAll(lc_buffer.items, @sizeOf(macho.mach_header_64) + headers_buf.items.len);
@@ -3863,7 +3862,9 @@ pub fn deleteExport(self: *MachO, exp: Export) void {
 fn freeUnnamedConsts(self: *MachO, decl_index: Module.Decl.Index) void {
     const unnamed_consts = self.unnamed_const_atoms.getPtr(decl_index) orelse return;
     for (unnamed_consts.items) |atom| {
-        const sect_id = atom.getSymbol(self).n_sect;
+        // TODO
+        // const sect_id = atom.getSymbol(self).n_sect;
+        const sect_id = self.getSectionByName("__TEXT", "__const").?;
         self.freeAtom(atom, sect_id, true);
         self.locals_free_list.append(self.base.allocator, atom.sym_index) catch {};
         self.locals.items[atom.sym_index].n_type = 0;
@@ -4402,8 +4403,6 @@ fn initSection(
     const index = try self.insertSection(segment_id, .{
         .sectname = makeStaticString(sectname),
         .segname = seg.segname,
-        .size = if (self.mode == .incremental) @intCast(u32, size) else 0,
-        .@"align" = alignment,
         .flags = opts.flags,
         .reserved1 = opts.reserved1,
         .reserved2 = opts.reserved2,
@@ -4413,6 +4412,9 @@ fn initSection(
 
     if (self.mode == .incremental) {
         const header = &self.sections.items(.header)[index];
+        header.size = size;
+        header.@"align" = alignment;
+
         const prev_end_off = if (index > 0) blk: {
             const prev_section = self.sections.get(index - 1);
             if (prev_section.segment_index == segment_id) {
@@ -4421,15 +4423,25 @@ fn initSection(
             } else break :blk seg.fileoff;
         } else 0;
         const alignment_pow_2 = try math.powi(u32, 2, alignment);
-        const padding: u64 = if (index == 0) try self.calcMinHeaderPad() else 0;
+        // TODO better prealloc for __text section
+        // const padding: u64 = if (index == 0) try self.calcMinHeaderPad() else 0;
+        const padding: u64 = if (index == 0) 0x1000 else 0;
         const off = mem.alignForwardGeneric(u64, padding + prev_end_off, alignment_pow_2);
-        log.debug("allocating {s},{s} section at 0x{x}", .{ header.segName(), header.sectName(), off });
-
-        header.addr = seg.vmaddr + off - seg.fileoff;
 
         if (!header.isZerofill()) {
             header.offset = @intCast(u32, off);
         }
+        header.addr = seg.vmaddr + off - seg.fileoff;
+
+        // TODO this will break if we are inserting section that is not the last section
+        // in a segment.
+        const max_size = self.allocatedSize(segment_id, off);
+
+        if (size > max_size) {
+            try self.growSection(index, @intCast(u32, size));
+        }
+
+        log.debug("allocating {s},{s} section at 0x{x}", .{ header.segName(), header.sectName(), off });
 
         self.updateSectionOrdinals(index + 1);
     }
@@ -4494,7 +4506,7 @@ fn updateSectionOrdinals(self: *MachO, start: u8) void {
 
     const slice = self.sections.slice();
     for (slice.items(.last_atom)[start..]) |last_atom| {
-        var atom = last_atom.?;
+        var atom = last_atom orelse continue;
 
         while (true) {
             const sym = atom.getSymbolPtr(self);
@@ -4536,17 +4548,6 @@ fn shiftLocalsByOffset(self: *MachO, sect_id: u8, offset: i64) !void {
     }
 }
 
-fn findFreeSpace(self: MachO, segment_id: u8, alignment: u64, start: ?u64) u64 {
-    const seg = self.segments.items[segment_id];
-    const indexes = self.getSectionIndexes(segment_id);
-    if (indexes.end - indexes.start == 0) {
-        return if (start) |v| v else seg.fileoff;
-    }
-    const last_sect = self.sections.items(.header)[indexes.end - 1];
-    const final_off = last_sect.offset + padToIdeal(last_sect.size);
-    return mem.alignForwardGeneric(u64, final_off, alignment);
-}
-
 fn growSegment(self: *MachO, segment_index: u8, new_size: u64) !void {
     const segment = &self.segments.items[segment_index];
     const new_segment_size = mem.alignForwardGeneric(u64, new_size, self.page_size);
@@ -4885,14 +4886,14 @@ fn getSegmentAllocBase(self: MachO, indices: []const ?u8) struct { vmaddr: u64,
     return .{ .vmaddr = 0, .fileoff = 0 };
 }
 
-pub fn writeSegmentHeaders(self: *MachO, start: usize, end: usize, ncmds: *u32, writer: anytype) !void {
-    for (self.segments.items[start..end]) |seg, i| {
+fn writeSegmentHeaders(self: *MachO, ncmds: *u32, writer: anytype) !void {
+    for (self.segments.items) |seg, i| {
         if (seg.nsects == 0 and
             (mem.eql(u8, seg.segName(), "__DATA_CONST") or
             mem.eql(u8, seg.segName(), "__DATA"))) continue;
         try writer.writeStruct(seg);
 
-        const indexes = self.getSectionIndexes(@intCast(u8, start + i));
+        const indexes = self.getSectionIndexes(@intCast(u8, i));
         for (self.sections.items(.header)[indexes.start..indexes.end]) |header| {
             try writer.writeStruct(header);
         }
@@ -5718,7 +5719,7 @@ pub fn getSectionByName(self: MachO, segname: []const u8, sectname: []const u8)
     } else return null;
 }
 
-fn getSectionIndexes(self: MachO, segment_index: u8) struct { start: u8, end: u8 } {
+pub fn getSectionIndexes(self: MachO, segment_index: u8) struct { start: u8, end: u8 } {
     var start: u8 = 0;
     const nsects = for (self.segments.items) |seg, i| {
         if (i == segment_index) break @intCast(u8, seg.nsects);