authorgravatar for kcbanner@gmail.comCasey Banner <kcbanner@gmail.com> 2022-11-19 08:57:08-05:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2022-11-19 15:57:08+02:00
logf746e118795af8f8eb16deefc8f6fed26f3be4dd
tree3749fb9bae897a3b5538c6d795bd4e67e8a8220d
parent0697883d01cb1bbdc5c34175b06eb47d5158011a
signaturebadge-question-mark Signed by PGP key 4AEE18F83AFDEB23

linker: fail the compilation if there were linker errors

There was no check for linker errors after flushing, which meant that if the link failed the build would continue and try to copy the non-existant exe, and also write the manifest as if it had succeeded. Also adds parsing of lld output, which is surfaced at the end of the compilation with the other errors instead of via stderr

4 files changed, 98 insertions(+), 15 deletions(-)

src/Compilation.zig+89-3
......@@ -51,6 +51,7 @@ whole_cache_manifest: ?*Cache.Manifest = null,
5151whole_cache_manifest_mutex: std.Thread.Mutex = .{},
5252
5353link_error_flags: link.File.ErrorFlags = .{},
54lld_errors: std.ArrayListUnmanaged(LldError) = .{},
5455
5556work_queue: std.fifo.LinearFifo(Job, .Dynamic),
5657anon_work_queue: std.fifo.LinearFifo(Job, .Dynamic),
......@@ -335,6 +336,21 @@ pub const MiscError = struct {
335336 }
336337};
337338
339pub const LldError = struct {
340 /// Allocated with gpa.
341 msg: []const u8,
342 context_lines: []const []const u8 = &.{},
343
344 pub fn deinit(self: *LldError, gpa: Allocator) void {
345 for (self.context_lines) |line| {
346 gpa.free(line);
347 }
348
349 gpa.free(self.context_lines);
350 gpa.free(self.msg);
351 }
352};
353
338354/// To support incremental compilation, errors are stored in various places
339355/// so that they can be created and destroyed appropriately. This structure
340356/// is used to collect all the errors from the various places into one
......@@ -498,7 +514,7 @@ pub const AllErrors = struct {
498514 }
499515 ttyconf.setColor(stderr, .Reset);
500516 for (plain.notes) |note| {
501 try note.renderToWriter(ttyconf, stderr, "error", .Red, indent + 4);
517 try note.renderToWriter(ttyconf, stderr, "note", .Cyan, indent + 4);
502518 }
503519 },
504520 }
......@@ -2166,6 +2182,11 @@ pub fn destroy(self: *Compilation) void {
21662182 }
21672183 self.failed_c_objects.deinit(gpa);
21682184
2185 for (self.lld_errors.items) |*lld_error| {
2186 lld_error.deinit(gpa);
2187 }
2188 self.lld_errors.deinit(gpa);
2189
21692190 self.clearMiscFailures();
21702191
21712192 self.cache_parent.manifest_dir.close();
......@@ -2465,6 +2486,10 @@ pub fn update(comp: *Compilation) !void {
24652486 try comp.flush(main_progress_node);
24662487 }
24672488
2489 if (comp.totalErrorCount() != 0) {
2490 return;
2491 }
2492
24682493 // Failure here only means an unnecessary cache miss.
24692494 man.writeManifest() catch |err| {
24702495 log.warn("failed to write cache manifest: {s}", .{@errorName(err)});
......@@ -2494,7 +2519,7 @@ fn flush(comp: *Compilation, prog_node: *std.Progress.Node) !void {
24942519 // This is needed before reading the error flags.
24952520 comp.bin_file.flush(comp, prog_node) catch |err| switch (err) {
24962521 error.FlushFailure => {}, // error reported through link_error_flags
2497 error.LLDReportedFailure => {}, // error reported through log.err
2522 error.LLDReportedFailure => {}, // error reported via lockAndParseLldStderr
24982523 else => |e| return e,
24992524 };
25002525 comp.link_error_flags = comp.bin_file.errorFlags();
......@@ -2727,7 +2752,7 @@ pub fn makeBinFileWritable(self: *Compilation) !void {
27272752/// This function is temporally single-threaded.
27282753pub fn totalErrorCount(self: *Compilation) usize {
27292754 var total: usize = self.failed_c_objects.count() + self.misc_failures.count() +
2730 @boolToInt(self.alloc_failure_occurred);
2755 @boolToInt(self.alloc_failure_occurred) + self.lld_errors.items.len;
27312756
27322757 if (self.bin_file.options.module) |module| {
27332758 total += module.failed_exports.count();
......@@ -2815,6 +2840,21 @@ pub fn getAllErrorsAlloc(self: *Compilation) !AllErrors {
28152840 });
28162841 }
28172842 }
2843 for (self.lld_errors.items) |lld_error| {
2844 const notes = try arena_allocator.alloc(AllErrors.Message, lld_error.context_lines.len);
2845 for (lld_error.context_lines) |context_line, i| {
2846 notes[i] = .{ .plain = .{
2847 .msg = try arena_allocator.dupe(u8, context_line),
2848 } };
2849 }
2850
2851 try errors.append(.{
2852 .plain = .{
2853 .msg = try arena_allocator.dupe(u8, lld_error.msg),
2854 .notes = notes,
2855 },
2856 });
2857 }
28182858 for (self.misc_failures.values()) |*value| {
28192859 try AllErrors.addPlainWithChildren(&arena, &errors, value.msg, value.children);
28202860 }
......@@ -4989,6 +5029,52 @@ pub fn lockAndSetMiscFailure(
49895029 return setMiscFailure(comp, tag, format, args);
49905030}
49915031
5032fn parseLldStderr(comp: *Compilation, comptime prefix: []const u8, stderr: []const u8) Allocator.Error!void {
5033 var context_lines = std.ArrayList([]const u8).init(comp.gpa);
5034 defer context_lines.deinit();
5035
5036 var current_err: ?*LldError = null;
5037 var lines = mem.split(u8, stderr, std.cstr.line_sep);
5038 while (lines.next()) |line| {
5039 if (mem.startsWith(u8, line, prefix ++ ":")) {
5040 if (current_err) |err| {
5041 err.context_lines = context_lines.toOwnedSlice();
5042 }
5043
5044 var split = std.mem.split(u8, line, "error: ");
5045 _ = split.first();
5046
5047 const duped_msg = try std.fmt.allocPrint(comp.gpa, "{s}: {s}", .{ prefix, split.rest() });
5048 errdefer comp.gpa.free(duped_msg);
5049
5050 current_err = try comp.lld_errors.addOne(comp.gpa);
5051 current_err.?.* = .{ .msg = duped_msg };
5052 } else if (current_err != null) {
5053 const context_prefix = ">>> ";
5054 var trimmed = mem.trimRight(u8, line, &std.ascii.whitespace);
5055 if (mem.startsWith(u8, trimmed, context_prefix)) {
5056 trimmed = trimmed[context_prefix.len..];
5057 }
5058
5059 if (trimmed.len > 0) {
5060 const duped_line = try comp.gpa.dupe(u8, trimmed);
5061 try context_lines.append(duped_line);
5062 }
5063 }
5064 }
5065
5066 if (current_err) |err| {
5067 err.context_lines = context_lines.toOwnedSlice();
5068 }
5069}
5070
5071pub fn lockAndParseLldStderr(comp: *Compilation, comptime prefix: []const u8, stderr: []const u8) void {
5072 comp.mutex.lock();
5073 defer comp.mutex.unlock();
5074
5075 comp.parseLldStderr(prefix, stderr) catch comp.setAllocFailure();
5076}
5077
49925078pub fn dump_argv(argv: []const []const u8) void {
49935079 for (argv[0 .. argv.len - 1]) |arg| {
49945080 std.debug.print("{s} ", .{arg});
src/link/Coff/lld.zig+3-4
......@@ -175,7 +175,8 @@ pub fn linkWithLLD(self: *Coff, comp: *Compilation, prog_node: *std.Progress.Nod
175175 // We will invoke ourselves as a child process to gain access to LLD.
176176 // This is necessary because LLD does not behave properly as a library -
177177 // it calls exit() and does not reset all global data between invocations.
178 try argv.appendSlice(&[_][]const u8{ comp.self_exe_path.?, "lld-link" });
178 const linker_command = "lld-link";
179 try argv.appendSlice(&[_][]const u8{ comp.self_exe_path.?, linker_command });
179180
180181 try argv.append("-ERRORLIMIT:0");
181182 try argv.append("-NOLOGO");
......@@ -556,9 +557,7 @@ pub fn linkWithLLD(self: *Coff, comp: *Compilation, prog_node: *std.Progress.Nod
556557 switch (term) {
557558 .Exited => |code| {
558559 if (code != 0) {
559 // TODO parse this output and surface with the Compilation API rather than
560 // directly outputting to stderr here.
561 std.debug.print("{s}", .{stderr});
560 comp.lockAndParseLldStderr(linker_command, stderr);
562561 return error.LLDReportedFailure;
563562 }
564563 },
src/link/Elf.zig+3-4
......@@ -1422,7 +1422,8 @@ fn linkWithLLD(self: *Elf, comp: *Compilation, prog_node: *std.Progress.Node) !v
14221422 // We will invoke ourselves as a child process to gain access to LLD.
14231423 // This is necessary because LLD does not behave properly as a library -
14241424 // it calls exit() and does not reset all global data between invocations.
1425 try argv.appendSlice(&[_][]const u8{ comp.self_exe_path.?, "ld.lld" });
1425 const linker_command = "ld.lld";
1426 try argv.appendSlice(&[_][]const u8{ comp.self_exe_path.?, linker_command });
14261427 if (is_obj) {
14271428 try argv.append("-r");
14281429 }
......@@ -1841,9 +1842,7 @@ fn linkWithLLD(self: *Elf, comp: *Compilation, prog_node: *std.Progress.Node) !v
18411842 switch (term) {
18421843 .Exited => |code| {
18431844 if (code != 0) {
1844 // TODO parse this output and surface with the Compilation API rather than
1845 // directly outputting to stderr here.
1846 std.debug.print("{s}", .{stderr});
1845 comp.lockAndParseLldStderr(linker_command, stderr);
18471846 return error.LLDReportedFailure;
18481847 }
18491848 },
src/link/Wasm.zig+3-4
......@@ -3125,7 +3125,8 @@ fn linkWithLLD(wasm: *Wasm, comp: *Compilation, prog_node: *std.Progress.Node) !
31253125 // We will invoke ourselves as a child process to gain access to LLD.
31263126 // This is necessary because LLD does not behave properly as a library -
31273127 // it calls exit() and does not reset all global data between invocations.
3128 try argv.appendSlice(&[_][]const u8{ comp.self_exe_path.?, "wasm-ld" });
3128 const linker_command = "wasm-ld";
3129 try argv.appendSlice(&[_][]const u8{ comp.self_exe_path.?, linker_command });
31293130 try argv.append("--error-limit=0");
31303131
31313132 if (wasm.base.options.lto) {
......@@ -3357,9 +3358,7 @@ fn linkWithLLD(wasm: *Wasm, comp: *Compilation, prog_node: *std.Progress.Node) !
33573358 switch (term) {
33583359 .Exited => |code| {
33593360 if (code != 0) {
3360 // TODO parse this output and surface with the Compilation API rather than
3361 // directly outputting to stderr here.
3362 std.debug.print("{s}", .{stderr});
3361 comp.lockAndParseLldStderr(linker_command, stderr);
33633362 return error.LLDReportedFailure;
33643363 }
33653364 },