From fa49d883e25c9c0871070779bfd1549be00517cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20Ingve=20Schj=C3=B8lberg?= Date: Mon, 20 Jul 2026 10:59:58 +0200 Subject: [PATCH 1/3] [vulkan] Harden split_block_region against mis-sized splits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit split_block_region derived the kept region's size by subtracting the conformed remainder from the original free region, which assumes conforming the remainder leaves it unchanged. That assumption doesn't hold: the remainder is an arbitrary leftover that was never conformed as its own buffer, and Vulkan's conform sizes it from vkGetBufferMemoryRequirements (then rounds up to nearest_multiple), which may pad it beyond the arithmetic leftover — even when the incoming request was fully conformed. When that happens the subtraction sizes the kept region smaller than the request, and the VkBuffer later created for it is undersized. Size the allocated region directly from the remainder's conformed offset instead, so conform-induced padding of the remainder can no longer shrink it. Add asserts that the two regions stay within the original free region, turning the remaining unhandled case — a conformed remainder overrunning into the next region — into a loud failure rather than silent overlap. Co-authored-by: Claude Fable 5 --- src/runtime/internal/region_allocator.h | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/runtime/internal/region_allocator.h b/src/runtime/internal/region_allocator.h index 8c04116a3a65..9a0148d578b3 100644 --- a/src/runtime/internal/region_allocator.h +++ b/src/runtime/internal/region_allocator.h @@ -448,6 +448,8 @@ bool RegionAllocator::can_split(const BlockRegion *block_region, const MemoryReq } BlockRegion *RegionAllocator::split_block_region(void *user_context, BlockRegion *block_region, const MemoryRequest &request) { + const size_t original_size = block_region->memory.allocation.size; + const size_t original_offset = block_region->memory.allocation.offset; if ((block_region->usage_count == 0) && (block_region->memory.handle != nullptr)) { #ifdef DEBUG_RUNTIME_INTERNAL @@ -465,12 +467,12 @@ BlockRegion *RegionAllocator::split_block_region(void *user_context, BlockRegion } MemoryRequest split_request = request; - split_request.size = block_region->memory.allocation.size - request.size; - split_request.offset = block_region->memory.allocation.offset + request.size; + split_request.size = original_size - request.size; + split_request.offset = original_offset + request.size; #ifdef DEBUG_RUNTIME_INTERNAL debug(user_context) << "RegionAllocator: Splitting " - << "current region (offset=" << (int32_t)block_region->memory.allocation.offset << " size=" << (int32_t)(block_region->memory.allocation.size) << " bytes) " + << "current region (offset=" << (int32_t)original_offset << " size=" << (int32_t)(original_size) << " bytes) " << "to create empty region (offset=" << (int32_t)split_request.offset << " size=" << (int32_t)(split_request.size) << " bytes)"; #endif BlockRegion *next_region = block_region->next_ptr; @@ -483,7 +485,13 @@ BlockRegion *RegionAllocator::split_block_region(void *user_context, BlockRegion } empty_region->prev_ptr = block_region; block_region->next_ptr = empty_region; - block_region->memory.allocation.size -= empty_region->memory.allocation.size; + + // Derive the allocated region's size from the empty region's conformed offset; + // subtracting the empty region's size would shrink this region below the request + // whenever conforming the empty region adjusted its offset or size. + block_region->memory.allocation.size = empty_region->memory.allocation.offset - original_offset; + halide_abort_if_false(user_context, block_region->memory.allocation.size >= request.size); + halide_abort_if_false(user_context, empty_region->memory.allocation.offset + empty_region->memory.allocation.size <= original_offset + original_size); return empty_region; } From 1315d7c2f2fea7b01673b217f24b54c4a0149085 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20Ingve=20Schj=C3=B8lberg?= Date: Mon, 20 Jul 2026 13:04:10 +0200 Subject: [PATCH 2/3] [vulkan] Invalidate stale region handle when coalescing into prev region RegionAllocator caches each region's VkBuffer (memory.handle) across free/reuse, on the implicit invariant that a cached handle's size equals the region's allocation size. coalesce_block_regions broke that invariant: when it merged a collected region into an available previous neighbour it grew prev_region's allocation.size but left prev_region's cached handle untouched, still sized to the old (smaller) region. A later reservation landing on the coalesced region with an exact conformed-size match takes the handle-reuse path in alloc_block_region, which reuses the cached handle with no size check, and is handed a VkBuffer smaller than the region. Deallocate the previous neighbour's handle before growing it, so the merged region falls back to a fresh, correctly-sized allocation on next use. Extract the repeated "deallocate the handle if the region is unused" guard into deallocate_block_region_if_unused and use it at the three existing sites (coalesce, split, free) plus the new prev-merge site; free_block_region now layers its status/usage reset on top of the helper. Co-authored-by: Claude Opus 4.8 --- src/runtime/internal/region_allocator.h | 72 ++++++++++--------------- 1 file changed, 27 insertions(+), 45 deletions(-) diff --git a/src/runtime/internal/region_allocator.h b/src/runtime/internal/region_allocator.h index 9a0148d578b3..b8dccc377e60 100644 --- a/src/runtime/internal/region_allocator.h +++ b/src/runtime/internal/region_allocator.h @@ -94,6 +94,9 @@ class RegionAllocator { // Invokes the deallocation callback to free memory for the block region int free_block_region(void *user_context, BlockRegion *region); + // Deallocate only if the region is unused. + int deallocate_block_region_if_unused(void *user_context, BlockRegion *region); + // Returns true if the given block region is the last region in the list bool is_last_block_region(void *user_context, const BlockRegion *region) const; @@ -389,24 +392,11 @@ bool RegionAllocator::can_coalesce(const BlockRegion *block_region) const { } BlockRegion *RegionAllocator::coalesce_block_regions(void *user_context, BlockRegion *block_region) { - - if ((block_region->usage_count == 0) && (block_region->memory.handle != nullptr)) { -#ifdef DEBUG_RUNTIME_INTERNAL - debug(user_context) << "RegionAllocator: Freeing unused region to coalesce (" - << "block_ptr=" << (void *)block_region->block_ptr << " " - << "block_region=" << (void *)block_region << " " - << "memory_size=" << (uint32_t)(block_region->memory.allocation.size) << " " - << "block_reserved=" << (uint32_t)block->reserved << " " - << ")"; -#endif - halide_abort_if_false(user_context, allocators.region.deallocate != nullptr); - MemoryRegion *memory_region = &(block_region->memory); - allocators.region.deallocate(user_context, memory_region); - block_region->memory.handle = nullptr; - } + deallocate_block_region_if_unused(user_context, block_region); BlockRegion *prev_region = block_region->prev_ptr; if (is_available(prev_region) && (prev_region != block_region)) { + deallocate_block_region_if_unused(user_context, prev_region); #ifdef DEBUG_RUNTIME_INTERNAL debug(user_context) << "RegionAllocator: Coalescing " @@ -451,20 +441,7 @@ BlockRegion *RegionAllocator::split_block_region(void *user_context, BlockRegion const size_t original_size = block_region->memory.allocation.size; const size_t original_offset = block_region->memory.allocation.offset; - if ((block_region->usage_count == 0) && (block_region->memory.handle != nullptr)) { -#ifdef DEBUG_RUNTIME_INTERNAL - debug(user_context) << "RegionAllocator: Split deallocate region (" - << "block_ptr=" << (void *)block_region->block_ptr << " " - << "block_region=" << (void *)block_region << " " - << "memory_size=" << (uint32_t)(block_region->memory.allocation.size) << " " - << "block_reserved=" << (uint32_t)block_region->block_ptr->reserved << " " - << ")"; -#endif - halide_abort_if_false(user_context, allocators.region.deallocate != nullptr); - MemoryRegion *memory_region = &(block_region->memory); - allocators.region.deallocate(user_context, memory_region); - block_region->memory.handle = nullptr; - } + deallocate_block_region_if_unused(user_context, block_region); MemoryRequest split_request = request; split_request.size = original_size - request.size; @@ -659,27 +636,32 @@ int RegionAllocator::free_block_region(void *user_context, BlockRegion *block_re << "usage_count=" << (uint32_t)block_region->usage_count << " " << "block_reserved=" << (uint32_t)block->reserved << ")"; #endif - int error_code = 0; - if ((block_region->usage_count == 0) && (block_region->memory.handle != nullptr)) { -#ifdef DEBUG_RUNTIME_INTERNAL - debug(user_context) << " deallocating region (" - << "block_ptr=" << (void *)block_region->block_ptr << " " - << "block_region=" << (void *)block_region << " " - << "memory_size=" << (uint32_t)(block_region->memory.allocation.size) << " " - << "block_reserved=" << (uint32_t)block->reserved << " " - << ")"; -#endif - // NOTE: Deallocate but leave memory size as is, so that coalesce can compute region merging sizes - halide_abort_if_false(user_context, allocators.region.deallocate != nullptr); - MemoryRegion *memory_region = &(block_region->memory); - error_code = allocators.region.deallocate(user_context, memory_region); - block_region->memory.handle = nullptr; - } + int error_code = deallocate_block_region_if_unused(user_context, block_region); block_region->usage_count = 0; block_region->status = AllocationStatus::Available; return error_code; } +int RegionAllocator::deallocate_block_region_if_unused(void *user_context, BlockRegion *region) { + if ((region->usage_count != 0) || (region->memory.handle == nullptr)) { + return 0; + } + +#ifdef DEBUG_RUNTIME_INTERNAL + debug(user_context) << "RegionAllocator: deallocating unused region (" + << "block_ptr=" << (void *)region->block_ptr << " " + << "block_region=" << (void *)region << " " + << "memory_size=" << (uint32_t)(region->memory.allocation.size) << " " + << "block_reserved=" << (uint32_t)block->reserved << " " + << ")"; +#endif + halide_abort_if_false(user_context, allocators.region.deallocate != nullptr); + MemoryRegion *memory_region = &(region->memory); + int error_code = allocators.region.deallocate(user_context, memory_region); + region->memory.handle = nullptr; + return error_code; +} + int RegionAllocator::release(void *user_context) { #ifdef DEBUG_RUNTIME_INTERNAL debug(user_context) << "RegionAllocator: Releasing all regions (" From 70566592b7e5dd4cddd3d7a43837b149572a3163 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20Ingve=20Schj=C3=B8lberg?= Date: Mon, 20 Jul 2026 17:10:06 +0200 Subject: [PATCH 3/3] [vulkan] Verify the region handle-size invariant in allocator tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RegionAllocator caches each region's handle across release/reserve and reuses it in alloc_block_region with no size check, on the invariant that a cached handle's backing allocation always matches the region's allocation.size. The test mocks couldn't see a violation: the mock handle was the constant sentinel (void *)1, carrying no size information. Encode the allocation size in the mock handle instead, and check it at every point the runtime hands a region across the callback boundary: on entry to allocate_region (a fresh allocation must never clobber a live handle), in deallocate_region (the handle must still match the region's size at death), and after every reserve() in the test cases — including the stress-test loops, where cached handles are actually re-issued under churn. The nullptr checks previously done at each reserve() site are folded into the new check_region helper. Co-authored-by: Claude Fable 5 --- test/runtime/block_allocator.cpp | 47 +++++++++++++++++++++----------- 1 file changed, 31 insertions(+), 16 deletions(-) diff --git a/test/runtime/block_allocator.cpp b/test/runtime/block_allocator.cpp index 7bc3d5ebd92f..80e53e79b0f9 100644 --- a/test/runtime/block_allocator.cpp +++ b/test/runtime/block_allocator.cpp @@ -43,8 +43,19 @@ int deallocate_block(void *user_context, MemoryBlock *block) { return halide_error_code_success; } +void *encode_region_handle(size_t size) { + return (void *)(size + 1); +} + +void check_region(void *user_context, MemoryRegion *region) { + HALIDE_CHECK(user_context, region != nullptr); + HALIDE_CHECK(user_context, region->handle == encode_region_handle(region->allocation.size)); +} + int allocate_region(void *user_context, MemoryRegion *region) { - region->handle = (void *)1; + HALIDE_CHECK(user_context, region->handle == nullptr); + + region->handle = encode_region_handle(region->allocation.size); allocated_region_memory += region->allocation.size; debug(user_context) << "Test : allocate_region (" @@ -57,7 +68,8 @@ int allocate_region(void *user_context, MemoryRegion *region) { } int deallocate_region(void *user_context, MemoryRegion *region) { - region->handle = (void *)0; + HALIDE_CHECK(user_context, region->handle == encode_region_handle(region->allocation.size)); + region->handle = nullptr; allocated_region_memory -= region->allocation.size; debug(user_context) << "Test : deallocate_region (" @@ -120,12 +132,12 @@ int main(int argc, char **argv) { request.properties.usage = MemoryUsage::DefaultUsage; MemoryRegion *r1 = instance->reserve(user_context, request); - HALIDE_CHECK(user_context, r1 != nullptr); + check_region(user_context, r1); HALIDE_CHECK(user_context, allocated_block_memory == block_size); HALIDE_CHECK(user_context, allocated_region_memory == request.size); MemoryRegion *r2 = instance->reserve(user_context, request); - HALIDE_CHECK(user_context, r2 != nullptr); + check_region(user_context, r2); HALIDE_CHECK(user_context, allocated_block_memory == block_size); HALIDE_CHECK(user_context, allocated_region_memory == (2 * request.size)); @@ -133,7 +145,7 @@ int main(int argc, char **argv) { HALIDE_CHECK(user_context, allocated_region_memory == (1 * request.size)); MemoryRegion *r3 = instance->reserve(user_context, request); - halide_abort_if_false(user_context, r3 != nullptr); + check_region(user_context, r3); halide_abort_if_false(user_context, allocated_block_memory == block_size); halide_abort_if_false(user_context, allocated_region_memory == (2 * request.size)); instance->retain(user_context, r3); @@ -152,9 +164,9 @@ int main(int argc, char **argv) { request.size = block_size / 2; // request two half-size regions MemoryRegion *r4 = instance->reserve(user_context, request); - HALIDE_CHECK(user_context, r4 != nullptr); + check_region(user_context, r4); MemoryRegion *r5 = instance->reserve(user_context, request); - HALIDE_CHECK(user_context, r5 != nullptr); + check_region(user_context, r5); HALIDE_CHECK(user_context, nullptr == instance->reserve(user_context, request)); // requesting a third should fail HALIDE_CHECK(user_context, allocated_block_memory == block_size); @@ -167,7 +179,7 @@ int main(int argc, char **argv) { request.size = block_size; MemoryRegion *r6 = instance->reserve(user_context, request); - HALIDE_CHECK(user_context, r6 != nullptr); + check_region(user_context, r6); instance->destroy(user_context); deallocate_block(user_context, memory_block); @@ -308,12 +320,12 @@ int main(int argc, char **argv) { request.properties.usage = MemoryUsage::DefaultUsage; MemoryRegion *r1 = instance->reserve(user_context, request); - HALIDE_CHECK(user_context, r1 != nullptr); + check_region(user_context, r1); HALIDE_CHECK(user_context, allocated_block_memory == block_size); HALIDE_CHECK(user_context, allocated_region_memory == padded_size); MemoryRegion *r2 = instance->reserve(user_context, request); - HALIDE_CHECK(user_context, r2 != nullptr); + check_region(user_context, r2); HALIDE_CHECK(user_context, allocated_block_memory == block_size); HALIDE_CHECK(user_context, allocated_region_memory == (2 * padded_size)); @@ -324,9 +336,9 @@ int main(int argc, char **argv) { request.size = block_size / 2; // request two half-size regions MemoryRegion *r4 = instance->reserve(user_context, request); - HALIDE_CHECK(user_context, r4 != nullptr); + check_region(user_context, r4); MemoryRegion *r5 = instance->reserve(user_context, request); - HALIDE_CHECK(user_context, r5 != nullptr); + check_region(user_context, r5); HALIDE_CHECK(user_context, nullptr == instance->reserve(user_context, request)); // requesting a third should fail HALIDE_CHECK(user_context, allocated_block_memory == block_size); @@ -339,7 +351,7 @@ int main(int argc, char **argv) { request.size = block_size; MemoryRegion *r6 = instance->reserve(user_context, request); - HALIDE_CHECK(user_context, r6 != nullptr); + check_region(user_context, r6); instance->destroy(user_context); deallocate_block(user_context, memory_block); @@ -380,12 +392,12 @@ int main(int argc, char **argv) { request.properties.usage = MemoryUsage::DefaultUsage; MemoryRegion *r1 = instance->reserve(user_context, request); - HALIDE_CHECK(user_context, r1 != nullptr); + check_region(user_context, r1); HALIDE_CHECK(user_context, allocated_block_memory == config.minimum_block_size); HALIDE_CHECK(user_context, allocated_region_memory == request.size); MemoryRegion *r2 = instance->reserve(user_context, request); - HALIDE_CHECK(user_context, r2 != nullptr); + check_region(user_context, r2); HALIDE_CHECK(user_context, allocated_block_memory == config.minimum_block_size); HALIDE_CHECK(user_context, allocated_region_memory == (2 * request.size)); @@ -393,7 +405,7 @@ int main(int argc, char **argv) { HALIDE_CHECK(user_context, allocated_region_memory == (1 * request.size)); MemoryRegion *r3 = instance->reserve(user_context, request); - halide_abort_if_false(user_context, r3 != nullptr); + check_region(user_context, r3); halide_abort_if_false(user_context, allocated_block_memory == config.minimum_block_size); halide_abort_if_false(user_context, allocated_region_memory == (2 * request.size)); instance->retain(user_context, r3); @@ -489,6 +501,7 @@ int main(int argc, char **argv) { count = count > 1 ? count : 1; request.size = count * sizeof(int); MemoryRegion *region = instance->reserve(user_context, request); + check_region(user_context, region); pointers.append(user_context, region); } @@ -533,6 +546,7 @@ int main(int argc, char **argv) { request.size = count * sizeof(int); total_allocation_size += request.size; MemoryRegion *region = instance->reserve(user_context, request); + check_region(user_context, region); pointers.append(user_context, region); } @@ -549,6 +563,7 @@ int main(int argc, char **argv) { count = count > 1 ? count : 1; request.size = count * sizeof(int); MemoryRegion *region = instance->reserve(user_context, request); + check_region(user_context, region); pointers.append(user_context, region); }