Skip to content

Commit 12e6ebd

Browse files
ganehagclaude
andcommitted
fix: 5 more instances of the stale-write clone bug in string/regexp splitting
Continuing the sweep from bbd1ddb (grepped for the "publish empty, grow visible" idiom and its variants across every file that calls vmAllocManagedSlice, not just the ones with an explanatory comment already attached). Found the identical shape in: - string.zig: nativeStrSplit (both the empty-separator/rune and separator-based branches), nativeStrFields, and the split-with-limit function — each grows a published array_managed length by one element per split/field while writing through a captured local, in a loop whose body (substring/makeDynString) can allocate. - regexp.zig: nativeReFindAll and the split function that follows it — same shape, one array_managed element written per regex match. All five: pre-fill the destination with .null and publish the full length immediately instead of growing it visible incrementally, then read/write through the owning Object's own field per iteration rather than the captured local — same fix as core.clone (6e4b81c), host_abi's wire decoding (10837a5), and template's path splitting (bbd1ddb). Also checked and ruled out as non-instances: core.zig's nativeGcStats/ nativeGcStatsExt (superficially similar "publish empty then fill" shape, but every value written is a scalar .int or a ctx.cs.internStr string — internStr allocates from the chunk's own permanent bump region, never the collectible managed heap, so nothing in either loop can trigger compactManagedHeap at all). Verified: zig build compiler-test (native Debug, -Dgc_stress=true, and -Dheap_paranoia=true) all 993/993. Manually verified via the rebuilt CLI: std.string.split (both separator forms), std.string.fields, regexp find_all, and regexp split all produce correct output. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C3pMYWS2xy8zitUkpJaiZU
1 parent bbd1ddb commit 12e6ebd

2 files changed

Lines changed: 65 additions & 34 deletions

File tree

src/lang/native/regexp.zig

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -560,13 +560,22 @@ pub fn nativeReFindAll(ctx: VMContext, pattern_val: Value, s_val: Value) !Value
560560
const obj = try vmgc.allocTempRooted(ctx, .{ .array_managed = &[_]Value{} });
561561
defer ctx.vs.popTempRoot();
562562
const result = try vmgc.vmAllocManagedSlice(ctx, Value, matches.len);
563-
obj.* = .{ .array_managed = result[0..0] };
563+
// GC-audit 2026-09 (project_gc_rooting_hardening): pre-fill and
564+
// publish the full length immediately, then write through
565+
// obj.array_managed (re-derived each time) rather than the captured
566+
// `result` local — makeDynString can allocate and, rarely, trigger
567+
// compactManagedHeap, which would relocate obj's backing (a write
568+
// through the stale `result` afterward lands in freed/reused memory)
569+
// and, separately, size any mid-loop relocation from the length
570+
// visible at that point, losing the reserved-but-unwritten tail.
571+
for (result) |*slot| slot.* = .null;
572+
obj.* = .{ .array_managed = result[0..matches.len] };
564573
for (matches, 0..) |m, j| {
565574
// Re-derive s_val's bytes on every iteration: makeDynString's own
566575
// allocation can compact and relocate s_val's backing between calls.
567576
const s_now = try vms.asStringValue(s_val);
568-
result[j] = try vmgc.makeDynString(ctx, s_now[m[0]..m[1]]);
569-
obj.* = .{ .array_managed = result[0 .. j + 1] };
577+
const piece = try vmgc.makeDynString(ctx, s_now[m[0]..m[1]]);
578+
obj.array_managed[j] = piece;
570579
}
571580
return .{ .object = obj };
572581
}
@@ -629,13 +638,18 @@ pub fn nativeReSplit(ctx: VMContext, pattern_val: Value, s_val: Value) !Value {
629638
const obj = try vmgc.allocTempRooted(ctx, .{ .array_managed = &[_]Value{} });
630639
defer ctx.vs.popTempRoot();
631640
const result = try vmgc.vmAllocManagedSlice(ctx, Value, parts.items.len);
632-
obj.* = .{ .array_managed = result[0..0] };
641+
// GC-audit 2026-09 (project_gc_rooting_hardening): pre-fill and
642+
// publish the full length immediately, then write through
643+
// obj.array_managed (re-derived each time) rather than the captured
644+
// `result` local — see nativeReFindAll above for why.
645+
for (result) |*slot| slot.* = .null;
646+
obj.* = .{ .array_managed = result[0..parts.items.len] };
633647
for (parts.items, 0..) |part, j| {
634648
// Re-derive s_val's bytes on every iteration: makeDynString's own
635649
// allocation can compact and relocate s_val's backing between calls.
636650
const s_now = try vms.asStringValue(s_val);
637-
result[j] = try vmgc.makeDynString(ctx, s_now[part[0]..part[1]]);
638-
obj.* = .{ .array_managed = result[0 .. j + 1] };
651+
const piece = try vmgc.makeDynString(ctx, s_now[part[0]..part[1]]);
652+
obj.array_managed[j] = piece;
639653
}
640654
return .{ .object = obj };
641655
}

src/lang/native/string.zig

Lines changed: 45 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,21 @@ pub fn nativeStrSplit(ctx: VMContext, s_val: Value, sep_val: Value, managed: boo
3636
defer ctx.vs.popTempRoot();
3737
if (count > 0) {
3838
const pieces = try vmgc.vmAllocManagedSlice(ctx, Value, count);
39-
// Attach the slice as it fills: substring() can trigger GC, and
40-
// already-filled elements must be traced or they get reclaimed.
41-
arr_obj.* = .{ .array_managed = pieces[0..0] };
39+
// GC-audit 2026-09 (project_gc_rooting_hardening): pre-fill and
40+
// publish the full length immediately rather than growing it
41+
// visible by one element per iteration while writing through the
42+
// captured `pieces` local — substring()/makeDynString() can
43+
// allocate and, rarely, trigger compactManagedHeap, which would
44+
// relocate arr_obj's backing (a write through the stale `pieces`
45+
// afterward lands in whatever now occupies that freed memory)
46+
// and, separately, size any mid-loop relocation from the length
47+
// visible at that point, permanently losing the
48+
// reserved-but-unwritten tail of this allocation. Every slot is a
49+
// valid, traceable .null from the start, so no incremental
50+
// visibility is needed; write through arr_obj.array_managed
51+
// (re-derived each time) instead of `pieces`.
52+
for (pieces) |*slot| slot.* = .null;
53+
arr_obj.* = .{ .array_managed = pieces[0..count] };
4254
if (sep.len == 0) {
4355
var i: usize = 0;
4456
var pi: usize = 0;
@@ -48,26 +60,26 @@ pub fn nativeStrSplit(ctx: VMContext, s_val: Value, sep_val: Value, managed: boo
4860
// compact, relocating s_val's backing.
4961
const s = try vms.asStringValue(s_val);
5062
const w = try vmstr.utf8NextRuneByteLen(s, i);
51-
pieces[pi] = try substring(ctx, s[i .. i + w], managed);
63+
const piece = try substring(ctx, s[i .. i + w], managed);
64+
arr_obj.array_managed[pi] = piece;
5265
i += w;
5366
pi += 1;
54-
arr_obj.* = .{ .array_managed = pieces[0..pi] };
5567
}
5668
} else {
5769
var i: usize = 0;
5870
var pi: usize = 0;
5971
while (true) {
6072
const s = try vms.asStringValue(s_val);
6173
const pos = std.mem.indexOfPos(u8, s, i, sep) orelse break;
62-
pieces[pi] = try substring(ctx, s[i..pos], managed);
74+
const piece = try substring(ctx, s[i..pos], managed);
75+
arr_obj.array_managed[pi] = piece;
6376
pi += 1;
64-
arr_obj.* = .{ .array_managed = pieces[0..pi] };
6577
i = pos + sep.len;
6678
}
6779
const s = try vms.asStringValue(s_val);
68-
pieces[pi] = try substring(ctx, s[i..], managed);
80+
const piece = try substring(ctx, s[i..], managed);
81+
arr_obj.array_managed[pi] = piece;
6982
}
70-
arr_obj.* = .{ .array_managed = pieces[0..count] };
7183
}
7284
return .{ .object = arr_obj };
7385
}
@@ -270,9 +282,12 @@ pub fn nativeStrFields(ctx: VMContext, s_val: Value) !Value {
270282
defer ctx.vs.popTempRoot();
271283
if (count > 0) {
272284
const pieces = try vmgc.vmAllocManagedSlice(ctx, Value, count);
273-
// Attach as it fills: makeDynString can trigger GC and earlier
274-
// elements must be traced.
275-
arr_obj.* = .{ .array_managed = pieces[0..0] };
285+
// GC-audit 2026-09 (project_gc_rooting_hardening): pre-fill and
286+
// publish the full length immediately, then write through
287+
// arr_obj.array_managed (re-derived each time) rather than the
288+
// captured `pieces` local — see nativeStrSplit above for why.
289+
for (pieces) |*slot| slot.* = .null;
290+
arr_obj.* = .{ .array_managed = pieces[0..count] };
276291
var pi: usize = 0;
277292
i = 0;
278293
while (i < s0.len) {
@@ -285,11 +300,9 @@ pub fn nativeStrFields(ctx: VMContext, s_val: Value) !Value {
285300
const start = i;
286301
while (i < s.len and !isFieldSep(s[i])) i += 1;
287302
const piece = try vmgc.makeDynString(ctx, s[start..i]);
288-
pieces[pi] = piece;
303+
arr_obj.array_managed[pi] = piece;
289304
pi += 1;
290-
arr_obj.* = .{ .array_managed = pieces[0..pi] };
291305
}
292-
arr_obj.* = .{ .array_managed = pieces[0..count] };
293306
}
294307
return .{ .object = arr_obj };
295308
}
@@ -426,9 +439,12 @@ pub fn nativeStrSplitN(ctx: VMContext, s_val: Value, sep_val: Value, n_v: Value)
426439
const arr_obj = try vmgc.allocTempRooted(ctx, .{ .array = &[_]Value{} });
427440
defer ctx.vs.popTempRoot();
428441
const pieces = try vmgc.vmAllocManagedSlice(ctx, Value, count);
429-
// Attach as it fills: makeDynString can trigger GC and earlier elements
430-
// must be traced.
431-
arr_obj.* = .{ .array_managed = pieces[0..0] };
442+
// GC-audit 2026-09 (project_gc_rooting_hardening): pre-fill and
443+
// publish the full length immediately, then write through
444+
// arr_obj.array_managed (re-derived each time) rather than the
445+
// captured `pieces` local — see nativeStrSplit above for why.
446+
for (pieces) |*slot| slot.* = .null;
447+
arr_obj.* = .{ .array_managed = pieces[0..count] };
432448
if (sep0.len == 0) {
433449
// Split at UTF-8 codepoint boundaries (not byte boundaries).
434450
// Track byte_pos across iterations; re-derive s each time since
@@ -437,17 +453,18 @@ pub fn nativeStrSplitN(ctx: VMContext, s_val: Value, sep_val: Value, n_v: Value)
437453
var pi: usize = 0;
438454
while (pi < count) {
439455
const s = try vms.asStringValue(s_val);
440-
if (pi + 1 == count) {
456+
const piece = if (pi + 1 == count)
441457
// Last piece: remainder of the string (either the last
442458
// single rune when no limit hit, or the remaining suffix
443459
// when the max cap truncated the split).
444-
pieces[pi] = try vmgc.makeDynString(ctx, s[byte_pos..]);
445-
} else {
460+
try vmgc.makeDynString(ctx, s[byte_pos..])
461+
else blk: {
446462
const w = try vmstr.utf8NextRuneByteLen(s, byte_pos);
447-
pieces[pi] = try vmgc.makeDynString(ctx, s[byte_pos .. byte_pos + w]);
463+
const p = try vmgc.makeDynString(ctx, s[byte_pos .. byte_pos + w]);
448464
byte_pos += w;
449-
}
450-
arr_obj.* = .{ .array_managed = pieces[0 .. pi + 1] };
465+
break :blk p;
466+
};
467+
arr_obj.array_managed[pi] = piece;
451468
pi += 1;
452469
}
453470
} else {
@@ -457,15 +474,15 @@ pub fn nativeStrSplitN(ctx: VMContext, s_val: Value, sep_val: Value, n_v: Value)
457474
const s = try vms.asStringValue(s_val);
458475
const sep = try vms.asStringValue(sep_val);
459476
const idx = std.mem.indexOf(u8, s[pos..], sep).?;
460-
pieces[pi] = try vmgc.makeDynString(ctx, s[pos .. pos + idx]);
477+
const piece = try vmgc.makeDynString(ctx, s[pos .. pos + idx]);
478+
arr_obj.array_managed[pi] = piece;
461479
pos += idx + sep.len;
462480
pi += 1;
463-
arr_obj.* = .{ .array_managed = pieces[0..pi] };
464481
}
465482
const s = try vms.asStringValue(s_val);
466-
pieces[pi] = try vmgc.makeDynString(ctx, s[pos..]);
483+
const piece = try vmgc.makeDynString(ctx, s[pos..]);
484+
arr_obj.array_managed[pi] = piece;
467485
}
468-
arr_obj.* = .{ .array_managed = pieces[0..count] };
469486
return .{ .object = arr_obj };
470487
}
471488

0 commit comments

Comments
 (0)