macho: improve logs for dyld info

8 files changed, 88 insertions, 54 deletions

diff --git a/src/arch/aarch64/CodeGen.zig b/src/arch/aarch64/CodeGen.zig
index f65e19d561..ba7c56e2bd 100644
--- a/src/arch/aarch64/CodeGen.zig
+++ b/src/arch/aarch64/CodeGen.zig
@@ -3190,14 +3190,14 @@ fn airCall(self: *Self, inst: Air.Inst.Index, modifier: std.builtin.CallOptions.
                         lib_name,
                     });
                 }
-                const global_index = try macho_file.getGlobalSymbol(mem.sliceTo(decl_name, 0));
+                const sym_index = try macho_file.getGlobalSymbol(mem.sliceTo(decl_name, 0));
 
                 _ = try self.addInst(.{
                     .tag = .call_extern,
                     .data = .{
-                        .extern_fn = .{
+                        .relocation = .{
                             .atom_index = mod.declPtr(self.mod_fn.owner_decl).link.macho.sym_index,
-                            .global_index = global_index,
+                            .sym_index = sym_index,
                         },
                     },
                 });
diff --git a/src/arch/aarch64/Emit.zig b/src/arch/aarch64/Emit.zig
index 7469eaefeb..47a0c08893 100644
--- a/src/arch/aarch64/Emit.zig
+++ b/src/arch/aarch64/Emit.zig
@@ -649,7 +649,7 @@ fn mirDebugEpilogueBegin(self: *Emit) !void {
 
 fn mirCallExtern(emit: *Emit, inst: Mir.Inst.Index) !void {
     assert(emit.mir.instructions.items(.tag)[inst] == .call_extern);
-    const extern_fn = emit.mir.instructions.items(.data)[inst].extern_fn;
+    const relocation = emit.mir.instructions.items(.data)[inst].relocation;
 
     if (emit.bin_file.cast(link.File.MachO)) |macho_file| {
         const offset = blk: {
@@ -659,11 +659,13 @@ fn mirCallExtern(emit: *Emit, inst: Mir.Inst.Index) !void {
             break :blk offset;
         };
         // Add relocation to the decl.
-        const atom = macho_file.atom_by_index_table.get(extern_fn.atom_index).?;
-        const target = macho_file.globals.values()[extern_fn.global_index];
+        const atom = macho_file.atom_by_index_table.get(relocation.atom_index).?;
         try atom.relocs.append(emit.bin_file.allocator, .{
             .offset = offset,
-            .target = target,
+            .target = .{
+                .sym_index = relocation.sym_index,
+                .file = null,
+            },
             .addend = 0,
             .subtractor = null,
             .pcrel = true,
diff --git a/src/arch/aarch64/Mir.zig b/src/arch/aarch64/Mir.zig
index b162905f36..2fef069f7a 100644
--- a/src/arch/aarch64/Mir.zig
+++ b/src/arch/aarch64/Mir.zig
@@ -225,14 +225,16 @@ pub const Inst = struct {
         ///
         /// Used by e.g. b
         inst: Index,
-        /// An extern function
+        /// Relocation for the linker where:
+        /// * `atom_index` is the index of the source
+        /// * `sym_index` is the index of the target
         ///
         /// Used by e.g. call_extern
-        extern_fn: struct {
+        relocation: struct {
             /// Index of the containing atom.
             atom_index: u32,
             /// Index into the linker's string table.
-            global_index: u32,
+            sym_index: u32,
         },
         /// A 16-bit immediate value.
         ///
diff --git a/src/arch/x86_64/CodeGen.zig b/src/arch/x86_64/CodeGen.zig
index da35d3b4b6..b35db3e97a 100644
--- a/src/arch/x86_64/CodeGen.zig
+++ b/src/arch/x86_64/CodeGen.zig
@@ -2644,7 +2644,7 @@ fn loadMemPtrIntoRegister(self: *Self, reg: Register, ptr_ty: Type, ptr: MCValue
                     .flags = flags,
                 }),
                 .data = .{
-                    .load_reloc = .{
+                    .relocation = .{
                         .atom_index = fn_owner_decl.link.macho.sym_index,
                         .sym_index = sym_index,
                     },
@@ -3997,14 +3997,14 @@ fn airCall(self: *Self, inst: Air.Inst.Index, modifier: std.builtin.CallOptions.
                         lib_name,
                     });
                 }
-                const global_index = try macho_file.getGlobalSymbol(mem.sliceTo(decl_name, 0));
+                const sym_index = try macho_file.getGlobalSymbol(mem.sliceTo(decl_name, 0));
                 _ = try self.addInst(.{
                     .tag = .call_extern,
                     .ops = undefined,
                     .data = .{
-                        .extern_fn = .{
+                        .relocation = .{
                             .atom_index = mod.declPtr(self.mod_fn.owner_decl).link.macho.sym_index,
-                            .global_index = global_index,
+                            .sym_index = sym_index,
                         },
                     },
                 });
diff --git a/src/arch/x86_64/Emit.zig b/src/arch/x86_64/Emit.zig
index 8d91c3d7e6..52d68e81ec 100644
--- a/src/arch/x86_64/Emit.zig
+++ b/src/arch/x86_64/Emit.zig
@@ -982,7 +982,7 @@ fn mirLeaPie(emit: *Emit, inst: Mir.Inst.Index) InnerError!void {
     const tag = emit.mir.instructions.items(.tag)[inst];
     assert(tag == .lea_pie);
     const ops = emit.mir.instructions.items(.ops)[inst].decode();
-    const load_reloc = emit.mir.instructions.items(.data)[inst].load_reloc;
+    const relocation = emit.mir.instructions.items(.data)[inst].relocation;
 
     // lea reg1, [rip + reloc]
     // RM
@@ -1001,11 +1001,11 @@ fn mirLeaPie(emit: *Emit, inst: Mir.Inst.Index) InnerError!void {
             0b01 => @enumToInt(std.macho.reloc_type_x86_64.X86_64_RELOC_SIGNED),
             else => return emit.fail("TODO unused LEA PIE variants 0b10 and 0b11", .{}),
         };
-        const atom = macho_file.atom_by_index_table.get(load_reloc.atom_index).?;
-        log.debug("adding reloc of type {} to local @{d}", .{ reloc_type, load_reloc.sym_index });
+        const atom = macho_file.atom_by_index_table.get(relocation.atom_index).?;
+        log.debug("adding reloc of type {} to local @{d}", .{ reloc_type, relocation.sym_index });
         try atom.relocs.append(emit.bin_file.allocator, .{
             .offset = @intCast(u32, end_offset - 4),
-            .target = .{ .sym_index = load_reloc.sym_index, .file = null },
+            .target = .{ .sym_index = relocation.sym_index, .file = null },
             .addend = 0,
             .subtractor = null,
             .pcrel = true,
@@ -1116,7 +1116,7 @@ fn mirCmpFloatAvx(emit: *Emit, tag: Tag, inst: Mir.Inst.Index) InnerError!void {
 fn mirCallExtern(emit: *Emit, inst: Mir.Inst.Index) InnerError!void {
     const tag = emit.mir.instructions.items(.tag)[inst];
     assert(tag == .call_extern);
-    const extern_fn = emit.mir.instructions.items(.data)[inst].extern_fn;
+    const relocation = emit.mir.instructions.items(.data)[inst].relocation;
 
     const offset = blk: {
         // callq
@@ -1126,11 +1126,13 @@ fn mirCallExtern(emit: *Emit, inst: Mir.Inst.Index) InnerError!void {
 
     if (emit.bin_file.cast(link.File.MachO)) |macho_file| {
         // Add relocation to the decl.
-        const atom = macho_file.atom_by_index_table.get(extern_fn.atom_index).?;
-        const target = macho_file.globals.values()[extern_fn.global_index];
+        const atom = macho_file.atom_by_index_table.get(relocation.atom_index).?;
         try atom.relocs.append(emit.bin_file.allocator, .{
             .offset = offset,
-            .target = target,
+            .target = .{
+                .sym_index = relocation.sym_index,
+                .file = null,
+            },
             .addend = 0,
             .subtractor = null,
             .pcrel = true,
diff --git a/src/arch/x86_64/Mir.zig b/src/arch/x86_64/Mir.zig
index 91ad9f4d9c..f67b48a271 100644
--- a/src/arch/x86_64/Mir.zig
+++ b/src/arch/x86_64/Mir.zig
@@ -181,7 +181,7 @@ pub const Inst = struct {
         ///      0b00  reg1, [rip + reloc] // via GOT emits X86_64_RELOC_GOT relocation
         ///      0b01  reg1, [rip + reloc] // direct load emits X86_64_RELOC_SIGNED relocation
         /// Notes:
-        /// * `Data` contains `load_reloc`
+        /// * `Data` contains `relocation`
         lea_pie,
 
         /// ops flags: form:
@@ -368,7 +368,7 @@ pub const Inst = struct {
         /// Pseudo-instructions
         /// call extern function
         /// Notes:
-        ///   * target of the call is stored as `extern_fn` in `Data` union.
+        ///   * target of the call is stored as `relocation` in `Data` union.
         call_extern,
 
         /// end of prologue
@@ -439,15 +439,10 @@ pub const Inst = struct {
             /// A condition code for use with EFLAGS register.
             cc: bits.Condition,
         },
-        /// An extern function.
-        extern_fn: struct {
-            /// Index of the containing atom.
-            atom_index: u32,
-            /// Index into the linker's globals table.
-            global_index: u32,
-        },
-        /// PIE load relocation.
-        load_reloc: struct {
+        /// Relocation for the linker where:
+        /// * `atom_index` is the index of the source
+        /// * `sym_index` is the index of the target
+        relocation: struct {
             /// Index of the containing atom.
             atom_index: u32,
             /// Index into the linker's symbol table.
diff --git a/src/link/MachO.zig b/src/link/MachO.zig
index b441aaefc1..d4fbd14287 100644
--- a/src/link/MachO.zig
+++ b/src/link/MachO.zig
@@ -153,6 +153,13 @@ rustc_section_size: u64 = 0,
 
 locals: std.ArrayListUnmanaged(macho.nlist_64) = .{},
 globals: std.StringArrayHashMapUnmanaged(SymbolWithLoc) = .{},
+// FIXME Jakub
+// TODO storing index into globals might be dangerous if we delete a global
+// while not having everything resolved. Actually, perhaps `unresolved`
+// should not be stored at the global scope? Is this possible?
+// Otherwise, audit if this can be a problem.
+// An alternative, which I still need to investigate for perf reasons is to
+// store all global names in an adapted with context strtab.
 unresolved: std.AutoArrayHashMapUnmanaged(u32, bool) = .{},
 
 locals_free_list: std.ArrayListUnmanaged(u32) = .{},
@@ -2449,9 +2456,9 @@ pub fn createGotAtom(self: *MachO, target: SymbolWithLoc) !*Atom {
 
     const target_sym = self.getSymbol(target);
     if (target_sym.undf()) {
-        const global_index = @intCast(u32, self.globals.getIndex(self.getSymbolName(target)).?);
+        const global = self.globals.get(self.getSymbolName(target)).?;
         try atom.bindings.append(gpa, .{
-            .global_index = global_index,
+            .target = global,
             .offset = 0,
         });
     } else {
@@ -2483,9 +2490,10 @@ pub fn createTlvPtrAtom(self: *MachO, target: SymbolWithLoc) !*Atom {
     const atom = try MachO.createEmptyAtom(gpa, sym_index, @sizeOf(u64), 3);
     const target_sym = self.getSymbol(target);
     assert(target_sym.undf());
-    const global_index = @intCast(u32, self.globals.getIndex(self.getSymbolName(target)).?);
+
+    const global = self.globals.get(self.getSymbolName(target)).?;
     try atom.bindings.append(gpa, .{
-        .global_index = global_index,
+        .target = global,
         .offset = 0,
     });
 
@@ -2739,7 +2747,6 @@ pub fn createStubHelperAtom(self: *MachO) !*Atom {
 pub fn createLazyPointerAtom(self: *MachO, stub_sym_index: u32, target: SymbolWithLoc) !*Atom {
     const gpa = self.base.allocator;
     const sym_index = @intCast(u32, self.locals.items.len);
-    const global_index = @intCast(u32, self.globals.getIndex(self.getSymbolName(target)).?);
     try self.locals.append(gpa, .{
         .n_strx = 0,
         .n_type = macho.N_SECT,
@@ -2762,8 +2769,10 @@ pub fn createLazyPointerAtom(self: *MachO, stub_sym_index: u32, target: SymbolWi
         },
     });
     try atom.rebases.append(gpa, 0);
+
+    const global = self.globals.get(self.getSymbolName(target)).?;
     try atom.lazy_bindings.append(gpa, .{
-        .global_index = global_index,
+        .target = global,
         .offset = 0,
     });
 
@@ -4149,6 +4158,7 @@ pub fn deleteExport(self: *MachO, exp: Export) void {
     const sym = self.getSymbolPtr(sym_loc);
     const sym_name = self.getSymbolName(sym_loc);
     log.debug("deleting export '{s}'", .{sym_name});
+    assert(sym.sect() and sym.ext());
     sym.* = .{
         .n_strx = 0,
         .n_type = 0,
@@ -5307,7 +5317,9 @@ pub fn getGlobalSymbol(self: *MachO, name: []const u8) !u32 {
     defer if (gop.found_existing) gpa.free(sym_name);
 
     if (gop.found_existing) {
-        return @intCast(u32, self.globals.getIndex(sym_name).?);
+        // TODO audit this: can we ever reference anything from outside the Zig module?
+        assert(gop.value_ptr.file == null);
+        return gop.value_ptr.sym_index;
     }
 
     const sym_index = @intCast(u32, self.locals.items.len);
@@ -5324,7 +5336,7 @@ pub fn getGlobalSymbol(self: *MachO, name: []const u8) !u32 {
     };
     try self.unresolved.putNoClobber(gpa, global_index, true);
 
-    return global_index;
+    return sym_index;
 }
 
 fn getSegmentAllocBase(self: MachO, indices: []const ?u16) struct { vmaddr: u64, fileoff: u64 } {
@@ -5690,6 +5702,8 @@ fn updateSectionOrdinals(self: *MachO) !void {
         }
     }
 
+    // FIXME Jakub
+    // TODO no need for duping work here; simply walk the atom graph
     for (self.locals.items) |*sym| {
         if (sym.undf()) continue;
         if (sym.n_sect == 0) continue;
@@ -5735,11 +5749,12 @@ fn writeDyldInfoData(self: *MachO) !void {
             log.debug("dyld info for {s},{s}", .{ sect.segName(), sect.sectName() });
 
             while (true) {
-                log.debug("  ATOM %{d}", .{atom.sym_index});
+                log.debug("  ATOM(%{d}, '{s}')", .{ atom.sym_index, atom.getName(self) });
                 const sym = atom.getSymbol(self);
                 const base_offset = sym.n_value - seg.inner.vmaddr;
 
                 for (atom.rebases.items) |offset| {
+                    log.debug("    | rebase at {x}", .{base_offset + offset});
                     try rebase_pointers.append(.{
                         .offset = base_offset + offset,
                         .segment_id = match.seg,
@@ -5747,33 +5762,53 @@ fn writeDyldInfoData(self: *MachO) !void {
                 }
 
                 for (atom.bindings.items) |binding| {
-                    const global = self.globals.values()[binding.global_index];
-                    const bind_sym = self.getSymbol(global);
+                    const bind_sym = self.getSymbol(binding.target);
+                    const bind_sym_name = self.getSymbolName(binding.target);
+                    const dylib_ordinal = @divTrunc(
+                        @bitCast(i16, bind_sym.n_desc),
+                        macho.N_SYMBOL_RESOLVER,
+                    );
                     var flags: u4 = 0;
+                    log.debug("    | bind at {x}, import('{s}') in dylib({d})", .{
+                        binding.offset + base_offset,
+                        bind_sym_name,
+                        dylib_ordinal,
+                    });
                     if (bind_sym.weakRef()) {
+                        log.debug("    | marking as weak ref ", .{});
                         flags |= @truncate(u4, macho.BIND_SYMBOL_FLAGS_WEAK_IMPORT);
                     }
                     try bind_pointers.append(.{
                         .offset = binding.offset + base_offset,
                         .segment_id = match.seg,
-                        .dylib_ordinal = @divTrunc(@bitCast(i16, bind_sym.n_desc), macho.N_SYMBOL_RESOLVER),
-                        .name = self.getSymbolName(global),
+                        .dylib_ordinal = dylib_ordinal,
+                        .name = bind_sym_name,
                         .bind_flags = flags,
                     });
                 }
 
                 for (atom.lazy_bindings.items) |binding| {
-                    const global = self.globals.values()[binding.global_index];
-                    const bind_sym = self.getSymbol(global);
+                    const bind_sym = self.getSymbol(binding.target);
+                    const bind_sym_name = self.getSymbolName(binding.target);
+                    const dylib_ordinal = @divTrunc(
+                        @bitCast(i16, bind_sym.n_desc),
+                        macho.N_SYMBOL_RESOLVER,
+                    );
                     var flags: u4 = 0;
+                    log.debug("    | lazy bind at {x} import('{s}') ord({d})", .{
+                        binding.offset + base_offset,
+                        bind_sym_name,
+                        dylib_ordinal,
+                    });
                     if (bind_sym.weakRef()) {
+                        log.debug("    | marking as weak ref ", .{});
                         flags |= @truncate(u4, macho.BIND_SYMBOL_FLAGS_WEAK_IMPORT);
                     }
                     try lazy_bind_pointers.append(.{
                         .offset = binding.offset + base_offset,
                         .segment_id = match.seg,
-                        .dylib_ordinal = @divTrunc(@bitCast(i16, bind_sym.n_desc), macho.N_SYMBOL_RESOLVER),
-                        .name = self.getSymbolName(global),
+                        .dylib_ordinal = dylib_ordinal,
+                        .name = bind_sym_name,
                         .bind_flags = flags,
                     });
                 }
diff --git a/src/link/MachO/Atom.zig b/src/link/MachO/Atom.zig
index ecf2f10149..e5a940bdda 100644
--- a/src/link/MachO/Atom.zig
+++ b/src/link/MachO/Atom.zig
@@ -71,7 +71,7 @@ dbg_info_atom: Dwarf.Atom,
 dirty: bool = true,
 
 pub const Binding = struct {
-    global_index: u32,
+    target: SymbolWithLoc,
     offset: u64,
 };
 
@@ -536,10 +536,8 @@ fn addPtrBindingOrRebase(
     const gpa = context.macho_file.base.allocator;
     const sym = context.macho_file.getSymbol(target);
     if (sym.undf()) {
-        const sym_name = context.macho_file.getSymbolName(target);
-        const global_index = @intCast(u32, context.macho_file.globals.getIndex(sym_name).?);
         try self.bindings.append(gpa, .{
-            .global_index = global_index,
+            .target = target,
             .offset = @intCast(u32, rel.r_address - context.base_offset),
         });
     } else {