| author |  Andrew Kelley <andrew@ziglang.org> 2025-12-24 16:20:39-08:00 |
| committer | Andrew Kelley <andrew@ziglang.org> 2025-12-26 19:58:56-08:00 |
| log | d7bf0dab8243d23e95c49ad9b27f54d21df116f0 |
| tree | d75635fd5d0982da152aab3d4028a48036279714 |
| parent | 3ad9a6a6045aec32b590387eddbc8ba3d598bcfc |
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.2 files changed, 27 insertions(+), 12 deletions(-)
src/link/Wasm.zig+20-8
| ... | ... | @@ -429,7 +429,11 @@ pub const OutputFunctionIndex = enum(u32) { |
| 429 | 429 | |
| 430 | 430 | pub fn fromSymbolName(wasm: *const Wasm, name: String) OutputFunctionIndex { |
| 431 | 431 | if (wasm.flush_buffer.function_imports.getIndex(name)) |i| return @enumFromInt(i); |
| 432 | return fromFunctionIndex(wasm, FunctionIndex.fromSymbolName(wasm, name).?); | |
| 432 | return fromFunctionIndex(wasm, FunctionIndex.fromSymbolName(wasm, name) orelse { | |
| 433 | if (std.debug.runtime_safety) { | |
| 434 | std.debug.panic("function index for symbol not found: {s}", .{name.slice(wasm)}); | |
| 435 | } else unreachable; | |
| 436 | }); | |
| 433 | 437 | } |
| 434 | 438 | }; |
| 435 | 439 | |
| ... | ... | @@ -3534,7 +3538,10 @@ pub fn markFunctionImport( |
| 3534 | 3538 | import: *FunctionImport, |
| 3535 | 3539 | func_index: FunctionImport.Index, |
| 3536 | 3540 | ) link.File.FlushError!void { |
| 3537 | if (import.flags.alive) return; | |
| 3541 | // import.flags.alive might be already true from a previous update. In such | |
| 3542 | // case, we must still run the logic in this function, in case the item | |
| 3543 | // being marked was reverted by the `flush` logic that resets the hash | |
| 3544 | // table watermarks. | |
| 3538 | 3545 | import.flags.alive = true; |
| 3539 | 3546 | |
| 3540 | 3547 | const comp = wasm.base.comp; |
| ... | ... | @@ -3554,8 +3561,9 @@ pub fn markFunctionImport( |
| 3554 | 3561 | } else { |
| 3555 | 3562 | try wasm.function_imports.put(gpa, name, .fromObject(func_index, wasm)); |
| 3556 | 3563 | } |
| 3557 | } else { | |
| 3558 | try markFunction(wasm, import.resolution.unpack(wasm).object_function, import.flags.exported); | |
| 3564 | } else switch (import.resolution.unpack(wasm)) { | |
| 3565 | .object_function => try markFunction(wasm, import.resolution.unpack(wasm).object_function, import.flags.exported), | |
| 3566 | else => return, | |
| 3559 | 3567 | } |
| 3560 | 3568 | } |
| 3561 | 3569 | |
| ... | ... | @@ -3594,7 +3602,10 @@ fn markGlobalImport( |
| 3594 | 3602 | import: *GlobalImport, |
| 3595 | 3603 | global_index: GlobalImport.Index, |
| 3596 | 3604 | ) link.File.FlushError!void { |
| 3597 | if (import.flags.alive) return; | |
| 3605 | // import.flags.alive might be already true from a previous update. In such | |
| 3606 | // case, we must still run the logic in this function, in case the item | |
| 3607 | // being marked was reverted by the `flush` logic that resets the hash | |
| 3608 | // table watermarks. | |
| 3598 | 3609 | import.flags.alive = true; |
| 3599 | 3610 | |
| 3600 | 3611 | const comp = wasm.base.comp; |
| ... | ... | @@ -3624,8 +3635,9 @@ fn markGlobalImport( |
| 3624 | 3635 | } else { |
| 3625 | 3636 | try wasm.global_imports.put(gpa, name, .fromObject(global_index, wasm)); |
| 3626 | 3637 | } |
| 3627 | } else { | |
| 3628 | try markGlobal(wasm, import.resolution.unpack(wasm).object_global, import.flags.exported); | |
| 3638 | } else switch (import.resolution.unpack(wasm)) { | |
| 3639 | .object_global => try markGlobal(wasm, import.resolution.unpack(wasm).object_global, import.flags.exported), | |
| 3640 | else => return, | |
| 3629 | 3641 | } |
| 3630 | 3642 | } |
| 3631 | 3643 | |
| ... | ... | @@ -4043,7 +4055,7 @@ pub fn tagNameSymbolIndex(wasm: *Wasm, ip_index: InternPool.Index) Allocator.Err |
| 4043 | 4055 | const comp = wasm.base.comp; |
| 4044 | 4056 | assert(comp.config.output_mode == .Obj); |
| 4045 | 4057 | const gpa = comp.gpa; |
| 4046 | const name = try wasm.internStringFmt("__zig_tag_name_{d}", .{@intFromEnum(ip_index)}); | |
| 4058 | const name = try wasm.internStringFmt("__zig_tag_name_{d}", .{ip_index}); | |
| 4047 | 4059 | const gop = try wasm.symbol_table.getOrPut(gpa, name); |
| 4048 | 4060 | gop.value_ptr.* = {}; |
| 4049 | 4061 | return @enumFromInt(gop.index); |
src/link/Wasm/Flush.zig+7-4
| ... | ... | @@ -128,17 +128,20 @@ pub fn finish(f: *Flush, wasm: *Wasm) !void { |
| 128 | 128 | if (comp.zcu) |zcu| { |
| 129 | 129 | const ip: *const InternPool = &zcu.intern_pool; // No mutations allowed! |
| 130 | 130 | |
| 131 | log.debug("total MIR instructions: {d}", .{wasm.mir_instructions.len}); | |
| 132 | ||
| 131 | 133 | // Detect any intrinsics that were called; they need to have dependencies on the symbols marked. |
| 132 | 134 | // Likewise detect `@tagName` calls so those functions can be included in the output and synthesized. |
| 133 | 135 | for (wasm.mir_instructions.items(.tag), wasm.mir_instructions.items(.data)) |tag, *data| switch (tag) { |
| 134 | 136 | .call_intrinsic => { |
| 135 | 137 | const symbol_name = try wasm.internString(@tagName(data.intrinsic)); |
| 136 | 138 | const i: Wasm.FunctionImport.Index = @enumFromInt(wasm.object_function_imports.getIndex(symbol_name) orelse { |
| 137 | return diags.fail("missing compiler runtime intrinsic '{s}' (undefined linker symbol)", .{ | |
| 138 | @tagName(data.intrinsic), | |
| 139 | return diags.fail("missing compiler runtime intrinsic '{t}' (undefined linker symbol)", .{ | |
| 140 | data.intrinsic, | |
| 139 | 141 | }); |
| 140 | 142 | }); |
| 141 | 143 | try wasm.markFunctionImport(symbol_name, i.value(wasm), i); |
| 144 | log.debug("markFunctionImport intrinsic {d}={t}", .{ i, data.intrinsic }); | |
| 142 | 145 | }, |
| 143 | 146 | .call_tag_name => { |
| 144 | 147 | assert(ip.indexToKey(data.ip_index) == .enum_type); |
| ... | ... | @@ -147,11 +150,10 @@ pub fn finish(f: *Flush, wasm: *Wasm) !void { |
| 147 | 150 | wasm.tag_name_table_ref_count += 1; |
| 148 | 151 | const int_tag_ty = Zcu.Type.fromInterned(data.ip_index).intTagType(zcu); |
| 149 | 152 | gop.value_ptr.* = .{ .tag_name = .{ |
| 150 | .symbol_name = try wasm.internStringFmt("__zig_tag_name_{d}", .{@intFromEnum(data.ip_index)}), | |
| 153 | .symbol_name = try wasm.internStringFmt("__zig_tag_name_{d}", .{data.ip_index}), | |
| 151 | 154 | .type_index = try wasm.internFunctionType(.auto, &.{int_tag_ty.ip_index}, .slice_const_u8_sentinel_0, target), |
| 152 | 155 | .table_index = @intCast(wasm.tag_name_offs.items.len), |
| 153 | 156 | } }; |
| 154 | try wasm.functions.put(gpa, .fromZcuFunc(wasm, @enumFromInt(gop.index)), {}); | |
| 155 | 157 | const tag_names = ip.loadEnumType(data.ip_index).names; |
| 156 | 158 | for (tag_names.get(ip)) |tag_name| { |
| 157 | 159 | const slice = tag_name.toSlice(ip); |
| ... | ... | @@ -159,6 +161,7 @@ pub fn finish(f: *Flush, wasm: *Wasm) !void { |
| 159 | 161 | try wasm.tag_name_bytes.appendSlice(gpa, slice[0 .. slice.len + 1]); |
| 160 | 162 | } |
| 161 | 163 | } |
| 164 | try wasm.functions.put(gpa, .fromZcuFunc(wasm, @enumFromInt(gop.index)), {}); | |
| 162 | 165 | }, |
| 163 | 166 | else => continue, |
| 164 | 167 | }; |