Skip to content

Commit 42dac40

Browse files
committed
InternPool: fix segfault in rehashTrackedInsts
The `.empty` map in a shard is weird: it claims to have capacity 1, but you're not actually allowed to actually use that capacity. That's fine for the normal insertion algorithm, because it always resizes to a higher capacity when inserting the initial element. However, `rehashTrackedInsts` was not aware of this caveat, so sometimes tried to store to the single element of the `empty` map. This system exists to avoid an extra branch in the main resizing logic (since `new_cap = old_cap * 2` only works if the capacity is never non-zero). However, it's fine for `rehashTrackedInsts` to have an extra branch to handle this case, since it's literally called once per update.
1 parent 497592c commit 42dac40

File tree

1 file changed

+9
-2
lines changed

1 file changed

+9
-2
lines changed

src/InternPool.zig

+9-2
Original file line numberDiff line numberDiff line change
@@ -319,11 +319,18 @@ pub fn rehashTrackedInsts(
319319
// We know how big each shard must be, so ensure we have the capacity we need.
320320
for (ip.shards) |*shard| {
321321
const want_capacity = if (shard.mutate.tracked_inst_map.len == 0) 0 else cap: {
322-
break :cap std.math.ceilPowerOfTwo(u32, shard.mutate.tracked_inst_map.len * 5 / 3) catch unreachable;
322+
// We need to return a capacity of at least 2 to make sure we don't have the `Map(...).empty` value.
323+
// For this reason, note the `+ 1` in the below expression. This matches the behavior of `trackZir`.
324+
break :cap std.math.ceilPowerOfTwo(u32, shard.mutate.tracked_inst_map.len * 5 / 3 + 1) catch unreachable;
323325
};
324326
const have_capacity = shard.shared.tracked_inst_map.header().capacity; // no acquire because we hold the mutex
325327
if (have_capacity >= want_capacity) {
326-
@memset(shard.shared.tracked_inst_map.entries[0..have_capacity], .{ .value = .none, .hash = undefined });
328+
if (have_capacity == 1) {
329+
// The map is `.empty` -- we can't memset the entries, or we'll segfault, because
330+
// the buffer is secretly constant.
331+
} else {
332+
@memset(shard.shared.tracked_inst_map.entries[0..have_capacity], .{ .value = .none, .hash = undefined });
333+
}
327334
continue;
328335
}
329336
var arena = arena_state.promote(gpa);

0 commit comments

Comments
 (0)