From 233a615be835968e9c536b048a24a9a1d0511dae Mon Sep 17 00:00:00 2001 From: VoltEngine Date: Mon, 30 Jun 2025 01:02:47 +0200 Subject: [PATCH 1/3] spirv: fix null file handle crash Add makeWritable() call before writeAll() in SPIR-V linker flush function. SPIR-V linker was trying to write to uninitialized file handle. - Add .spirv case to makeWritable() switch in link.zig - Call makeWritable() with proper error handling in SpirV.zig - OutOfMemory propagates up, other errors become LinkFailure Fixes #23883 --- src/link.zig | 11 +++++++++-- src/link/SpirV.zig | 5 +++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/link.zig b/src/link.zig index 2a6e3b8031b3..831134bc4dc4 100644 --- a/src/link.zig +++ b/src/link.zig @@ -608,8 +608,15 @@ pub const File = struct { .mode = determineMode(output_mode, link_mode), }); }, - .c, .spirv => dev.checkAny(&.{ .c_linker, .spirv_linker }), - } + .spirv => { + if (base.file != null) return; + dev.check(.spirv_linker); + const emit = base.emit; + base.file = try emit.root_dir.handle.createFile(emit.sub_path, .{ + .truncate = true, + }); + }, + .c => dev.check(.c_linker), } /// Some linkers create a separate file for debug info, which we might need to temporarily close diff --git a/src/link/SpirV.zig b/src/link/SpirV.zig index 7b908d56edf6..619f5c38afb6 100644 --- a/src/link/SpirV.zig +++ b/src/link/SpirV.zig @@ -240,6 +240,11 @@ pub fn flush( else => |other| return diags.fail("error while linking: {s}", .{@errorName(other)}), }; + self.base.makeWritable() catch |err| switch (err) { + error.OutOfMemory => return error.OutOfMemory, + else => return error.LinkFailure, + }; + self.base.file.?.writeAll(std.mem.sliceAsBytes(linked_module)) catch |err| return diags.fail("failed to write: {s}", .{@errorName(err)}); } From 1fd369cc94b9b8436ffde2861ff4fe96cf50616d Mon Sep 17 00:00:00 2001 From: VoltEngine Date: Tue, 1 Jul 2025 23:36:43 +0200 Subject: [PATCH 2/3] spirv: fix null file handle crash and improve error handling Add makeWritable() call before file operations to ensure file handle is initialized. Implement ELF/COFF delegation pattern with flushInner() for consistent error handling and architectural parity with other major linkers. Prevents crash when SPIR-V linker attempts to write to uninitialized file. --- src/link.zig | 13 +++-------- src/link/SpirV.zig | 55 ++++++++++++++++++++++++++++++++++------------ 2 files changed, 44 insertions(+), 24 deletions(-) diff --git a/src/link.zig b/src/link.zig index 831134bc4dc4..59c0a7a4c975 100644 --- a/src/link.zig +++ b/src/link.zig @@ -568,9 +568,9 @@ pub const File = struct { const gpa = comp.gpa; switch (base.tag) { .lld => assert(base.file == null), - .coff, .elf, .macho, .plan9, .wasm, .goff, .xcoff => { + .coff, .elf, .macho, .plan9, .wasm, .goff, .xcoff, .spirv => { if (base.file != null) return; - dev.checkAny(&.{ .coff_linker, .elf_linker, .macho_linker, .plan9_linker, .wasm_linker, .goff_linker, .xcoff_linker }); + dev.checkAny(&.{ .coff_linker, .elf_linker, .macho_linker, .plan9_linker, .wasm_linker, .goff_linker, .xcoff_linker, .spirv_linker }); const emit = base.emit; if (base.child_pid) |pid| { if (builtin.os.tag == .windows) { @@ -608,15 +608,8 @@ pub const File = struct { .mode = determineMode(output_mode, link_mode), }); }, - .spirv => { - if (base.file != null) return; - dev.check(.spirv_linker); - const emit = base.emit; - base.file = try emit.root_dir.handle.createFile(emit.sub_path, .{ - .truncate = true, - }); - }, .c => dev.check(.c_linker), + } } /// Some linkers create a separate file for debug info, which we might need to temporarily close diff --git a/src/link/SpirV.zig b/src/link/SpirV.zig index 619f5c38afb6..fc49aa577194 100644 --- a/src/link/SpirV.zig +++ b/src/link/SpirV.zig @@ -180,6 +180,27 @@ pub fn flush( tid: Zcu.PerThread.Id, prog_node: std.Progress.Node, ) link.File.FlushError!void { + const tracy = trace(@src()); + defer tracy.end(); + + const comp = self.base.comp; + const diags = &comp.link_diags; + + const sub_prog_node = prog_node.start("SPIR-V Flush", 0); + defer sub_prog_node.end(); + + return flushInner(self, arena, tid) catch |err| switch (err) { + error.OutOfMemory => return error.OutOfMemory, + error.LinkFailure => return error.LinkFailure, + else => |e| return diags.fail("SPIR-V flush failed: {s}", .{@errorName(e)}), + }; +} + +fn flushInner( + self: *SpirV, + arena: Allocator, + tid: Zcu.PerThread.Id, +) !void { // The goal is to never use this because it's only needed if we need to // write to InternPool, but flush is too late to be writing to the // InternPool. @@ -189,12 +210,6 @@ pub fn flush( @panic("Attempted to compile for architecture that was disabled by build configuration"); } - const tracy = trace(@src()); - defer tracy.end(); - - const sub_prog_node = prog_node.start("Flush Module", 0); - defer sub_prog_node.end(); - const comp = self.base.comp; const spv = &self.object.spv; const diags = &comp.link_diags; @@ -235,18 +250,14 @@ pub fn flush( const module = try spv.finalize(arena); errdefer arena.free(module); - const linked_module = self.linkModule(arena, module, sub_prog_node) catch |err| switch (err) { + const linked_module = self.linkModule(arena, module, std.Progress.Node.none) catch |err| switch (err) { error.OutOfMemory => return error.OutOfMemory, else => |other| return diags.fail("error while linking: {s}", .{@errorName(other)}), }; - self.base.makeWritable() catch |err| switch (err) { - error.OutOfMemory => return error.OutOfMemory, - else => return error.LinkFailure, - }; - - self.base.file.?.writeAll(std.mem.sliceAsBytes(linked_module)) catch |err| - return diags.fail("failed to write: {s}", .{@errorName(err)}); + try self.base.makeWritable(); + try self.pwriteAll(std.mem.sliceAsBytes(linked_module), 0); + try self.setEndPos(linked_module.len * @sizeOf(Word)); } fn linkModule(self: *SpirV, a: Allocator, module: []Word, progress: std.Progress.Node) ![]Word { @@ -266,3 +277,19 @@ fn linkModule(self: *SpirV, a: Allocator, module: []Word, progress: std.Progress return binary.finalize(a); } + +pub fn pwriteAll(spirv_file: *SpirV, bytes: []const u8, offset: u64) error{LinkFailure}!void { + const comp = spirv_file.base.comp; + const diags = &comp.link_diags; + spirv_file.base.file.?.pwriteAll(bytes, offset) catch |err| { + return diags.fail("failed to write: {s}", .{@errorName(err)}); + }; +} + +pub fn setEndPos(spirv_file: *SpirV, length: u64) error{LinkFailure}!void { + const comp = spirv_file.base.comp; + const diags = &comp.link_diags; + spirv_file.base.file.?.setEndPos(length) catch |err| { + return diags.fail("failed to set file end pos: {s}", .{@errorName(err)}); + }; +} From 3869aad5687bc8405ca9fa318c9e75559210cd45 Mon Sep 17 00:00:00 2001 From: VoltEngine Date: Tue, 1 Jul 2025 23:42:26 +0200 Subject: [PATCH 3/3] spirv: fix progress node regression in linkModule call --- src/link/SpirV.zig | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/link/SpirV.zig b/src/link/SpirV.zig index fc49aa577194..6ba5b94395bf 100644 --- a/src/link/SpirV.zig +++ b/src/link/SpirV.zig @@ -189,7 +189,7 @@ pub fn flush( const sub_prog_node = prog_node.start("SPIR-V Flush", 0); defer sub_prog_node.end(); - return flushInner(self, arena, tid) catch |err| switch (err) { + return flushInner(self, arena, tid, sub_prog_node) catch |err| switch (err) { error.OutOfMemory => return error.OutOfMemory, error.LinkFailure => return error.LinkFailure, else => |e| return diags.fail("SPIR-V flush failed: {s}", .{@errorName(e)}), @@ -200,6 +200,7 @@ fn flushInner( self: *SpirV, arena: Allocator, tid: Zcu.PerThread.Id, + prog_node: std.Progress.Node, ) !void { // The goal is to never use this because it's only needed if we need to // write to InternPool, but flush is too late to be writing to the @@ -250,7 +251,7 @@ fn flushInner( const module = try spv.finalize(arena); errdefer arena.free(module); - const linked_module = self.linkModule(arena, module, std.Progress.Node.none) catch |err| switch (err) { + const linked_module = self.linkModule(arena, module, prog_node) catch |err| switch (err) { error.OutOfMemory => return error.OutOfMemory, else => |other| return diags.fail("error while linking: {s}", .{@errorName(other)}), };