macho: reinstate duplicate definition checking

4 files changed, 54 insertions, 69 deletions

diff --git a/src/link/MachO.zig b/src/link/MachO.zig
index 6061de7d14..df11df30a1 100644
--- a/src/link/MachO.zig
+++ b/src/link/MachO.zig
@@ -26,6 +26,7 @@ resolver: SymbolResolver = .{},
 /// This table will be populated after `scanRelocs` has run.
 /// Key is symbol index.
 undefs: std.AutoHashMapUnmanaged(SymbolResolver.Index, std.ArrayListUnmanaged(Ref)) = .{},
+dupes: std.AutoHashMapUnmanaged(SymbolResolver.Index, std.ArrayListUnmanaged(File.Index)) = .{},
 
 dyld_info_cmd: macho.dyld_info_command = .{},
 symtab_cmd: macho.symtab_command = .{},
@@ -311,6 +312,13 @@ pub fn deinit(self: *MachO) void {
         }
         self.undefs.deinit(gpa);
     }
+    {
+        var it = self.dupes.iterator();
+        while (it.next()) |entry| {
+            entry.value_ptr.deinit(gpa);
+        }
+        self.dupes.deinit(gpa);
+    }
 
     self.symtab.deinit(gpa);
     self.strtab.deinit(gpa);
@@ -518,14 +526,13 @@ pub fn flushModule(self: *MachO, arena: Allocator, tid: Zcu.PerThread.Id, prog_n
         try dead_strip.gcAtoms(self);
     }
 
-    // TODO
-    // self.checkDuplicates() catch |err| switch (err) {
-    //     error.HasDuplicates => return error.FlushFailure,
-    //     else => |e| {
-    //         try self.reportUnexpectedError("unexpected error while checking for duplicate symbol definitions", .{});
-    //         return e;
-    //     },
-    // };
+    self.checkDuplicates() catch |err| switch (err) {
+        error.HasDuplicates => return error.FlushFailure,
+        else => |e| {
+            try self.reportUnexpectedError("unexpected error while checking for duplicate symbol definitions", .{});
+            return e;
+        },
+    };
 
     self.markImportsAndExports();
     self.deadStripDylibs();
@@ -1434,25 +1441,16 @@ fn claimUnresolved(self: *MachO) void {
 }
 
 fn checkDuplicates(self: *MachO) !void {
-    const gpa = self.base.comp.gpa;
-
-    var dupes = std.AutoArrayHashMap(Symbol.Index, std.ArrayListUnmanaged(File.Index)).init(gpa);
-    defer {
-        for (dupes.values()) |*list| {
-            list.deinit(gpa);
-        }
-        dupes.deinit();
-    }
-
     if (self.getZigObject()) |zo| {
-        try zo.checkDuplicates(&dupes, self);
+        try zo.asFile().checkDuplicates(self);
     }
-
     for (self.objects.items) |index| {
-        try self.getFile(index).?.object.checkDuplicates(&dupes, self);
+        try self.getFile(index).?.checkDuplicates(self);
     }
-
-    try self.reportDuplicates(dupes);
+    if (self.getInternalObject()) |obj| {
+        try obj.asFile().checkDuplicates(self);
+    }
+    try self.reportDuplicates();
 }
 
 fn markImportsAndExports(self: *MachO) void {
@@ -3737,22 +3735,23 @@ pub fn reportUnexpectedError(self: *MachO, comptime format: []const u8, args: an
     try err.addNote(self, "please report this as a linker bug on https://github.com/ziglang/zig/issues/new/choose", .{});
 }
 
-fn reportDuplicates(self: *MachO, dupes: anytype) error{ HasDuplicates, OutOfMemory }!void {
+fn reportDuplicates(self: *MachO) error{ HasDuplicates, OutOfMemory }!void {
     const tracy = trace(@src());
     defer tracy.end();
 
     const max_notes = 3;
 
     var has_dupes = false;
-    var it = dupes.iterator();
+    var it = self.dupes.iterator();
     while (it.next()) |entry| {
-        const sym = self.getSymbol(entry.key_ptr.*);
+        const sym = self.resolver.keys.items[entry.key_ptr.* - 1];
         const notes = entry.value_ptr.*;
         const nnotes = @min(notes.items.len, max_notes) + @intFromBool(notes.items.len > max_notes);
 
         var err = try self.addErrorWithNotes(nnotes + 1);
         try err.addMsg(self, "duplicate symbol definition: {s}", .{sym.getName(self)});
         try err.addNote(self, "defined by {}", .{sym.getFile(self).?.fmtPath()});
+        has_dupes = true;
 
         var inote: usize = 0;
         while (inote < @min(notes.items.len, max_notes)) : (inote += 1) {
@@ -3764,10 +3763,7 @@ fn reportDuplicates(self: *MachO, dupes: anytype) error{ HasDuplicates, OutOfMem
             const remaining = notes.items.len - max_notes;
             try err.addNote(self, "defined {d} more times", .{remaining});
         }
-
-        has_dupes = true;
     }
-
     if (has_dupes) return error.HasDuplicates;
 }
 
@@ -4452,6 +4448,11 @@ pub const SymbolResolver = struct {
             return ref.getSymbol(macho_file).?.getName(macho_file);
         }
 
+        pub fn getFile(key: Key, macho_file: *MachO) ?File {
+            const ref = Ref{ .index = key.index, .file = key.file };
+            return ref.getFile(macho_file);
+        }
+
         fn eql(key: Key, other: Key, macho_file: *MachO) bool {
             const key_name = key.getName(macho_file);
             const other_name = other.getName(macho_file);
diff --git a/src/link/MachO/Object.zig b/src/link/MachO/Object.zig
index eda7ac91f1..10d987b044 100644
--- a/src/link/MachO/Object.zig
+++ b/src/link/MachO/Object.zig
@@ -1550,25 +1550,6 @@ pub fn mergeSymbolVisibility(self: *Object, macho_file: *MachO) void {
     }
 }
 
-// TODO
-// pub fn checkDuplicates(self: *Object, dupes: anytype, macho_file: *MachO) error{OutOfMemory}!void {
-//     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: *Object, macho_file: *MachO) !void {
     const tracy = trace(@src());
     defer tracy.end();
diff --git a/src/link/MachO/ZigObject.zig b/src/link/MachO/ZigObject.zig
index d65cef5c10..f8ec5171f4 100644
--- a/src/link/MachO/ZigObject.zig
+++ b/src/link/MachO/ZigObject.zig
@@ -308,25 +308,6 @@ pub fn mergeSymbolVisibility(self: *ZigObject, macho_file: *MachO) void {
     }
 }
 
-// TODO
-// pub fn checkDuplicates(self: *ZigObject, dupes: anytype, macho_file: *MachO) !void {
-//     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 resolveLiterals(self: *ZigObject, lp: *MachO.LiteralPool, macho_file: *MachO) !void {
     _ = self;
     _ = lp;
diff --git a/src/link/MachO/file.zig b/src/link/MachO/file.zig
index a035c79ffb..b9d9d407c9 100644
--- a/src/link/MachO/file.zig
+++ b/src/link/MachO/file.zig
@@ -24,7 +24,7 @@ pub const File = union(enum) {
         _ = options;
         switch (file) {
             .zig_object => |x| try writer.writeAll(x.path),
-            .internal => try writer.writeAll(""),
+            .internal => try writer.writeAll("internal"),
             .object => |x| try writer.print("{}", .{x.fmtPath()}),
             .dylib => |x| try writer.writeAll(x.path),
         }
@@ -57,7 +57,7 @@ pub const File = union(enum) {
         weak: bool = false,
         tentative: bool = false,
     }) u32 {
-        if (file == .object and !args.archive) {
+        if (file != .dylib and !args.archive) {
             const base: u32 = blk: {
                 if (args.tentative) break :blk 3;
                 break :blk if (args.weak) 2 else 1;
@@ -254,6 +254,28 @@ pub const File = union(enum) {
         }
     }
 
+    pub fn checkDuplicates(file: File, macho_file: *MachO) !void {
+        const tracy = trace(@src());
+        defer tracy.end();
+
+        const gpa = macho_file.base.comp.gpa;
+
+        for (file.getSymbols(), file.getNlists(), 0..) |sym, nlist, i| {
+            if (sym.visibility != .global) continue;
+            if (sym.flags.weak) continue;
+            if (nlist.undf()) continue;
+            const ref = file.getSymbolRef(@intCast(i), macho_file);
+            const ref_file = ref.getFile(macho_file) orelse continue;
+            if (ref_file.getIndex() == file.getIndex()) continue;
+
+            const gop = try macho_file.dupes.getOrPut(gpa, file.getGlobals()[i]);
+            if (!gop.found_existing) {
+                gop.value_ptr.* = .{};
+            }
+            try gop.value_ptr.append(gpa, file.getIndex());
+        }
+    }
+
     pub fn initOutputSections(file: File, macho_file: *MachO) !void {
         const tracy = trace(@src());
         defer tracy.end();