authorgravatar for kcbanner@gmail.comCasey Banner <kcbanner@gmail.com> 2022-11-19 08:57:08-05:00
committergravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2023-01-09 14:29:59-07:00
log90fedf24fcec4bd7a9e71be100e0ae73c7245b33
tree19b69f8409a471b8070e5711c729f5463e5822d8
parenta5bcdfd27e6589392ff70ce6e4ae65433a2984ec

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, 99 insertions(+), 16 deletions(-)

src/Compilation.zig+90-4
......@@ -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 }
......@@ -2068,7 +2084,7 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation {
20682084 // we force mark them for resolution here.
20692085
20702086 var tls_index_sym = switch (comp.getTarget().cpu.arch) {
2071 .x86 => "__tls_index",
2087 .i386 => "__tls_index",
20722088 else => "_tls_index",
20732089 };
20742090
......@@ -2162,6 +2178,11 @@ pub fn destroy(self: *Compilation) void {
21622178 }
21632179 self.failed_c_objects.deinit(gpa);
21642180
2181 for (self.lld_errors.items) |*lld_error| {
2182 lld_error.deinit(gpa);
2183 }
2184 self.lld_errors.deinit(gpa);
2185
21652186 self.clearMiscFailures();
21662187
21672188 self.cache_parent.manifest_dir.close();
......@@ -2461,6 +2482,10 @@ pub fn update(comp: *Compilation) !void {
24612482 try comp.flush(main_progress_node);
24622483 }
24632484
2485 if (comp.totalErrorCount() != 0) {
2486 return;
2487 }
2488
24642489 // Failure here only means an unnecessary cache miss.
24652490 man.writeManifest() catch |err| {
24662491 log.warn("failed to write cache manifest: {s}", .{@errorName(err)});
......@@ -2490,7 +2515,7 @@ fn flush(comp: *Compilation, prog_node: *std.Progress.Node) !void {
24902515 // This is needed before reading the error flags.
24912516 comp.bin_file.flush(comp, prog_node) catch |err| switch (err) {
24922517 error.FlushFailure => {}, // error reported through link_error_flags
2493 error.LLDReportedFailure => {}, // error reported through log.err
2518 error.LLDReportedFailure => {}, // error reported via lockAndParseLldStderr
24942519 else => |e| return e,
24952520 };
24962521 comp.link_error_flags = comp.bin_file.errorFlags();
......@@ -2723,7 +2748,7 @@ pub fn makeBinFileWritable(self: *Compilation) !void {
27232748/// This function is temporally single-threaded.
27242749pub fn totalErrorCount(self: *Compilation) usize {
27252750 var total: usize = self.failed_c_objects.count() + self.misc_failures.count() +
2726 @boolToInt(self.alloc_failure_occurred);
2751 @boolToInt(self.alloc_failure_occurred) + self.lld_errors.items.len;
27272752
27282753 if (self.bin_file.options.module) |module| {
27292754 total += module.failed_exports.count();
......@@ -2811,6 +2836,21 @@ pub fn getAllErrorsAlloc(self: *Compilation) !AllErrors {
28112836 });
28122837 }
28132838 }
2839 for (self.lld_errors.items) |lld_error| {
2840 const notes = try arena_allocator.alloc(AllErrors.Message, lld_error.context_lines.len);
2841 for (lld_error.context_lines) |context_line, i| {
2842 notes[i] = .{ .plain = .{
2843 .msg = try arena_allocator.dupe(u8, context_line),
2844 } };
2845 }
2846
2847 try errors.append(.{
2848 .plain = .{
2849 .msg = try arena_allocator.dupe(u8, lld_error.msg),
2850 .notes = notes,
2851 },
2852 });
2853 }
28142854 for (self.misc_failures.values()) |*value| {
28152855 try AllErrors.addPlainWithChildren(&arena, &errors, value.msg, value.children);
28162856 }
......@@ -4985,6 +5025,52 @@ pub fn lockAndSetMiscFailure(
49855025 return setMiscFailure(comp, tag, format, args);
49865026}
49875027
5028fn parseLldStderr(comp: *Compilation, comptime prefix: []const u8, stderr: []const u8) Allocator.Error!void {
5029 var context_lines = std.ArrayList([]const u8).init(comp.gpa);
5030 defer context_lines.deinit();
5031
5032 var current_err: ?*LldError = null;
5033 var lines = mem.split(u8, stderr, std.cstr.line_sep);
5034 while (lines.next()) |line| {
5035 if (mem.startsWith(u8, line, prefix ++ ":")) {
5036 if (current_err) |err| {
5037 err.context_lines = context_lines.toOwnedSlice();
5038 }
5039
5040 var split = std.mem.split(u8, line, "error: ");
5041 _ = split.first();
5042
5043 const duped_msg = try std.fmt.allocPrint(comp.gpa, "{s}: {s}", .{ prefix, split.rest() });
5044 errdefer comp.gpa.free(duped_msg);
5045
5046 current_err = try comp.lld_errors.addOne(comp.gpa);
5047 current_err.?.* = .{ .msg = duped_msg };
5048 } else if (current_err != null) {
5049 const context_prefix = ">>> ";
5050 var trimmed = mem.trimRight(u8, line, &std.ascii.whitespace);
5051 if (mem.startsWith(u8, trimmed, context_prefix)) {
5052 trimmed = trimmed[context_prefix.len..];
5053 }
5054
5055 if (trimmed.len > 0) {
5056 const duped_line = try comp.gpa.dupe(u8, trimmed);
5057 try context_lines.append(duped_line);
5058 }
5059 }
5060 }
5061
5062 if (current_err) |err| {
5063 err.context_lines = context_lines.toOwnedSlice();
5064 }
5065}
5066
5067pub fn lockAndParseLldStderr(comp: *Compilation, comptime prefix: []const u8, stderr: []const u8) void {
5068 comp.mutex.lock();
5069 defer comp.mutex.unlock();
5070
5071 comp.parseLldStderr(prefix, stderr) catch comp.setAllocFailure();
5072}
5073
49885074pub fn dump_argv(argv: []const []const u8) void {
49895075 for (argv[0 .. argv.len - 1]) |arg| {
49905076 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 },