From 7a96907b9233cd1385472a03ff75530d3a02556f Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Sun, 14 Jan 2024 19:20:11 +0100 Subject: [PATCH 1/4] elf: check for and report duplicate symbol definitions --- src/link/Elf.zig | 56 ++++++++++++++++++++++++++++++++++++++ src/link/Elf/Object.zig | 30 ++++++++++---------- src/link/Elf/ZigObject.zig | 26 ++++++++++++++++++ 3 files changed, 97 insertions(+), 15 deletions(-) diff --git a/src/link/Elf.zig b/src/link/Elf.zig index ef06c8e1dd..6ecafae009 100644 --- a/src/link/Elf.zig +++ b/src/link/Elf.zig @@ -1323,6 +1323,11 @@ pub fn flushModule(self: *Elf, arena: Allocator, prog_node: *std.Progress.Node) } } + self.checkDuplicates() catch |err| switch (err) { + error.HasDuplicates => return error.FlushFailure, + else => |e| return e, + }; + try self.initOutputSections(); try self.addLinkerDefinedSymbols(); self.claimUnresolved(); @@ -3491,6 +3496,27 @@ fn allocateLinkerDefinedSymbols(self: *Elf) void { } } +fn checkDuplicates(self: *Elf) !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.zigObjectPtr()) |zig_object| { + try zig_object.checkDuplicates(&dupes, self); + } + for (self.objects.items) |index| { + try self.file(index).?.object.checkDuplicates(&dupes, self); + } + + try self.reportDuplicates(dupes); +} + fn initOutputSections(self: *Elf) !void { for (self.objects.items) |index| { try self.file(index).?.object.initOutputSections(self); @@ -6164,6 +6190,36 @@ fn reportUndefinedSymbols(self: *Elf, undefs: anytype) !void { } } +fn reportDuplicates(self: *Elf, dupes: anytype) error{ HasDuplicates, OutOfMemory }!void { + const max_notes = 3; + var has_dupes = false; + var it = dupes.iterator(); + while (it.next()) |entry| { + const sym = self.symbol(entry.key_ptr.*); + 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.name(self)}); + try err.addNote(self, "defined by {}", .{sym.file(self).?.fmtPath()}); + + var inote: usize = 0; + while (inote < @min(notes.items.len, max_notes)) : (inote += 1) { + const file_ptr = self.file(notes.items[inote]).?; + try err.addNote(self, "defined by {}", .{file_ptr.fmtPath()}); + } + + if (notes.items.len > max_notes) { + 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; +} + fn reportMissingLibraryError( self: *Elf, checked_paths: []const []const u8, diff --git a/src/link/Elf/Object.zig b/src/link/Elf/Object.zig index b6dced258c..bdb9ae2911 100644 --- a/src/link/Elf/Object.zig +++ b/src/link/Elf/Object.zig @@ -581,30 +581,30 @@ pub fn markEhFrameAtomsDead(self: Object, elf_file: *Elf) void { } } -pub fn checkDuplicates(self: *Object, elf_file: *Elf) void { +pub fn checkDuplicates(self: *Object, dupes: anytype, elf_file: *Elf) error{OutOfMemory}!void { const first_global = self.first_global orelse return; for (self.globals(), 0..) |index, i| { - const sym_idx = @as(u32, @intCast(first_global + i)); - const this_sym = self.symtab.items[sym_idx]; + const sym_idx = first_global + i; + const sym = self.symtab.items[sym_idx]; const global = elf_file.symbol(index); - const global_file = global.getFile(elf_file) orelse continue; + const global_file = global.file(elf_file) orelse continue; - if (self.index == global_file.getIndex() or - this_sym.st_shndx == elf.SHN_UNDEF or - this_sym.st_bind() == elf.STB_WEAK or - this_sym.st_shndx == elf.SHN_COMMON) continue; + if (self.index == global_file.index() or + sym.st_shndx == elf.SHN_UNDEF or + sym.st_bind() == elf.STB_WEAK or + sym.st_shndx == elf.SHN_COMMON) continue; - if (this_sym.st_shndx != elf.SHN_ABS) { - const atom_index = self.atoms.items[this_sym.st_shndx]; + if (sym.st_shndx != elf.SHN_ABS) { + const atom_index = self.atoms.items[sym.st_shndx]; const atom = elf_file.atom(atom_index) orelse continue; if (!atom.flags.alive) continue; } - elf_file.base.fatal("multiple definition: {}: {}: {s}", .{ - self.fmtPath(), - global_file.fmtPath(), - global.getName(elf_file), - }); + const gop = try dupes.getOrPut(index); + if (!gop.found_existing) { + gop.value_ptr.* = .{}; + } + try gop.value_ptr.append(elf_file.base.comp.gpa, self.index); } } diff --git a/src/link/Elf/ZigObject.zig b/src/link/Elf/ZigObject.zig index a29c3ab69c..ef8509e915 100644 --- a/src/link/Elf/ZigObject.zig +++ b/src/link/Elf/ZigObject.zig @@ -451,6 +451,32 @@ pub fn markLive(self: *ZigObject, elf_file: *Elf) void { } } +pub fn checkDuplicates(self: *ZigObject, dupes: anytype, elf_file: *Elf) error{OutOfMemory}!void { + for (self.globals(), 0..) |index, i| { + const esym = self.global_esyms.items(.elf_sym)[i]; + const shndx = self.global_esyms.items(.shndx)[i]; + const global = elf_file.symbol(index); + const global_file = global.file(elf_file) orelse continue; + + if (self.index == global_file.index() or + esym.st_shndx == elf.SHN_UNDEF or + esym.st_bind() == elf.STB_WEAK or + esym.st_shndx == elf.SHN_COMMON) continue; + + if (esym.st_shndx == SHN_ATOM) { + const atom_index = self.atoms.items[shndx]; + const atom = elf_file.atom(atom_index) orelse continue; + if (!atom.flags.alive) continue; + } + + const gop = try dupes.getOrPut(index); + if (!gop.found_existing) { + gop.value_ptr.* = .{}; + } + try gop.value_ptr.append(elf_file.base.comp.gpa, self.index); + } +} + /// This is just a temporary helper function that allows us to re-read what we wrote to file into a buffer. /// We need this so that we can write to an archive. /// TODO implement writing ZigObject data directly to a buffer instead. From d7c2324cdbcb32e285e03aa7938529b9412c72e6 Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Sun, 14 Jan 2024 20:51:03 +0100 Subject: [PATCH 2/4] test/link/elf: trigger build system bug testing relocatable mode --- test/link/elf.zig | 63 +++++++++++++++++++++++------------------------ 1 file changed, 31 insertions(+), 32 deletions(-) diff --git a/test/link/elf.zig b/test/link/elf.zig index 92aba4f352..8c22d0ad7f 100644 --- a/test/link/elf.zig +++ b/test/link/elf.zig @@ -695,41 +695,40 @@ fn testDsoUndef(b: *Build, opts: Options) *Step { fn testEmitRelocatable(b: *Build, opts: Options) *Step { const test_step = addTestStep(b, "emit-relocatable", opts); - const obj1 = addObject(b, opts, .{ - .name = "obj1", - .zig_source_bytes = - \\const std = @import("std"); - \\extern var bar: i32; - \\export fn foo() i32 { - \\ return bar; - \\} - \\export fn printFoo() void { - \\ std.debug.print("foo={d}\n", .{foo()}); - \\} - , - .c_source_bytes = - \\#include - \\int bar = 42; - \\void printBar() { - \\ fprintf(stderr, "bar=%d\n", bar); - \\} - , + const a_o = addObject(b, opts, .{ .name = "a", .zig_source_bytes = + \\const std = @import("std"); + \\extern var bar: i32; + \\export fn foo() i32 { + \\ return bar; + \\} + \\export fn printFoo() void { + \\ std.debug.print("foo={d}\n", .{foo()}); + \\} }); - obj1.linkLibC(); - const exe = addExecutable(b, opts, .{ - .name = "test", - .zig_source_bytes = - \\const std = @import("std"); - \\extern fn printFoo() void; - \\extern fn printBar() void; - \\pub fn main() void { - \\ printFoo(); - \\ printBar(); - \\} - , + const b_o = addObject(b, opts, .{ .name = "b", .c_source_bytes = + \\#include + \\int bar = 42; + \\void printBar() { + \\ fprintf(stderr, "bar=%d\n", bar); + \\} }); - exe.addObject(obj1); + b_o.linkLibC(); + + const c_o = addObject(b, opts, .{ .name = "c" }); + c_o.addObject(a_o); + c_o.addObject(b_o); + + const exe = addExecutable(b, opts, .{ .name = "test", .zig_source_bytes = + \\const std = @import("std"); + \\extern fn printFoo() void; + \\extern fn printBar() void; + \\pub fn main() void { + \\ printFoo(); + \\ printBar(); + \\} + }); + exe.addObject(c_o); exe.linkLibC(); const run = addRunArtifact(exe); From a8b9f0cf2268e0be9551a7d14f76e5e4f1b79527 Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Sun, 14 Jan 2024 23:07:21 +0100 Subject: [PATCH 3/4] std/Build/Step/Compile: do not propagate deps of complex addObject step --- lib/std/Build/Step/Compile.zig | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/std/Build/Step/Compile.zig b/lib/std/Build/Step/Compile.zig index 839a5f2b07..efa5129d97 100644 --- a/lib/std/Build/Step/Compile.zig +++ b/lib/std/Build/Step/Compile.zig @@ -1077,9 +1077,8 @@ fn make(step: *Step, prog_node: *std.Progress.Node) !void { .exe => return step.fail("cannot link with an executable build artifact", .{}), .@"test" => return step.fail("cannot link with a test", .{}), .obj => { - const included_in_lib = !my_responsibility and - compile.kind == .lib and other.kind == .obj; - if (!already_linked and !included_in_lib) { + const included_in_lib_or_obj = !my_responsibility and (compile.kind == .lib or compile.kind == .obj); + if (!already_linked and !included_in_lib_or_obj) { try zig_args.append(other.getEmittedBin().getPath(b)); total_linker_objects += 1; } From b1ffc2b8b353f64362c27df2ecc44436db234a29 Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Mon, 15 Jan 2024 00:18:50 +0100 Subject: [PATCH 4/4] test/link/elf: patch up relocatable test --- test/link/elf.zig | 1 + 1 file changed, 1 insertion(+) diff --git a/test/link/elf.zig b/test/link/elf.zig index 8c22d0ad7f..0d76a5b64b 100644 --- a/test/link/elf.zig +++ b/test/link/elf.zig @@ -705,6 +705,7 @@ fn testEmitRelocatable(b: *Build, opts: Options) *Step { \\ std.debug.print("foo={d}\n", .{foo()}); \\} }); + a_o.linkLibC(); const b_o = addObject(b, opts, .{ .name = "b", .c_source_bytes = \\#include