macho: report duplicate symbols

kubkon · Jul 17, 2024 · bfce77c · bfce77c
1 parent dfabc65
commit bfce77c
Show file tree

Hide file tree

Showing 3 changed files with 131 additions and 3 deletions.
diff --git a/src/MachO.zig b/src/MachO.zig
@@ -21,11 +21,15 @@ segments: std.ArrayListUnmanaged(macho.segment_command_64) = .{},
 sections: std.MultiArrayList(Section) = .{},
 
 resolver: SymbolResolver = .{},
+
 /// This table will be populated after `scanRelocs` has run.
 /// Key is symbol index.
 undefs: std.AutoHashMapUnmanaged(Ref, std.ArrayListUnmanaged(Ref)) = .{},
 undefs_mutex: std.Thread.Mutex = .{},
 
+dupes: std.AutoHashMapUnmanaged(SymbolResolver.Index, std.ArrayListUnmanaged(File.Index)) = .{},
+dupes_mutex: std.Thread.Mutex = .{},
+
 pagezero_seg_index: ?u8 = null,
 text_seg_index: ?u8 = null,
 linkedit_seg_index: ?u8 = null,
@@ -102,7 +106,20 @@ pub fn deinit(self: *MachO) void {
     self.file_handles.deinit(gpa);
 
     self.resolver.deinit(gpa);
-    self.undefs.deinit(gpa);
+    {
+        var it = self.undefs.iterator();
+        while (it.next()) |entry| {
+            entry.value_ptr.deinit(gpa);
+        }
+        self.undefs.deinit(gpa);
+    }
+    {
+        var it = self.dupes.iterator();
+        while (it.next()) |entry| {
+            entry.value_ptr.deinit(gpa);
+        }
+        self.dupes.deinit(gpa);
+    }
 
     self.objects.deinit(gpa);
     self.dylibs.deinit(gpa);
@@ -259,6 +276,8 @@ pub fn flush(self: *MachO) !void {
         try dead_strip.gcAtoms(self);
     }
 
+    try self.checkDuplicates();
+
     self.markImportsAndExports();
     self.deadStripDylibs();
 
@@ -1129,6 +1148,71 @@ fn claimUnresolved(self: *MachO) void {
     }
 }
 
+fn checkDuplicates(self: *MachO) !void {
+    const tracy = trace(@src());
+    defer tracy.end();
+
+    var wg: WaitGroup = .{};
+    {
+        wg.reset();
+        defer wg.wait();
+        for (self.objects.items) |index| {
+            self.base.thread_pool.spawnWg(&wg, checkDuplicatesWorker, .{ self, self.getFile(index).? });
+        }
+        if (self.getInternalObject()) |object| {
+            self.base.thread_pool.spawnWg(&wg, checkDuplicatesWorker, .{ self, object.asFile() });
+        }
+    }
+
+    if (self.has_errors.swap(false, .seq_cst)) return error.FlushFailed;
+
+    try self.reportDuplicates();
+}
+
+fn checkDuplicatesWorker(self: *MachO, file: File) void {
+    const tracy = trace(@src());
+    defer tracy.end();
+    file.checkDuplicates(self) catch |err| {
+        self.base.fatal("{}: failed to check duplicate definitions: {s}", .{
+            file.fmtPath(),
+            @errorName(err),
+        });
+        _ = self.has_errors.swap(true, .seq_cst);
+    };
+}
+
+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 = self.dupes.iterator();
+    while (it.next()) |entry| {
+        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.base.addErrorWithNotes(nnotes + 1);
+        try err.addMsg("duplicate symbol definition: {s}", .{sym.getName(self)});
+        try err.addNote("defined by {}", .{sym.getFile(self).?.fmtPath()});
+        has_dupes = true;
+
+        var inote: usize = 0;
+        while (inote < @min(notes.items.len, max_notes)) : (inote += 1) {
+            const file = self.getFile(notes.items[inote]).?;
+            try err.addNote("defined by {}", .{file.fmtPath()});
+        }
+
+        if (notes.items.len > max_notes) {
+            const remaining = notes.items.len - max_notes;
+            try err.addNote("defined {d} more times", .{remaining});
+        }
+    }
+    if (has_dupes) return error.HasDuplicates;
+}
+
 fn scanRelocs(self: *MachO) !void {
     const tracy = trace(@src());
     defer tracy.end();
@@ -3042,6 +3126,11 @@ pub const SymbolResolver = struct {
             return ref.getSymbol(macho_file).?.getName(macho_file);
         }
 
+        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/MachO/file.zig b/src/MachO/file.zig
@@ -34,6 +34,31 @@ 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.allocator;
+
+        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;
+
+            macho_file.dupes_mutex.lock();
+            defer macho_file.dupes_mutex.unlock();
+
+            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 scanRelocs(file: File, macho_file: *MachO) !void {
         switch (file) {
             .dylib => unreachable,
@@ -55,7 +80,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;
@@ -116,6 +141,20 @@ pub const File = union(enum) {
         };
     }
 
+    pub fn getNlists(file: File) []macho.nlist_64 {
+        return switch (file) {
+            .dylib => unreachable,
+            .internal => |x| x.symtab.items,
+            .object => |x| x.symtab.items(.nlist),
+        };
+    }
+
+    pub fn getGlobals(file: File) []MachO.SymbolResolver.Index {
+        return switch (file) {
+            inline else => |x| x.globals.items,
+        };
+    }
+
     pub fn markImportsAndExports(file: File, macho_file: *MachO) void {
         const nsyms = switch (file) {
             .dylib => unreachable,

diff --git a/src/main.zig b/src/main.zig
@@ -118,7 +118,7 @@ pub fn main() !void {
         error.FlushFailed,
         error.InferCpuFailed,
         error.ParseFailed,
-        error.MultipleSymbolDefinition,
+        error.HasDuplicates,
         error.UndefinedSymbols,
         error.RelocError,
         error.ResolveFailed,