Commit a8629fb850

Jakub Konka <kubkon@jakubkonka.com>
2024-01-18 21:55:31
macho: fix symbol index dereference in codegen wrt ZigObject
This is incredibly confusing and I really need to simplify it. Elf also possesses this shortcoming so once I get Coff up to speed it should hopefully become clear on how to refactor this.
1 parent 30b7d3e
Changed files (6)
src/arch/x86_64/Emit.zig
@@ -156,9 +156,10 @@ pub fn emitMir(emit: *Emit) Error!void {
                         .Lib => emit.lower.link_mode == .Static,
                     };
                     const atom = macho_file.getSymbol(data.atom_index).getAtom(macho_file).?;
-                    const sym = macho_file.getSymbol(data.sym_index);
+                    const sym_index = macho_file.getZigObject().?.symbols.items[data.sym_index];
+                    const sym = macho_file.getSymbol(sym_index);
                     if (sym.flags.needs_zig_got and !is_obj_or_static_lib) {
-                        _ = try sym.getOrCreateZigGotEntry(data.sym_index, macho_file);
+                        _ = try sym.getOrCreateZigGotEntry(sym_index, macho_file);
                     }
                     const @"type": link.File.MachO.Relocation.Type = if (sym.flags.needs_zig_got and !is_obj_or_static_lib)
                         .zig_got_load
src/link/MachO/Atom.zig
@@ -402,7 +402,11 @@ pub fn scanRelocs(self: Atom, macho_file: *MachO) !void {
     defer tracy.end();
     assert(self.flags.alive);
 
-    const object = self.getFile(macho_file).object;
+    const dynrel_ctx = switch (self.getFile(macho_file)) {
+        .zig_object => |x| &x.dynamic_relocs,
+        .object => |x| &x.dynamic_relocs,
+        else => unreachable,
+    };
     const relocs = self.getRelocs(macho_file);
 
     for (relocs) |rel| {
@@ -437,6 +441,10 @@ pub fn scanRelocs(self: Atom, macho_file: *MachO) !void {
                 }
             },
 
+            .zig_got_load => {
+                assert(rel.getTargetSymbol(macho_file).flags.has_zig_got);
+            },
+
             .got => {
                 rel.getTargetSymbol(macho_file).flags.needs_got = true;
             },
@@ -448,7 +456,7 @@ pub fn scanRelocs(self: Atom, macho_file: *MachO) !void {
                 const symbol = rel.getTargetSymbol(macho_file);
                 if (!symbol.flags.tlv) {
                     try macho_file.reportParseError2(
-                        object.index,
+                        self.getFile(macho_file).getIndex(),
                         "{s}: illegal thread-local variable reference to regular symbol {s}",
                         .{ self.getName(macho_file), symbol.getName(macho_file) },
                     );
@@ -470,27 +478,34 @@ pub fn scanRelocs(self: Atom, macho_file: *MachO) !void {
                             continue;
                         }
                         if (symbol.flags.import) {
-                            object.num_bind_relocs += 1;
+                            dynrel_ctx.bind_relocs += 1;
                             if (symbol.flags.weak) {
-                                object.num_weak_bind_relocs += 1;
+                                dynrel_ctx.weak_bind_relocs += 1;
                                 macho_file.binds_to_weak = true;
                             }
                             continue;
                         }
                         if (symbol.flags.@"export") {
                             if (symbol.flags.weak) {
-                                object.num_weak_bind_relocs += 1;
+                                dynrel_ctx.weak_bind_relocs += 1;
                                 macho_file.binds_to_weak = true;
                             } else if (symbol.flags.interposable) {
-                                object.num_bind_relocs += 1;
+                                dynrel_ctx.bind_relocs += 1;
                             }
                         }
                     }
-                    object.num_rebase_relocs += 1;
+                    dynrel_ctx.rebase_relocs += 1;
                 }
             },
 
-            else => {},
+            .signed,
+            .signed1,
+            .signed2,
+            .signed4,
+            .page,
+            .pageoff,
+            .subtractor,
+            => {},
         }
     }
 }
src/link/MachO/Object.zig
@@ -25,10 +25,8 @@ unwind_records: std.ArrayListUnmanaged(UnwindInfo.Record.Index) = .{},
 
 alive: bool = true,
 hidden: bool = false,
-num_rebase_relocs: u32 = 0,
-num_bind_relocs: u32 = 0,
-num_weak_bind_relocs: u32 = 0,
 
+dynamic_relocs: MachO.DynamicRelocs = .{},
 output_symtab_ctx: MachO.SymtabCtx = .{},
 
 pub fn isObject(path: []const u8) !bool {
src/link/MachO/ZigObject.zig
@@ -41,6 +41,7 @@ anon_decls: AnonDeclTable = .{},
 /// A table of relocations.
 relocs: RelocationTable = .{},
 
+dynamic_relocs: MachO.DynamicRelocs = .{},
 output_symtab_ctx: MachO.SymtabCtx = .{},
 
 pub fn init(self: *ZigObject, macho_file: *MachO) !void {
@@ -222,10 +223,31 @@ pub fn markLive(self: *ZigObject, macho_file: *MachO) void {
 }
 
 pub fn checkDuplicates(self: *ZigObject, dupes: anytype, macho_file: *MachO) !void {
-    _ = self;
-    _ = dupes;
-    _ = macho_file;
-    @panic("TODO checkDuplicates");
+    for (self.symbols.items, 0..) |index, nlist_idx| {
+        const sym = macho_file.getSymbol(index);
+        if (sym.visibility != .global) continue;
+        const file = sym.getFile(macho_file) orelse continue;
+        if (file.getIndex() == self.index) continue;
+
+        const nlist = self.symtab.items(.nlist)[nlist_idx];
+        if (!nlist.undf() and !nlist.tentative() and !(nlist.weakDef() or nlist.pext())) {
+            const gop = try dupes.getOrPut(index);
+            if (!gop.found_existing) {
+                gop.value_ptr.* = .{};
+            }
+            try gop.value_ptr.append(macho_file.base.comp.gpa, self.index);
+        }
+    }
+}
+
+pub fn scanRelocs(self: *ZigObject, macho_file: *MachO) !void {
+    for (self.atoms.items) |atom_index| {
+        const atom = macho_file.getAtom(atom_index) orelse continue;
+        if (!atom.flags.alive) continue;
+        const sect = atom.getInputSection(macho_file);
+        if (sect.isZerofill()) continue;
+        try atom.scanRelocs(macho_file);
+    }
 }
 
 pub fn calcSymtabSize(self: *ZigObject, macho_file: *MachO) !void {
@@ -537,7 +559,8 @@ pub fn updateDecl(
         const name = mod.intern_pool.stringToSlice(decl.name);
         const lib_name = mod.intern_pool.stringToSliceUnwrap(variable.lib_name);
         const index = try self.getGlobalSymbol(macho_file, name, lib_name);
-        macho_file.getSymbol(index).flags.needs_got = true;
+        const actual_index = self.symbols.items[index];
+        macho_file.getSymbol(actual_index).flags.needs_got = true;
         return;
     }
 
src/link/MachO.zig
@@ -542,8 +542,6 @@ pub fn flushModule(self: *MachO, arena: Allocator, prog_node: *std.Progress.Node
         self.internal_object = index;
     }
 
-    state_log.debug("{}", .{self.dumpState()});
-
     try self.addUndefinedGlobals();
     try self.resolveSymbols();
     try self.resolveSyntheticSymbols();
@@ -2389,14 +2387,23 @@ fn initDyldInfoSections(self: *MachO) !void {
     if (self.la_symbol_ptr_sect_index != null) try self.la_symbol_ptr.addDyldRelocs(self);
     try self.initExportTrie();
 
+    var objects = try std.ArrayList(File.Index).initCapacity(gpa, self.objects.items.len + 1);
+    defer objects.deinit();
+    if (self.getZigObject()) |zo| objects.appendAssumeCapacity(zo.index);
+    objects.appendSliceAssumeCapacity(self.objects.items);
+
     var nrebases: usize = 0;
     var nbinds: usize = 0;
     var nweak_binds: usize = 0;
-    for (self.objects.items) |index| {
-        const object = self.getFile(index).?.object;
-        nrebases += object.num_rebase_relocs;
-        nbinds += object.num_bind_relocs;
-        nweak_binds += object.num_weak_bind_relocs;
+    for (objects.items) |index| {
+        const ctx = switch (self.getFile(index).?) {
+            .zig_object => |x| x.dynamic_relocs,
+            .object => |x| x.dynamic_relocs,
+            else => unreachable,
+        };
+        nrebases += ctx.rebase_relocs;
+        nbinds += ctx.bind_relocs;
+        nweak_binds += ctx.weak_bind_relocs;
     }
     try self.rebase.entries.ensureUnusedCapacity(gpa, nrebases);
     try self.bind.entries.ensureUnusedCapacity(gpa, nbinds);
@@ -3947,6 +3954,12 @@ const HotUpdateState = struct {
     mach_task: ?std.os.darwin.MachTask = null,
 };
 
+pub const DynamicRelocs = struct {
+    rebase_relocs: u32 = 0,
+    bind_relocs: u32 = 0,
+    weak_bind_relocs: u32 = 0,
+};
+
 pub const SymtabCtx = struct {
     ilocal: u32 = 0,
     istab: u32 = 0,
src/codegen.zig
@@ -993,7 +993,7 @@ fn genDeclRef(
             else
                 null;
             const sym_index = try macho_file.getGlobalSymbol(sym_name, lib_name);
-            macho_file.getSymbol(sym_index).flags.needs_got = true;
+            macho_file.getSymbol(macho_file.getZigObject().?.symbols.items[sym_index]).flags.needs_got = true;
             return GenResult.mcv(.{ .load_symbol = sym_index });
         }
         const sym_index = try macho_file.getZigObject().?.getOrCreateMetadataForDecl(macho_file, decl_index);