| author |  Matthew Lugg <mlugg@mlugg.co.uk> 2026-07-19 12:41:03+01:00 |
| committer | Matthew Lugg <mlugg@mlugg.co.uk> 2026-07-21 10:46:42+02:00 |
| log | 7faf6be3535195b96b69fb9b8d12f16015d7ec36 |
| tree | 721629bf3782496333ae15c701432c0136c97d2f |
| parent | 37909d3170769ee361f4e62cb9558efe12630781 |
link.Lld: handle errors properly in ZigLLVMWriteArchive
So... it turns out https://codeberg.org/ziglang/zig/issues/31520 is
probably our fault! This commit does not close that issue, but it at
least makes some progress.
LLVM's `Error` type is basically `anyerror!void`, and when an error
occurs, it can detect that the caller didn't look at the error code, in
which case it will abort with an error message. Our implementation of
`ZigLLVMWriteArchive` was checking *that* there was an error, but not
what that error *was*, so was triggering this code path. (I think that
LLVM's approach to error handling here is unnecessarily dangerous, but
whatcha gonna do :shrug:)
It is possible for the "no such file or directory" error to occur in
this function due to a TOCTOU bug. If we are producing a static library,
and some object file is deleted after the Zig frontend checks that it
exists but *before* `ZigLLVMWriteArchive` has a chance to read the file,
LLVM will report that the input file does not exist.
The reason I found this bug is that I was able to somewhat consistently
trigger the same condition by running a faulty build script which had
duplicated steps. Consider a case where build step A and B generate the
exact same object file, using the same options etc; and build step C
depends on A, and puts that object file into a static library. At some
point, step A and B both start running. Step A finishes first, placing
an object file into the cache directory. The build system then starts
the dependent step C. The compiler frontend confirms that the object
file (in the cache directory) exists, as expected, and passes it off to
`ZigLLVMWriteArchive`---but around this time, step B finishes, and
begins writing the *new* file. Depending on how LLVM writes that output
file, there may be a period of time where the file does not exist or is
truncated, either of which could cause `ZigLLVMWriteArchive` to fail.
The reproduction I described above does not explain #31520, because it
relies on a faulty build script (where the same object file is being
generated by multiple identical build steps), but that is not happening
in #31520. It also relies on building a static library, which again, is
not happening in #31520. Instead, if #31520 is indeed coming from this
function (which I believe is the most likely explanation), it must be
that #31520 is caused by a compiler bug which ultimately has the same
effect (an object file in the cache being overwritten while being used
to build a static library)
This makes sense, because in many compilations we *do* build several
static libraries: our vendored implementations of libraries, including
musl libc, libc++, libunwind, etc. However, this still doesn't fully
explain the bug. When we build these libraries, we create a
`Compilation` with a bunch of C source file inputs. For each of those,
the Zig compiler intentionally keeps hold of a shared advisory lock on
the output file (well, technically on its cache manifest), and does not
release it until the `Compilation` is destroyed, which only happens
after we've completely finished emitting our archive. The advisory lock
should be preventing any other Zig compiler process from trying to write
the object file until we've finished building the archive. I did briefly
audit the C object compilation logic for bugs, but barring a bug in
`std.Build.Cache` itself, I couldn't spot any problems.
Nonetheless, I still consider it likely that `ZigLLVMWriteArchive` is
where the errors in #31520 are coming from. This patch---which fixes the
unclean termination and actually reports LLVM's error properly---will
help to test that hypothesis, and if it's correct, the improved error
message will hopefully help to track down the underlying bug.4 files changed, 42 insertions(+), 9 deletions(-)
src- codegen
- llvm
- link
- zig_llvm.cpp
- zig_llvm.h
src/codegen/llvm/bindings.zig+2
| ... | ... | @@ -331,6 +331,8 @@ extern fn ZigLLVMWriteArchive( |
| 331 | 331 | file_names_ptr: [*]const [*:0]const u8, |
| 332 | 332 | file_names_len: usize, |
| 333 | 333 | archive_kind: ArchiveKind, |
| 334 | err_file_index_out: *usize, | |
| 335 | err_msg_out: *[*:0]u8, | |
| 334 | 336 | ) bool; |
| 335 | 337 | |
| 336 | 338 | pub const ParseCommandLineOptions = ZigLLVMParseCommandLineOptions; |
src/link/Lld.zig+18-5
| ... | ... | @@ -269,12 +269,12 @@ pub fn flush( |
| 269 | 269 | .wasm => wasmLink(lld, arena), |
| 270 | 270 | }; |
| 271 | 271 | result catch |err| switch (err) { |
| 272 | error.OutOfMemory, error.AlreadyReported => |e| return e, | |
| 272 | error.OutOfMemory, error.AlreadyReported, error.Canceled => |e| return e, | |
| 273 | 273 | else => |e| return lld.base.comp.link_diags.fail("failed to link with LLD: {t}", .{e}), |
| 274 | 274 | }; |
| 275 | 275 | } |
| 276 | 276 | |
| 277 | fn linkAsArchive(lld: *Lld, arena: Allocator) !void { | |
| 277 | fn linkAsArchive(lld: *Lld, arena: Allocator) link.Error!void { | |
| 278 | 278 | const base = &lld.base; |
| 279 | 279 | const comp = base.comp; |
| 280 | 280 | const directory = base.emit.root_dir; // Just an alias to make it shorter to type. |
| ... | ... | @@ -338,7 +338,9 @@ fn linkAsArchive(lld: *Lld, arena: Allocator) !void { |
| 338 | 338 | const llvm = @import("../codegen/llvm.zig"); |
| 339 | 339 | const target = &comp.root_mod.resolved_target.result; |
| 340 | 340 | llvm.initializeLLVMTarget(target.cpu.arch); |
| 341 | const bad = llvm_bindings.WriteArchive( | |
| 341 | var err_file_index: usize = undefined; | |
| 342 | var err_msg: [*:0]u8 = undefined; | |
| 343 | if (llvm_bindings.WriteArchive( | |
| 342 | 344 | full_out_path_z, |
| 343 | 345 | object_files.items.ptr, |
| 344 | 346 | object_files.items.len, |
| ... | ... | @@ -346,8 +348,19 @@ fn linkAsArchive(lld: *Lld, arena: Allocator) !void { |
| 346 | 348 | .windows => .COFF, |
| 347 | 349 | else => if (target.os.tag.isDarwin()) .DARWIN else .GNU, |
| 348 | 350 | }, |
| 349 | ); | |
| 350 | if (bad) return error.UnableToWriteArchive; | |
| 351 | &err_file_index, | |
| 352 | &err_msg, | |
| 353 | )) { | |
| 354 | defer std.c.free(err_msg); | |
| 355 | if (err_file_index < object_files.items.len) { | |
| 356 | return comp.link_diags.fail("LLD failed to open input file '{s}': {s}", .{ | |
| 357 | object_files.items[err_file_index], | |
| 358 | err_msg, | |
| 359 | }); | |
| 360 | } else { | |
| 361 | return comp.link_diags.fail("LLD failed to write archive: {s}", .{err_msg}); | |
| 362 | } | |
| 363 | } | |
| 351 | 364 | } |
| 352 | 365 | |
| 353 | 366 | fn addCommonArgs(argv: *std.array_list.Managed([]const u8), coff: bool) !void { |
src/zig_llvm.cpp+16-3
| ... | ... | @@ -473,19 +473,32 @@ void ZigLLVMParseCommandLineOptions(size_t argc, const char *const *argv) { |
| 473 | 473 | } |
| 474 | 474 | |
| 475 | 475 | bool ZigLLVMWriteArchive(const char *archive_name, const char **file_names, size_t file_name_count, |
| 476 | ZigLLVMArchiveKind archive_kind) | |
| 476 | ZigLLVMArchiveKind archive_kind, size_t *err_file_index_out, char **err_msg_out) | |
| 477 | 477 | { |
| 478 | 478 | SmallVector<NewArchiveMember, 4> new_members; |
| 479 | 479 | for (size_t i = 0; i < file_name_count; i += 1) { |
| 480 | 480 | Expected<NewArchiveMember> new_member = NewArchiveMember::getFile(file_names[i], true); |
| 481 | 481 | Error err = new_member.takeError(); |
| 482 | if (err) return true; | |
| 482 | if (err) { | |
| 483 | *err_file_index_out = i; | |
| 484 | const std::string msg = toString(std::move(err)); | |
| 485 | *err_msg_out = (char *)malloc(msg.length() + 1); | |
| 486 | strcpy(*err_msg_out, msg.c_str()); | |
| 487 | return true; | |
| 488 | } | |
| 483 | 489 | new_members.push_back(std::move(*new_member)); |
| 484 | 490 | } |
| 485 | 491 | Error err = writeArchive(archive_name, new_members, |
| 486 | 492 | SymtabWritingMode::NormalSymtab, static_cast<object::Archive::Kind>(archive_kind), true, false, nullptr); |
| 487 | 493 | |
| 488 | if (err) return true; | |
| 494 | if (err) { | |
| 495 | *err_file_index_out = file_name_count; | |
| 496 | const std::string msg = toString(std::move(err)); | |
| 497 | *err_msg_out = (char *)malloc(msg.length() + 1); | |
| 498 | strcpy(*err_msg_out, msg.c_str()); | |
| 499 | return true; | |
| 500 | } | |
| 501 | ||
| 489 | 502 | return false; |
| 490 | 503 | } |
| 491 | 504 |
src/zig_llvm.h+6-1
| ... | ... | @@ -121,7 +121,12 @@ ZIG_EXTERN_C bool ZigLLDLinkCOFF(int argc, const char **argv, bool can_exit_earl |
| 121 | 121 | ZIG_EXTERN_C bool ZigLLDLinkELF(int argc, const char **argv, bool can_exit_early, bool disable_output); |
| 122 | 122 | ZIG_EXTERN_C bool ZigLLDLinkWasm(int argc, const char **argv, bool can_exit_early, bool disable_output); |
| 123 | 123 | |
| 124 | // On error, populates `*err_file_index_out` and `*err_msg_out` and returns `true`. The caller is | |
| 125 | // responsible for freeing `*err_msg_out` using `free`. | |
| 126 | // | |
| 127 | // If an error occurs reading an input file, `*err_file_index_out` is set to the index of that input | |
| 128 | // file in `file_names`. Otherwise, `*err_file_index_out` is set to `file_name_count`. | |
| 124 | 129 | ZIG_EXTERN_C bool ZigLLVMWriteArchive(const char *archive_name, const char **file_names, size_t file_name_count, |
| 125 | ZigLLVMArchiveKind archive_kind); | |
| 130 | ZigLLVMArchiveKind archive_kind, size_t *err_file_index_out, char **err_msg_out); | |
| 126 | 131 | |
| 127 | 132 | #endif |