authorgravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2025-02-05 14:25:29-08:00
committergravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2025-02-06 14:23:23-08:00
log86261915497fe6b9e0802b2d1e23b5f49c1d2757
tree52b08560ef36070751c6cf9bc0cf1062c9d30da1
parent7320e8b3cd7c8f482fd99793f2ed29cf610319d7

std.heap.GeneralPurposeAllocator: fix slot_counts calculation

In larger small buckets, the comptime logic that computed slot count did not verify that the number it produced was valid. Now it verifies it, which made this bug into a compile error. Then I fixed the bug by introducing a "minimum slots per bucket" declaration.

1 files changed, 12 insertions(+), 7 deletions(-)

lib/std/heap/general_purpose_allocator.zig+12-7
......@@ -104,7 +104,7 @@ const StackTrace = std.builtin.StackTrace;
104104const page_size: usize = @max(std.heap.page_size_max, switch (builtin.os.tag) {
105105 .windows => 64 * 1024, // Makes `std.heap.PageAllocator` take the happy path.
106106 .wasi => 64 * 1024, // Max alignment supported by `std.heap.WasmAllocator`.
107 else => 512 * 1024, // Avoids too many active mappings when `page_size_max` is low.
107 else => 128 * 1024, // Avoids too many active mappings when `page_size_max` is low.
108108});
109109const page_align: mem.Alignment = .fromByteUnits(page_size);
110110
......@@ -112,8 +112,7 @@ const page_align: mem.Alignment = .fromByteUnits(page_size);
112112const SlotIndex = std.meta.Int(.unsigned, math.log2(page_size) + 1);
113113const Log2USize = std.math.Log2Int(usize);
114114
115const default_test_stack_trace_frames: usize = if (builtin.is_test) 10 else 6;
116const default_sys_stack_trace_frames: usize = if (std.debug.sys_can_stack_trace) default_test_stack_trace_frames else 0;
115const default_sys_stack_trace_frames: usize = if (std.debug.sys_can_stack_trace) 6 else 0;
117116const default_stack_trace_frames: usize = switch (builtin.mode) {
118117 .Debug => default_sys_stack_trace_frames,
119118 else => 0,
......@@ -219,7 +218,10 @@ pub fn GeneralPurposeAllocator(comptime config: Config) type {
219218
220219 pub const Error = mem.Allocator.Error;
221220
222 const small_bucket_count = math.log2(page_size);
221 /// Avoids creating buckets that would only be able to store a small
222 /// number of slots. Value of 1 means 2 is the minimum slot count.
223 const minimum_slots_per_bucket_log2 = 1;
224 const small_bucket_count = math.log2(page_size) - minimum_slots_per_bucket_log2;
223225 const largest_bucket_object_size = 1 << (small_bucket_count - 1);
224226 const LargestSizeClassInt = std.math.IntFittingRange(0, largest_bucket_object_size);
225227
......@@ -385,21 +387,24 @@ pub fn GeneralPurposeAllocator(comptime config: Config) type {
385387 /// This is executed only at compile-time to prepopulate a lookup table.
386388 fn calculateSlotCount(size_class_index: usize) SlotIndex {
387389 const size_class = @as(usize, 1) << @as(Log2USize, @intCast(size_class_index));
388 var lower: usize = 8;
390 var lower: usize = 1 << minimum_slots_per_bucket_log2;
389391 var upper: usize = (page_size - bucketSize(lower)) / size_class;
390392 while (upper > lower) {
391393 const proposed: usize = lower + (upper - lower) / 2;
392394 if (proposed == lower) return lower;
393395 const slots_end = proposed * size_class;
394396 const header_begin = mem.alignForward(usize, slots_end, @alignOf(BucketHeader));
395 const bucket_size = bucketSize(proposed);
396 const end = header_begin + bucket_size;
397 const end = header_begin + bucketSize(proposed);
397398 if (end > page_size) {
398399 upper = proposed - 1;
399400 } else {
400401 lower = proposed;
401402 }
402403 }
404 const slots_end = lower * size_class;
405 const header_begin = mem.alignForward(usize, slots_end, @alignOf(BucketHeader));
406 const end = header_begin + bucketSize(lower);
407 assert(end <= page_size);
403408 return lower;
404409 }
405410