From d7bf0dab8243d23e95c49ad9b27f54d21df116f0 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Wed, 24 Dec 2025 16:20:39 -0800 Subject: [PATCH] link.Wasm: fix incremental update crash Description of problem: - wasm linker does GC in flush() - it has the mechanism where it tracks the end index of a bunch of ArrayHashMap before flush() and after flush, shrinkRetainingCapacity() them to restore them to pre-flush() state - this includes `functions`, which contains `__divti3` - flush() notices the call to `__divti3` and calls markFunctionImport(), but that function does nothing on a second update because `alive` is already set to `true` so it incorrectly skips adding the intrinsic back to `functions` I tried to remember why I thought it was OK to use this `alive` flag which is state that's not being restored after flush(). If I remember correctly, I was just leaving the code how it was before, with the plan to change the data layout after encountering this exact problem. However, I found a solution that doesn't require changing data layout, and still takes advantage of the 1-bit-per-symbol data layout. --- src/link/Wasm.zig | 28 ++++++++++++++++++++-------- src/link/Wasm/Flush.zig | 11 +++++++---- 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/src/link/Wasm.zig b/src/link/Wasm.zig index e9c76ff40a0d8c809197de186c3e8faea577932b..af800d77d263b0a99a43833494ad90884a2dd260 100644 --- a/src/link/Wasm.zig +++ b/src/link/Wasm.zig @@ -429,7 +429,11 @@ pub const OutputFunctionIndex = enum(u32) { pub fn fromSymbolName(wasm: *const Wasm, name: String) OutputFunctionIndex { if (wasm.flush_buffer.function_imports.getIndex(name)) |i| return @enumFromInt(i); - return fromFunctionIndex(wasm, FunctionIndex.fromSymbolName(wasm, name).?); + return fromFunctionIndex(wasm, FunctionIndex.fromSymbolName(wasm, name) orelse { + if (std.debug.runtime_safety) { + std.debug.panic("function index for symbol not found: {s}", .{name.slice(wasm)}); + } else unreachable; + }); } }; @@ -3534,7 +3538,10 @@ pub fn markFunctionImport( import: *FunctionImport, func_index: FunctionImport.Index, ) link.File.FlushError!void { - if (import.flags.alive) return; + // import.flags.alive might be already true from a previous update. In such + // case, we must still run the logic in this function, in case the item + // being marked was reverted by the `flush` logic that resets the hash + // table watermarks. import.flags.alive = true; const comp = wasm.base.comp; @@ -3554,8 +3561,9 @@ pub fn markFunctionImport( } else { try wasm.function_imports.put(gpa, name, .fromObject(func_index, wasm)); } - } else { - try markFunction(wasm, import.resolution.unpack(wasm).object_function, import.flags.exported); + } else switch (import.resolution.unpack(wasm)) { + .object_function => try markFunction(wasm, import.resolution.unpack(wasm).object_function, import.flags.exported), + else => return, } } @@ -3594,7 +3602,10 @@ fn markGlobalImport( import: *GlobalImport, global_index: GlobalImport.Index, ) link.File.FlushError!void { - if (import.flags.alive) return; + // import.flags.alive might be already true from a previous update. In such + // case, we must still run the logic in this function, in case the item + // being marked was reverted by the `flush` logic that resets the hash + // table watermarks. import.flags.alive = true; const comp = wasm.base.comp; @@ -3624,8 +3635,9 @@ fn markGlobalImport( } else { try wasm.global_imports.put(gpa, name, .fromObject(global_index, wasm)); } - } else { - try markGlobal(wasm, import.resolution.unpack(wasm).object_global, import.flags.exported); + } else switch (import.resolution.unpack(wasm)) { + .object_global => try markGlobal(wasm, import.resolution.unpack(wasm).object_global, import.flags.exported), + else => return, } } @@ -4043,7 +4055,7 @@ pub fn tagNameSymbolIndex(wasm: *Wasm, ip_index: InternPool.Index) Allocator.Err const comp = wasm.base.comp; assert(comp.config.output_mode == .Obj); const gpa = comp.gpa; - const name = try wasm.internStringFmt("__zig_tag_name_{d}", .{@intFromEnum(ip_index)}); + const name = try wasm.internStringFmt("__zig_tag_name_{d}", .{ip_index}); const gop = try wasm.symbol_table.getOrPut(gpa, name); gop.value_ptr.* = {}; return @enumFromInt(gop.index); diff --git a/src/link/Wasm/Flush.zig b/src/link/Wasm/Flush.zig index b189e2e60079160b55f2e741618b6078726e7581..5bd18a1936fc25288c3c8467e6582e0c9c6a24bf 100644 --- a/src/link/Wasm/Flush.zig +++ b/src/link/Wasm/Flush.zig @@ -128,17 +128,20 @@ pub fn finish(f: *Flush, wasm: *Wasm) !void { if (comp.zcu) |zcu| { const ip: *const InternPool = &zcu.intern_pool; // No mutations allowed! + log.debug("total MIR instructions: {d}", .{wasm.mir_instructions.len}); + // Detect any intrinsics that were called; they need to have dependencies on the symbols marked. // Likewise detect `@tagName` calls so those functions can be included in the output and synthesized. for (wasm.mir_instructions.items(.tag), wasm.mir_instructions.items(.data)) |tag, *data| switch (tag) { .call_intrinsic => { const symbol_name = try wasm.internString(@tagName(data.intrinsic)); const i: Wasm.FunctionImport.Index = @enumFromInt(wasm.object_function_imports.getIndex(symbol_name) orelse { - return diags.fail("missing compiler runtime intrinsic '{s}' (undefined linker symbol)", .{ - @tagName(data.intrinsic), + return diags.fail("missing compiler runtime intrinsic '{t}' (undefined linker symbol)", .{ + data.intrinsic, }); }); try wasm.markFunctionImport(symbol_name, i.value(wasm), i); + log.debug("markFunctionImport intrinsic {d}={t}", .{ i, data.intrinsic }); }, .call_tag_name => { assert(ip.indexToKey(data.ip_index) == .enum_type); @@ -147,11 +150,10 @@ pub fn finish(f: *Flush, wasm: *Wasm) !void { wasm.tag_name_table_ref_count += 1; const int_tag_ty = Zcu.Type.fromInterned(data.ip_index).intTagType(zcu); gop.value_ptr.* = .{ .tag_name = .{ - .symbol_name = try wasm.internStringFmt("__zig_tag_name_{d}", .{@intFromEnum(data.ip_index)}), + .symbol_name = try wasm.internStringFmt("__zig_tag_name_{d}", .{data.ip_index}), .type_index = try wasm.internFunctionType(.auto, &.{int_tag_ty.ip_index}, .slice_const_u8_sentinel_0, target), .table_index = @intCast(wasm.tag_name_offs.items.len), } }; - try wasm.functions.put(gpa, .fromZcuFunc(wasm, @enumFromInt(gop.index)), {}); const tag_names = ip.loadEnumType(data.ip_index).names; for (tag_names.get(ip)) |tag_name| { const slice = tag_name.toSlice(ip); @@ -159,6 +161,7 @@ pub fn finish(f: *Flush, wasm: *Wasm) !void { try wasm.tag_name_bytes.appendSlice(gpa, slice[0 .. slice.len + 1]); } } + try wasm.functions.put(gpa, .fromZcuFunc(wasm, @enumFromInt(gop.index)), {}); }, else => continue, }; -- 2.54.0