From bc69d5a00fb197a7bafc716b84c8675382074c19 Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Wed, 26 Apr 2023 12:32:03 +0200 Subject: [PATCH] macho: invalidate GOT/stub relocs after segment shift in memory --- src/link/MachO.zig | 34 ++++++++++++++++++++++------- src/link/MachO/Relocation.zig | 41 +++++++++++++++++++++++------------ test/behavior/bugs/1851.zig | 4 ---- 3 files changed, 53 insertions(+), 26 deletions(-) diff --git a/src/link/MachO.zig b/src/link/MachO.zig index 47954f8871..df9b8a768a 100644 --- a/src/link/MachO.zig +++ b/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, }); } diff --git a/src/link/MachO/Relocation.zig b/src/link/MachO/Relocation.zig index e511901009..2685cc26e2 100644 --- a/src/link/MachO/Relocation.zig +++ b/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, } } diff --git a/test/behavior/bugs/1851.zig b/test/behavior/bugs/1851.zig index ff23e9a7c4..cf7c7ced28 100644 --- a/test/behavior/bugs/1851.zig +++ b/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);