authorgravatar for kubkon@jakubkonka.comJakub Konka <kubkon@jakubkonka.com> 2024-07-16 21:13:57+02:00
committergravatar for kubkon@jakubkonka.comJakub Konka <kubkon@jakubkonka.com> 2024-07-18 09:13:09+02:00
log34f34dbe3246547d4586d284cf44510e3f8feaa2
tree05602e26db6b788edc0e74eaa7b70ac98346de64
parentd19aab2e872df58c56a17ca7b9ee1ea9aab82b99

macho: reinstate duplicate definition checking

5 files changed, 86 insertions(+), 69 deletions(-)

src/link/MachO.zig+30-29
...@@ -26,6 +26,7 @@ resolver: SymbolResolver = .{},...@@ -26,6 +26,7 @@ resolver: SymbolResolver = .{},
26/// This table will be populated after `scanRelocs` has run.26/// This table will be populated after `scanRelocs` has run.
27/// Key is symbol index.27/// Key is symbol index.
28undefs: std.AutoHashMapUnmanaged(SymbolResolver.Index, std.ArrayListUnmanaged(Ref)) = .{},28undefs: std.AutoHashMapUnmanaged(SymbolResolver.Index, std.ArrayListUnmanaged(Ref)) = .{},
29dupes: std.AutoHashMapUnmanaged(SymbolResolver.Index, std.ArrayListUnmanaged(File.Index)) = .{},
2930
30dyld_info_cmd: macho.dyld_info_command = .{},31dyld_info_cmd: macho.dyld_info_command = .{},
31symtab_cmd: macho.symtab_command = .{},32symtab_cmd: macho.symtab_command = .{},
...@@ -311,6 +312,13 @@ pub fn deinit(self: *MachO) void {...@@ -311,6 +312,13 @@ pub fn deinit(self: *MachO) void {
311 }312 }
312 self.undefs.deinit(gpa);313 self.undefs.deinit(gpa);
313 }314 }
315 {
316 var it = self.dupes.iterator();
317 while (it.next()) |entry| {
318 entry.value_ptr.deinit(gpa);
319 }
320 self.dupes.deinit(gpa);
321 }
314322
315 self.symtab.deinit(gpa);323 self.symtab.deinit(gpa);
316 self.strtab.deinit(gpa);324 self.strtab.deinit(gpa);
...@@ -518,14 +526,13 @@ pub fn flushModule(self: *MachO, arena: Allocator, tid: Zcu.PerThread.Id, prog_n...@@ -518,14 +526,13 @@ pub fn flushModule(self: *MachO, arena: Allocator, tid: Zcu.PerThread.Id, prog_n
518 try dead_strip.gcAtoms(self);526 try dead_strip.gcAtoms(self);
519 }527 }
520528
521 // TODO529 self.checkDuplicates() catch |err| switch (err) {
522 // self.checkDuplicates() catch |err| switch (err) {530 error.HasDuplicates => return error.FlushFailure,
523 // error.HasDuplicates => return error.FlushFailure,531 else => |e| {
524 // else => |e| {532 try self.reportUnexpectedError("unexpected error while checking for duplicate symbol definitions", .{});
525 // try self.reportUnexpectedError("unexpected error while checking for duplicate symbol definitions", .{});533 return e;
526 // return e;534 },
527 // },535 };
528 // };
529536
530 self.markImportsAndExports();537 self.markImportsAndExports();
531 self.deadStripDylibs();538 self.deadStripDylibs();
...@@ -1434,25 +1441,16 @@ fn claimUnresolved(self: *MachO) void {...@@ -1434,25 +1441,16 @@ fn claimUnresolved(self: *MachO) void {
1434}1441}
14351442
1436fn checkDuplicates(self: *MachO) !void {1443fn checkDuplicates(self: *MachO) !void {
1437 const gpa = self.base.comp.gpa;
1438
1439 var dupes = std.AutoArrayHashMap(Symbol.Index, std.ArrayListUnmanaged(File.Index)).init(gpa);
1440 defer {
1441 for (dupes.values()) |*list| {
1442 list.deinit(gpa);
1443 }
1444 dupes.deinit();
1445 }
1446
1447 if (self.getZigObject()) |zo| {1444 if (self.getZigObject()) |zo| {
1448 try zo.checkDuplicates(&dupes, self);1445 try zo.asFile().checkDuplicates(self);
1449 }1446 }
1450
1451 for (self.objects.items) |index| {1447 for (self.objects.items) |index| {
1452 try self.getFile(index).?.object.checkDuplicates(&dupes, self);1448 try self.getFile(index).?.checkDuplicates(self);
1453 }1449 }
14541450 if (self.getInternalObject()) |obj| {
1455 try self.reportDuplicates(dupes);1451 try obj.asFile().checkDuplicates(self);
1452 }
1453 try self.reportDuplicates();
1456}1454}
14571455
1458fn markImportsAndExports(self: *MachO) void {1456fn markImportsAndExports(self: *MachO) void {
...@@ -3737,22 +3735,23 @@ pub fn reportUnexpectedError(self: *MachO, comptime format: []const u8, args: an...@@ -3737,22 +3735,23 @@ pub fn reportUnexpectedError(self: *MachO, comptime format: []const u8, args: an
3737 try err.addNote(self, "please report this as a linker bug on https://github.com/ziglang/zig/issues/new/choose", .{});3735 try err.addNote(self, "please report this as a linker bug on https://github.com/ziglang/zig/issues/new/choose", .{});
3738}3736}
37393737
3740fn reportDuplicates(self: *MachO, dupes: anytype) error{ HasDuplicates, OutOfMemory }!void {3738fn reportDuplicates(self: *MachO) error{ HasDuplicates, OutOfMemory }!void {
3741 const tracy = trace(@src());3739 const tracy = trace(@src());
3742 defer tracy.end();3740 defer tracy.end();
37433741
3744 const max_notes = 3;3742 const max_notes = 3;
37453743
3746 var has_dupes = false;3744 var has_dupes = false;
3747 var it = dupes.iterator();3745 var it = self.dupes.iterator();
3748 while (it.next()) |entry| {3746 while (it.next()) |entry| {
3749 const sym = self.getSymbol(entry.key_ptr.*);3747 const sym = self.resolver.keys.items[entry.key_ptr.* - 1];
3750 const notes = entry.value_ptr.*;3748 const notes = entry.value_ptr.*;
3751 const nnotes = @min(notes.items.len, max_notes) + @intFromBool(notes.items.len > max_notes);3749 const nnotes = @min(notes.items.len, max_notes) + @intFromBool(notes.items.len > max_notes);
37523750
3753 var err = try self.addErrorWithNotes(nnotes + 1);3751 var err = try self.addErrorWithNotes(nnotes + 1);
3754 try err.addMsg(self, "duplicate symbol definition: {s}", .{sym.getName(self)});3752 try err.addMsg(self, "duplicate symbol definition: {s}", .{sym.getName(self)});
3755 try err.addNote(self, "defined by {}", .{sym.getFile(self).?.fmtPath()});3753 try err.addNote(self, "defined by {}", .{sym.getFile(self).?.fmtPath()});
3754 has_dupes = true;
37563755
3757 var inote: usize = 0;3756 var inote: usize = 0;
3758 while (inote < @min(notes.items.len, max_notes)) : (inote += 1) {3757 while (inote < @min(notes.items.len, max_notes)) : (inote += 1) {
...@@ -3764,10 +3763,7 @@ fn reportDuplicates(self: *MachO, dupes: anytype) error{ HasDuplicates, OutOfMem...@@ -3764,10 +3763,7 @@ fn reportDuplicates(self: *MachO, dupes: anytype) error{ HasDuplicates, OutOfMem
3764 const remaining = notes.items.len - max_notes;3763 const remaining = notes.items.len - max_notes;
3765 try err.addNote(self, "defined {d} more times", .{remaining});3764 try err.addNote(self, "defined {d} more times", .{remaining});
3766 }3765 }
3767
3768 has_dupes = true;
3769 }3766 }
3770
3771 if (has_dupes) return error.HasDuplicates;3767 if (has_dupes) return error.HasDuplicates;
3772}3768}
37733769
...@@ -4452,6 +4448,11 @@ pub const SymbolResolver = struct {...@@ -4452,6 +4448,11 @@ pub const SymbolResolver = struct {
4452 return ref.getSymbol(macho_file).?.getName(macho_file);4448 return ref.getSymbol(macho_file).?.getName(macho_file);
4453 }4449 }
44544450
4451 pub fn getFile(key: Key, macho_file: *MachO) ?File {
4452 const ref = Ref{ .index = key.index, .file = key.file };
4453 return ref.getFile(macho_file);
4454 }
4455
4455 fn eql(key: Key, other: Key, macho_file: *MachO) bool {4456 fn eql(key: Key, other: Key, macho_file: *MachO) bool {
4456 const key_name = key.getName(macho_file);4457 const key_name = key.getName(macho_file);
4457 const other_name = other.getName(macho_file);4458 const other_name = other.getName(macho_file);
src/link/MachO/Object.zig-19
...@@ -1550,25 +1550,6 @@ pub fn mergeSymbolVisibility(self: *Object, macho_file: *MachO) void {...@@ -1550,25 +1550,6 @@ pub fn mergeSymbolVisibility(self: *Object, macho_file: *MachO) void {
1550 }1550 }
1551}1551}
15521552
1553// TODO
1554// pub fn checkDuplicates(self: *Object, dupes: anytype, macho_file: *MachO) error{OutOfMemory}!void {
1555// for (self.symbols.items, 0..) |index, nlist_idx| {
1556// const sym = macho_file.getSymbol(index);
1557// if (sym.visibility != .global) continue;
1558// const file = sym.getFile(macho_file) orelse continue;
1559// if (file.getIndex() == self.index) continue;
1560
1561// const nlist = self.symtab.items(.nlist)[nlist_idx];
1562// if (!nlist.undf() and !nlist.tentative() and !(nlist.weakDef() or nlist.pext())) {
1563// const gop = try dupes.getOrPut(index);
1564// if (!gop.found_existing) {
1565// gop.value_ptr.* = .{};
1566// }
1567// try gop.value_ptr.append(macho_file.base.comp.gpa, self.index);
1568// }
1569// }
1570// }
1571
1572pub fn scanRelocs(self: *Object, macho_file: *MachO) !void {1553pub fn scanRelocs(self: *Object, macho_file: *MachO) !void {
1573 const tracy = trace(@src());1554 const tracy = trace(@src());
1574 defer tracy.end();1555 defer tracy.end();
src/link/MachO/ZigObject.zig-19
...@@ -308,25 +308,6 @@ pub fn mergeSymbolVisibility(self: *ZigObject, macho_file: *MachO) void {...@@ -308,25 +308,6 @@ pub fn mergeSymbolVisibility(self: *ZigObject, macho_file: *MachO) void {
308 }308 }
309}309}
310310
311// TODO
312// pub fn checkDuplicates(self: *ZigObject, dupes: anytype, macho_file: *MachO) !void {
313// for (self.symbols.items, 0..) |index, nlist_idx| {
314// const sym = macho_file.getSymbol(index);
315// if (sym.visibility != .global) continue;
316// const file = sym.getFile(macho_file) orelse continue;
317// if (file.getIndex() == self.index) continue;
318
319// const nlist = self.symtab.items(.nlist)[nlist_idx];
320// if (!nlist.undf() and !nlist.tentative() and !(nlist.weakDef() or nlist.pext())) {
321// const gop = try dupes.getOrPut(index);
322// if (!gop.found_existing) {
323// gop.value_ptr.* = .{};
324// }
325// try gop.value_ptr.append(macho_file.base.comp.gpa, self.index);
326// }
327// }
328// }
329
330pub fn resolveLiterals(self: *ZigObject, lp: *MachO.LiteralPool, macho_file: *MachO) !void {311pub fn resolveLiterals(self: *ZigObject, lp: *MachO.LiteralPool, macho_file: *MachO) !void {
331 _ = self;312 _ = self;
332 _ = lp;313 _ = lp;
src/link/MachO/file.zig+24-2
...@@ -24,7 +24,7 @@ pub const File = union(enum) {...@@ -24,7 +24,7 @@ pub const File = union(enum) {
24 _ = options;24 _ = options;
25 switch (file) {25 switch (file) {
26 .zig_object => |x| try writer.writeAll(x.path),26 .zig_object => |x| try writer.writeAll(x.path),
27 .internal => try writer.writeAll(""),27 .internal => try writer.writeAll("internal"),
28 .object => |x| try writer.print("{}", .{x.fmtPath()}),28 .object => |x| try writer.print("{}", .{x.fmtPath()}),
29 .dylib => |x| try writer.writeAll(x.path),29 .dylib => |x| try writer.writeAll(x.path),
30 }30 }
...@@ -57,7 +57,7 @@ pub const File = union(enum) {...@@ -57,7 +57,7 @@ pub const File = union(enum) {
57 weak: bool = false,57 weak: bool = false,
58 tentative: bool = false,58 tentative: bool = false,
59 }) u32 {59 }) u32 {
60 if (file == .object and !args.archive) {60 if (file != .dylib and !args.archive) {
61 const base: u32 = blk: {61 const base: u32 = blk: {
62 if (args.tentative) break :blk 3;62 if (args.tentative) break :blk 3;
63 break :blk if (args.weak) 2 else 1;63 break :blk if (args.weak) 2 else 1;
...@@ -254,6 +254,28 @@ pub const File = union(enum) {...@@ -254,6 +254,28 @@ pub const File = union(enum) {
254 }254 }
255 }255 }
256256
257 pub fn checkDuplicates(file: File, macho_file: *MachO) !void {
258 const tracy = trace(@src());
259 defer tracy.end();
260
261 const gpa = macho_file.base.comp.gpa;
262
263 for (file.getSymbols(), file.getNlists(), 0..) |sym, nlist, i| {
264 if (sym.visibility != .global) continue;
265 if (sym.flags.weak) continue;
266 if (nlist.undf()) continue;
267 const ref = file.getSymbolRef(@intCast(i), macho_file);
268 const ref_file = ref.getFile(macho_file) orelse continue;
269 if (ref_file.getIndex() == file.getIndex()) continue;
270
271 const gop = try macho_file.dupes.getOrPut(gpa, file.getGlobals()[i]);
272 if (!gop.found_existing) {
273 gop.value_ptr.* = .{};
274 }
275 try gop.value_ptr.append(gpa, file.getIndex());
276 }
277 }
278
257 pub fn initOutputSections(file: File, macho_file: *MachO) !void {279 pub fn initOutputSections(file: File, macho_file: *MachO) !void {
258 const tracy = trace(@src());280 const tracy = trace(@src());
259 defer tracy.end();281 defer tracy.end();
test/link/macho.zig+32
...@@ -30,6 +30,7 @@ pub fn testAll(b: *Build, build_opts: BuildOptions) *Step {...@@ -30,6 +30,7 @@ pub fn testAll(b: *Build, build_opts: BuildOptions) *Step {
3030
31 // Exercise linker with LLVM backend31 // Exercise linker with LLVM backend
32 macho_step.dependOn(testDeadStrip(b, .{ .target = default_target }));32 macho_step.dependOn(testDeadStrip(b, .{ .target = default_target }));
33 macho_step.dependOn(testDuplicateDefinitions(b, .{ .target = default_target }));
33 macho_step.dependOn(testEmptyObject(b, .{ .target = default_target }));34 macho_step.dependOn(testEmptyObject(b, .{ .target = default_target }));
34 macho_step.dependOn(testEmptyZig(b, .{ .target = default_target }));35 macho_step.dependOn(testEmptyZig(b, .{ .target = default_target }));
35 macho_step.dependOn(testEntryPoint(b, .{ .target = default_target }));36 macho_step.dependOn(testEntryPoint(b, .{ .target = default_target }));
...@@ -182,6 +183,37 @@ fn testDeadStrip(b: *Build, opts: Options) *Step {...@@ -182,6 +183,37 @@ fn testDeadStrip(b: *Build, opts: Options) *Step {
182 return test_step;183 return test_step;
183}184}
184185
186fn testDuplicateDefinitions(b: *Build, opts: Options) *Step {
187 const test_step = addTestStep(b, "duplicate-definitions", opts);
188
189 const obj = addObject(b, opts, .{ .name = "a", .zig_source_bytes =
190 \\var x: usize = 1;
191 \\export fn strong() void { x += 1; }
192 \\export fn weak() void { x += 1; }
193 });
194
195 const exe = addExecutable(b, opts, .{ .name = "main", .zig_source_bytes =
196 \\var x: usize = 1;
197 \\export fn strong() void { x += 1; }
198 \\comptime { @export(weakImpl, .{ .name = "weak", .linkage = .weak }); }
199 \\fn weakImpl() callconv(.C) void { x += 1; }
200 \\extern fn weak() void;
201 \\pub fn main() void {
202 \\ weak();
203 \\ strong();
204 \\}
205 });
206 exe.addObject(obj);
207
208 expectLinkErrors(exe, test_step, .{ .exact = &.{
209 "error: duplicate symbol definition: _strong",
210 "note: defined by /?/a.o",
211 "note: defined by /?/main.o",
212 } });
213
214 return test_step;
215}
216
185fn testDeadStripDylibs(b: *Build, opts: Options) *Step {217fn testDeadStripDylibs(b: *Build, opts: Options) *Step {
186 const test_step = addTestStep(b, "dead-strip-dylibs", opts);218 const test_step = addTestStep(b, "dead-strip-dylibs", opts);
187219