From b3cd38ea4a7520fabbb05d3d2e74351c7c8effdb Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Mon, 24 Oct 2022 16:48:15 -0700 Subject: [PATCH] link: add an explicit error set for flush() and flushModule() This makes it easier to understand how control flow should happen in various cases; already just by doing this it is revealed that UndefinedSymbol and UndefinedSymbolReference should be merged, and that MissingMainEntrypoint should be removed in favor of the ErrorFlags mechanism thath we already have for missing the main entrypoint. The main motivation for this change, however, is preventing a compile error when there is conditional compilation inside linker implementations, causing the flush() error set to depend on compilation options. With this change, the error set is fixed, and, notably, the `-Donly-c` flag no longer has compilation errors due to this error set. --- src/link.zig | 89 +++++++++++++++++++++++++++++++++-- src/link/Coff.zig | 4 +- src/link/Elf.zig | 4 +- src/link/MachO.zig | 4 +- src/link/MachO/dead_strip.zig | 24 +++++----- src/link/MachO/zld.zig | 2 +- src/link/NvPtx.zig | 4 +- src/link/Plan9.zig | 4 +- src/link/SpirV.zig | 4 +- src/link/Wasm.zig | 10 ++-- 10 files changed, 116 insertions(+), 33 deletions(-) diff --git a/src/link.zig b/src/link.zig index b0c8294482..9d4ac0d55b 100644 --- a/src/link.zig +++ b/src/link.zig @@ -677,9 +677,92 @@ pub const File = struct { } } + /// TODO audit this error set. most of these should be collapsed into one error, + /// and ErrorFlags should be updated to convey the meaning to the user. + pub const FlushError = error{ + CacheUnavailable, + CurrentWorkingDirectoryUnlinked, + DivisionByZero, + DllImportLibraryNotFound, + EmptyStubFile, + ExpectedFuncType, + FailedToEmit, + FailedToResolveRelocationTarget, + FileSystem, + FilesOpenedWithWrongFlags, + FlushFailure, + FrameworkNotFound, + FunctionSignatureMismatch, + GlobalTypeMismatch, + InvalidCharacter, + InvalidEntryKind, + InvalidFormat, + InvalidIndex, + InvalidMagicByte, + InvalidWasmVersion, + LLDCrashed, + LLDReportedFailure, + LLD_LinkingIsTODO_ForSpirV, + LibCInstallationMissingCRTDir, + LibCInstallationNotAvailable, + LibraryNotFound, + LinkingWithoutZigSourceUnimplemented, + MalformedArchive, + MalformedDwarf, + MalformedSection, + MemoryTooBig, + MemoryTooSmall, + MismatchedCpuArchitecture, + MissAlignment, + MissingEndForBody, + MissingEndForExpression, + /// TODO: this should be removed from the error set in favor of using ErrorFlags + MissingMainEntrypoint, + MissingSymbol, + MissingTableSymbols, + ModuleNameMismatch, + MultipleSymbolDefinitions, + NoObjectsToLink, + NotObject, + NotObjectFile, + NotSupported, + OutOfMemory, + Overflow, + PermissionDenied, + StreamTooLong, + SwapFile, + SymbolCollision, + SymbolMismatchingType, + TODOImplementPlan9Objs, + TODOImplementWritingLibFiles, + TODOImplementWritingStaticLibFiles, + UnableToSpawnSelf, + UnableToSpawnWasm, + UnableToWriteArchive, + UndefinedLocal, + /// TODO: merge with UndefinedSymbolReference + UndefinedSymbol, + /// TODO: merge with UndefinedSymbol + UndefinedSymbolReference, + Underflow, + UnexpectedRemainder, + UnexpectedTable, + UnexpectedValue, + UnhandledDwFormValue, + UnhandledSymbolType, + UnknownFeature, + Unseekable, + UnsupportedCpuArchitecture, + UnsupportedVersion, + } || + fs.File.WriteFileError || + fs.File.OpenError || + std.ChildProcess.SpawnError || + fs.Dir.CopyFileError; + /// Commit pending changes and write headers. Takes into account final output mode /// and `use_lld`, not only `effectiveOutputMode`. - pub fn flush(base: *File, comp: *Compilation, prog_node: *std.Progress.Node) !void { + pub fn flush(base: *File, comp: *Compilation, prog_node: *std.Progress.Node) FlushError!void { if (build_options.only_c) { assert(base.tag == .c); return @fieldParentPtr(C, "base", base).flush(comp, prog_node); @@ -717,7 +800,7 @@ pub const File = struct { /// Commit pending changes and write headers. Works based on `effectiveOutputMode` /// rather than final output mode. - pub fn flushModule(base: *File, comp: *Compilation, prog_node: *std.Progress.Node) !void { + pub fn flushModule(base: *File, comp: *Compilation, prog_node: *std.Progress.Node) FlushError!void { if (build_options.only_c) { assert(base.tag == .c); return @fieldParentPtr(C, "base", base).flushModule(comp, prog_node); @@ -880,7 +963,7 @@ pub const File = struct { } } - pub fn linkAsArchive(base: *File, comp: *Compilation, prog_node: *std.Progress.Node) !void { + pub fn linkAsArchive(base: *File, comp: *Compilation, prog_node: *std.Progress.Node) FlushError!void { const tracy = trace(@src()); defer tracy.end(); diff --git a/src/link/Coff.zig b/src/link/Coff.zig index 80182104a9..8f29f4bd56 100644 --- a/src/link/Coff.zig +++ b/src/link/Coff.zig @@ -1442,7 +1442,7 @@ fn resolveGlobalSymbol(self: *Coff, current: SymbolWithLoc) !void { gop.value_ptr.* = current; } -pub fn flush(self: *Coff, comp: *Compilation, prog_node: *std.Progress.Node) !void { +pub fn flush(self: *Coff, comp: *Compilation, prog_node: *std.Progress.Node) link.File.FlushError!void { if (self.base.options.emit == null) { if (build_options.have_llvm) { if (self.llvm_object) |llvm_object| { @@ -1461,7 +1461,7 @@ pub fn flush(self: *Coff, comp: *Compilation, prog_node: *std.Progress.Node) !vo } } -pub fn flushModule(self: *Coff, comp: *Compilation, prog_node: *std.Progress.Node) !void { +pub fn flushModule(self: *Coff, comp: *Compilation, prog_node: *std.Progress.Node) link.File.FlushError!void { const tracy = trace(@src()); defer tracy.end(); diff --git a/src/link/Elf.zig b/src/link/Elf.zig index 1a722c1dde..a7f49ab6d4 100644 --- a/src/link/Elf.zig +++ b/src/link/Elf.zig @@ -932,7 +932,7 @@ pub fn populateMissingMetadata(self: *Elf) !void { } } -pub fn flush(self: *Elf, comp: *Compilation, prog_node: *std.Progress.Node) !void { +pub fn flush(self: *Elf, comp: *Compilation, prog_node: *std.Progress.Node) link.File.FlushError!void { if (self.base.options.emit == null) { if (build_options.have_llvm) { if (self.llvm_object) |llvm_object| { @@ -951,7 +951,7 @@ pub fn flush(self: *Elf, comp: *Compilation, prog_node: *std.Progress.Node) !voi } } -pub fn flushModule(self: *Elf, comp: *Compilation, prog_node: *std.Progress.Node) !void { +pub fn flushModule(self: *Elf, comp: *Compilation, prog_node: *std.Progress.Node) link.File.FlushError!void { const tracy = trace(@src()); defer tracy.end(); diff --git a/src/link/MachO.zig b/src/link/MachO.zig index ca8ca6f2bd..bec66b66f0 100644 --- a/src/link/MachO.zig +++ b/src/link/MachO.zig @@ -405,7 +405,7 @@ pub fn createEmpty(gpa: Allocator, options: link.Options) !*MachO { return self; } -pub fn flush(self: *MachO, comp: *Compilation, prog_node: *std.Progress.Node) !void { +pub fn flush(self: *MachO, comp: *Compilation, prog_node: *std.Progress.Node) link.File.FlushError!void { if (self.base.options.emit == null) { if (build_options.have_llvm) { if (self.llvm_object) |llvm_object| { @@ -430,7 +430,7 @@ pub fn flush(self: *MachO, comp: *Compilation, prog_node: *std.Progress.Node) !v } } -pub fn flushModule(self: *MachO, comp: *Compilation, prog_node: *std.Progress.Node) !void { +pub fn flushModule(self: *MachO, comp: *Compilation, prog_node: *std.Progress.Node) link.File.FlushError!void { const tracy = trace(@src()); defer tracy.end(); diff --git a/src/link/MachO/dead_strip.zig b/src/link/MachO/dead_strip.zig index 6090162ce8..c3af7a19d8 100644 --- a/src/link/MachO/dead_strip.zig +++ b/src/link/MachO/dead_strip.zig @@ -17,7 +17,7 @@ const N_DEAD = @import("zld.zig").N_DEAD; const AtomTable = std.AutoHashMap(AtomIndex, void); -pub fn gcAtoms(zld: *Zld, reverse_lookups: [][]u32) !void { +pub fn gcAtoms(zld: *Zld, reverse_lookups: [][]u32) Allocator.Error!void { const gpa = zld.gpa; var arena = std.heap.ArenaAllocator.init(gpa); @@ -30,8 +30,8 @@ pub fn gcAtoms(zld: *Zld, reverse_lookups: [][]u32) !void { try alive.ensureTotalCapacity(@intCast(u32, zld.atoms.items.len)); try collectRoots(zld, &roots); - try mark(zld, roots, &alive, reverse_lookups); - try prune(zld, alive); + mark(zld, roots, &alive, reverse_lookups); + prune(zld, alive); } fn collectRoots(zld: *Zld, roots: *AtomTable) !void { @@ -133,7 +133,7 @@ fn markLive( atom_index: AtomIndex, alive: *AtomTable, reverse_lookups: [][]u32, -) anyerror!void { +) void { if (alive.contains(atom_index)) return; const atom = zld.getAtom(atom_index); @@ -171,7 +171,7 @@ fn markLive( const other_atom = zld.getAtom(other_atom_index); const other_sym = zld.getSymbol(other_atom.getSymbolWithLoc()); if (other_sym.n_sect == sect_id) { - try markLive(zld, other_atom_index, alive, reverse_lookups); + markLive(zld, other_atom_index, alive, reverse_lookups); } } continue; @@ -194,11 +194,11 @@ fn markLive( zld.getAtom(target_atom_index).file, }); - try markLive(zld, target_atom_index, alive, reverse_lookups); + markLive(zld, target_atom_index, alive, reverse_lookups); } } -fn refersLive(zld: *Zld, atom_index: AtomIndex, alive: AtomTable, reverse_lookups: [][]u32) !bool { +fn refersLive(zld: *Zld, atom_index: AtomIndex, alive: AtomTable, reverse_lookups: [][]u32) bool { const atom = zld.getAtom(atom_index); const sym_loc = atom.getSymbolWithLoc(); @@ -240,10 +240,10 @@ fn refersLive(zld: *Zld, atom_index: AtomIndex, alive: AtomTable, reverse_lookup return false; } -fn mark(zld: *Zld, roots: AtomTable, alive: *AtomTable, reverse_lookups: [][]u32) !void { +fn mark(zld: *Zld, roots: AtomTable, alive: *AtomTable, reverse_lookups: [][]u32) void { var it = roots.keyIterator(); while (it.next()) |root| { - try markLive(zld, root.*, alive, reverse_lookups); + markLive(zld, root.*, alive, reverse_lookups); } var loop: bool = true; @@ -265,8 +265,8 @@ fn mark(zld: *Zld, roots: AtomTable, alive: *AtomTable, reverse_lookups: [][]u32 const source_sect = object.getSourceSection(sect_id); if (source_sect.isDontDeadStripIfReferencesLive()) { - if (try refersLive(zld, atom_index, alive.*, reverse_lookups)) { - try markLive(zld, atom_index, alive, reverse_lookups); + if (refersLive(zld, atom_index, alive.*, reverse_lookups)) { + markLive(zld, atom_index, alive, reverse_lookups); loop = true; } } @@ -275,7 +275,7 @@ fn mark(zld: *Zld, roots: AtomTable, alive: *AtomTable, reverse_lookups: [][]u32 } } -fn prune(zld: *Zld, alive: AtomTable) !void { +fn prune(zld: *Zld, alive: AtomTable) void { log.debug("pruning dead atoms", .{}); for (zld.objects.items) |*object| { var i: usize = 0; diff --git a/src/link/MachO/zld.zig b/src/link/MachO/zld.zig index 8d599e0185..20917a80c5 100644 --- a/src/link/MachO/zld.zig +++ b/src/link/MachO/zld.zig @@ -3735,7 +3735,7 @@ const SymbolResolver = struct { unresolved: std.AutoArrayHashMap(u32, void), }; -pub fn linkWithZld(macho_file: *MachO, comp: *Compilation, prog_node: *std.Progress.Node) !void { +pub fn linkWithZld(macho_file: *MachO, comp: *Compilation, prog_node: *std.Progress.Node) link.File.FlushError!void { const tracy = trace(@src()); defer tracy.end(); diff --git a/src/link/NvPtx.zig b/src/link/NvPtx.zig index 4873511d55..c010e28263 100644 --- a/src/link/NvPtx.zig +++ b/src/link/NvPtx.zig @@ -93,11 +93,11 @@ pub fn freeDecl(self: *NvPtx, decl_index: Module.Decl.Index) void { return self.llvm_object.freeDecl(decl_index); } -pub fn flush(self: *NvPtx, comp: *Compilation, prog_node: *std.Progress.Node) !void { +pub fn flush(self: *NvPtx, comp: *Compilation, prog_node: *std.Progress.Node) link.File.FlushError!void { return self.flushModule(comp, prog_node); } -pub fn flushModule(self: *NvPtx, comp: *Compilation, prog_node: *std.Progress.Node) !void { +pub fn flushModule(self: *NvPtx, comp: *Compilation, prog_node: *std.Progress.Node) link.File.FlushError!void { if (!build_options.have_llvm) return; if (build_options.skip_non_native) { @panic("Attempted to compile for architecture that was disabled by build configuration"); diff --git a/src/link/Plan9.zig b/src/link/Plan9.zig index 1da33bdf86..0af4fe81d5 100644 --- a/src/link/Plan9.zig +++ b/src/link/Plan9.zig @@ -356,7 +356,7 @@ fn updateFinish(self: *Plan9, decl: *Module.Decl) !void { } } -pub fn flush(self: *Plan9, comp: *Compilation, prog_node: *std.Progress.Node) !void { +pub fn flush(self: *Plan9, comp: *Compilation, prog_node: *std.Progress.Node) link.File.FlushError!void { assert(!self.base.options.use_lld); switch (self.base.options.effectiveOutputMode()) { @@ -392,7 +392,7 @@ fn declCount(self: *Plan9) usize { return self.data_decl_table.count() + fn_decl_count; } -pub fn flushModule(self: *Plan9, comp: *Compilation, prog_node: *std.Progress.Node) !void { +pub fn flushModule(self: *Plan9, comp: *Compilation, prog_node: *std.Progress.Node) link.File.FlushError!void { if (build_options.skip_non_native and builtin.object_format != .plan9) { @panic("Attempted to compile for object format that was disabled by build configuration"); } diff --git a/src/link/SpirV.zig b/src/link/SpirV.zig index b2f6edddfb..695df33930 100644 --- a/src/link/SpirV.zig +++ b/src/link/SpirV.zig @@ -176,7 +176,7 @@ pub fn freeDecl(self: *SpirV, decl_index: Module.Decl.Index) void { self.decl_table.swapRemoveAt(index); } -pub fn flush(self: *SpirV, comp: *Compilation, prog_node: *std.Progress.Node) !void { +pub fn flush(self: *SpirV, comp: *Compilation, prog_node: *std.Progress.Node) link.File.FlushError!void { if (build_options.have_llvm and self.base.options.use_lld) { return error.LLD_LinkingIsTODO_ForSpirV; // TODO: LLD Doesn't support SpirV at all. } else { @@ -184,7 +184,7 @@ pub fn flush(self: *SpirV, comp: *Compilation, prog_node: *std.Progress.Node) !v } } -pub fn flushModule(self: *SpirV, comp: *Compilation, prog_node: *std.Progress.Node) !void { +pub fn flushModule(self: *SpirV, comp: *Compilation, prog_node: *std.Progress.Node) link.File.FlushError!void { if (build_options.skip_non_native) { @panic("Attempted to compile for architecture that was disabled by build configuration"); } diff --git a/src/link/Wasm.zig b/src/link/Wasm.zig index 2327837b31..4c3de84e01 100644 --- a/src/link/Wasm.zig +++ b/src/link/Wasm.zig @@ -501,7 +501,7 @@ fn resolveSymbolsInObject(wasm: *Wasm, object_index: u16) !void { if (symbol.isUndefined()) { log.err("Local symbols are not allowed to reference imports", .{}); log.err(" symbol '{s}' defined in '{s}'", .{ sym_name, object.name }); - return error.undefinedLocal; + return error.UndefinedLocal; } try wasm.resolved_symbols.putNoClobber(wasm.base.allocator, location, {}); continue; @@ -2091,7 +2091,7 @@ fn resetState(wasm: *Wasm) void { wasm.debug_pubtypes_index = null; } -pub fn flush(wasm: *Wasm, comp: *Compilation, prog_node: *std.Progress.Node) !void { +pub fn flush(wasm: *Wasm, comp: *Compilation, prog_node: *std.Progress.Node) link.File.FlushError!void { if (wasm.base.options.emit == null) { if (build_options.have_llvm) { if (wasm.llvm_object) |llvm_object| { @@ -2107,7 +2107,7 @@ pub fn flush(wasm: *Wasm, comp: *Compilation, prog_node: *std.Progress.Node) !vo } } -pub fn flushModule(wasm: *Wasm, comp: *Compilation, prog_node: *std.Progress.Node) !void { +pub fn flushModule(wasm: *Wasm, comp: *Compilation, prog_node: *std.Progress.Node) link.File.FlushError!void { const tracy = trace(@src()); defer tracy.end(); @@ -3195,7 +3195,7 @@ fn linkWithLLD(wasm: *Wasm, comp: *Compilation, prog_node: *std.Progress.Node) ! const term = child.spawnAndWait() catch |err| { log.err("unable to spawn {s}: {s}", .{ argv.items[0], @errorName(err) }); - return error.UnableToSpawnwasm; + return error.UnableToSpawnWasm; }; switch (term) { .Exited => |code| { @@ -3216,7 +3216,7 @@ fn linkWithLLD(wasm: *Wasm, comp: *Compilation, prog_node: *std.Progress.Node) ! const term = child.wait() catch |err| { log.err("unable to spawn {s}: {s}", .{ argv.items[0], @errorName(err) }); - return error.UnableToSpawnwasm; + return error.UnableToSpawnWasm; }; switch (term) {