Commit bc69d5a00f

Jakub Konka <kubkon@jakubkonka.com>
2023-04-26 12:32:03
macho: invalidate GOT/stub relocs after segment shift in memory
1 parent b5b0b55
Changed files (3)
src
test
behavior
src/link/MachO/Relocation.zig
@@ -37,14 +37,33 @@ pub const Type = enum {
     tlv_initializer,
 };
 
-/// Returns true if and only if the reloc is dirty AND the target address is available.
+/// Returns true if and only if the reloc can be resolved.
 pub fn isResolvable(self: Relocation, macho_file: *MachO) bool {
-    const addr = self.getTargetBaseAddress(macho_file) orelse return false;
-    if (addr == 0) return false;
-    return self.dirty;
+    _ = self.getTargetBaseAddress(macho_file) orelse return false;
+    return true;
+}
+
+pub fn isGotIndirection(self: Relocation) bool {
+    return switch (self.type) {
+        .got, .got_page, .got_pageoff => true,
+        else => false,
+    };
+}
+
+pub fn isStubTrampoline(self: Relocation, macho_file: *MachO) bool {
+    return switch (self.type) {
+        .branch => macho_file.getSymbol(self.target).undf(),
+        else => false,
+    };
 }
 
 pub fn getTargetBaseAddress(self: Relocation, macho_file: *MachO) ?u64 {
+    if (self.isStubTrampoline(macho_file)) {
+        const index = macho_file.stub_table.lookup.get(self.target) orelse return null;
+        const header = macho_file.sections.items(.header)[macho_file.stubs_section_index.?];
+        return header.addr +
+            index * @import("stubs.zig").calcStubEntrySize(macho_file.base.options.target.cpu.arch);
+    }
     switch (self.type) {
         .got, .got_page, .got_pageoff => {
             const got_index = macho_file.got_table.lookup.get(self.target) orelse return null;
@@ -56,17 +75,11 @@ pub fn getTargetBaseAddress(self: Relocation, macho_file: *MachO) ?u64 {
             const atom = macho_file.getAtom(atom_index);
             return atom.getSymbol(macho_file).n_value;
         },
-        .branch => {
-            if (macho_file.stub_table.lookup.get(self.target)) |index| {
-                const header = macho_file.sections.items(.header)[macho_file.stubs_section_index.?];
-                return header.addr +
-                    index * @import("stubs.zig").calcStubEntrySize(macho_file.base.options.target.cpu.arch);
-            }
-            const atom_index = macho_file.getAtomIndexForSymbol(self.target) orelse return null;
-            const atom = macho_file.getAtom(atom_index);
-            return atom.getSymbol(macho_file).n_value;
+        else => {
+            const target_atom_index = macho_file.getAtomIndexForSymbol(self.target) orelse return null;
+            const target_atom = macho_file.getAtom(target_atom_index);
+            return target_atom.getSymbol(macho_file).n_value;
         },
-        else => return macho_file.getSymbol(self.target).n_value,
     }
 }
 
src/link/MachO.zig
@@ -1146,7 +1146,9 @@ pub fn writeAtom(self: *MachO, atom_index: Atom.Index, code: []u8) !void {
     if (self.relocs.getPtr(atom_index)) |rels| {
         try relocs.ensureTotalCapacityPrecise(rels.items.len);
         for (rels.items) |*reloc| {
-            if (reloc.isResolvable(self)) relocs.appendAssumeCapacity(reloc);
+            if (reloc.isResolvable(self) and reloc.dirty) {
+                relocs.appendAssumeCapacity(reloc);
+            }
         }
     }
 
@@ -1332,18 +1334,33 @@ fn markRelocsDirtyByTarget(self: *MachO, target: SymbolWithLoc) void {
 
 fn markRelocsDirtyByAddress(self: *MachO, addr: u64) void {
     log.debug("marking relocs dirty by address: {x}", .{addr});
+
+    const got_moved = blk: {
+        const sect_id = self.got_section_index orelse break :blk false;
+        break :blk self.sections.items(.header)[sect_id].addr > addr;
+    };
+    const stubs_moved = blk: {
+        const sect_id = self.stubs_section_index orelse break :blk false;
+        break :blk self.sections.items(.header)[sect_id].addr > addr;
+    };
+
     for (self.relocs.values()) |*relocs| {
         for (relocs.items) |*reloc| {
-            const target_addr = reloc.getTargetBaseAddress(self) orelse continue;
-            if (target_addr < addr) continue;
-            reloc.dirty = true;
+            if (reloc.isGotIndirection()) {
+                reloc.dirty = reloc.dirty or got_moved;
+            } else if (reloc.isStubTrampoline(self)) {
+                reloc.dirty = reloc.dirty or stubs_moved;
+            } else {
+                const target_addr = reloc.getTargetBaseAddress(self) orelse continue;
+                if (target_addr > addr) reloc.dirty = true;
+            }
         }
     }
 
     // TODO: dirty only really affected GOT cells
     for (self.got_table.entries.items) |entry| {
         const target_addr = self.getSymbol(entry).n_value;
-        if (target_addr >= addr) {
+        if (target_addr > addr) {
             self.got_table_contents_dirty = true;
             break;
         }
@@ -1353,7 +1370,7 @@ fn markRelocsDirtyByAddress(self: *MachO, addr: u64) void {
         const stubs_addr = self.getSegment(self.stubs_section_index.?).vmaddr;
         const stub_helper_addr = self.getSegment(self.stub_helper_section_index.?).vmaddr;
         const laptr_addr = self.getSegment(self.la_symbol_ptr_section_index.?).vmaddr;
-        if (stubs_addr >= addr or stub_helper_addr >= addr or laptr_addr >= addr)
+        if (stubs_addr > addr or stub_helper_addr > addr or laptr_addr > addr)
             self.stub_table_contents_dirty = true;
     }
 }
@@ -2794,7 +2811,7 @@ fn growSection(self: *MachO, sect_id: u8, needed_size: u64) !void {
 
     const sect_vm_capacity = self.allocatedVirtualSize(segment.vmaddr);
     if (needed_size > sect_vm_capacity) {
-        self.markRelocsDirtyByAddress(segment.vmaddr + needed_size);
+        self.markRelocsDirtyByAddress(segment.vmaddr + segment.vmsize);
         try self.growSectionVirtualMemory(sect_id, needed_size);
     }
 
@@ -4067,11 +4084,12 @@ pub fn findFirst(comptime T: type, haystack: []align(1) const T, start: usize, p
 pub fn logSections(self: *MachO) void {
     log.debug("sections:", .{});
     for (self.sections.items(.header), 0..) |header, i| {
-        log.debug("  sect({d}): {s},{s} @{x}, sizeof({x})", .{
+        log.debug("  sect({d}): {s},{s} @{x} ({x}), sizeof({x})", .{
             i + 1,
             header.segName(),
             header.sectName(),
             header.offset,
+            header.addr,
             header.size,
         });
     }
test/behavior/bugs/1851.zig
@@ -16,10 +16,6 @@ test "allocation and looping over 3-byte integer" {
         return error.SkipZigTest; // TODO
     }
 
-    if (builtin.zig_backend == .stage2_x86_64 and builtin.os.tag == .macos) {
-        return error.SkipZigTest; // TODO
-    }
-
     try expect(@sizeOf(u24) == 4);
     try expect(@sizeOf([1]u24) == 4);
     try expect(@alignOf(u24) == 4);