From 6b63ad0d588227c96de971c807f6522f69926e72 Mon Sep 17 00:00:00 2001 From: Dmitry Date: Fri, 6 Nov 2026 19:50:05 +0200 Subject: [PATCH] Allocate more block trampolines when all sets are full imp_implementationWithBlock() allocated a trampoline set only when the list was empty. Once the first set was full (128 IMPs on AArch64, 256 on x86-64, with 4 KiB pages) it reached UNREACHABLE() and returned a garbage IMP. With assertions enabled it aborted one IMP earlier, because the check that `set->first_free < trampoline_header_per_page` compares -1 with a size_t. When no set has a free trampoline, allocate a new set and put it at the head of the list. The older sets stay in the list, so imp_getBlock() and imp_removeBlock() still find their IMPs, and trampolines freed in any set are reused. Return a null IMP if a set can't be allocated. Also fix three smaller bugs that the tests now catch: - Store the copy of the block in the trampoline, not the block that was passed in. For a stack block, the IMP called through a dead stack frame and imp_removeBlock() released the wrong block. - imp_getBlock() returns nil for a removed IMP, instead of the free list link. - imp_removeBlock() returns NO for an IMP that was already removed, instead of releasing the free list link and putting the trampoline on the free list twice. Add Test/ManyBlockImps.m, which sets and unsets 5000 block IMPs (normal and struct-return), and a stack block case to Test/BlockImpTest.m. --- Test/BlockImpTest.m | 19 +++++ Test/CMakeLists.txt | 2 +- Test/ManyBlockImps.m | 187 +++++++++++++++++++++++++++++++++++++++++++ block_to_imp.c | 87 ++++++++++++++------ 4 files changed, 269 insertions(+), 26 deletions(-) create mode 100644 Test/ManyBlockImps.m diff --git a/Test/BlockImpTest.m b/Test/BlockImpTest.m index f16bc4e2..7d38410d 100644 --- a/Test/BlockImpTest.m +++ b/Test/BlockImpTest.m @@ -21,6 +21,18 @@ +(int)count: (int)i; +(struct big)sret; @end +// Makes an IMP from a block that is still on the stack. +__attribute__((noinline)) +static IMP stackBlockIMP(int value) +{ + int (^blk)(id) = ^(id self) { return value; }; + IMP imp = imp_implementationWithBlock(blk); + // The runtime must keep its own copy: this frame is gone by the time the + // IMP is called. + assert(imp_getBlock(imp) != NULL); + assert(imp_getBlock(imp) != blk); + return imp; +} int main(void) { @@ -67,5 +79,12 @@ int main(void) assert(s.e == 5); imp_removeBlock(imp); assert(imp_getBlock(imp) != blk); + + IMP stackImp = stackBlockIMP(7); + IMP stackImp2 = stackBlockIMP(42); + assert(7 == ((int(*)(id,SEL))stackImp)(cls, @selector(count:))); + assert(42 == ((int(*)(id,SEL))stackImp2)(cls, @selector(count:))); + assert(imp_removeBlock(stackImp)); + assert(imp_removeBlock(stackImp2)); return 0; } diff --git a/Test/CMakeLists.txt b/Test/CMakeLists.txt index f2c0544c..c718746b 100644 --- a/Test/CMakeLists.txt +++ b/Test/CMakeLists.txt @@ -57,7 +57,7 @@ set(TESTS ) if (EMBEDDED_BLOCKS_RUNTIME) - list(APPEND TESTS BlockImpTest.m) + list(APPEND TESTS BlockImpTest.m ManyBlockImps.m) endif() set(ENABLE_ALL_OBJC_ARC_TESTS On) diff --git a/Test/ManyBlockImps.m b/Test/ManyBlockImps.m new file mode 100644 index 00000000..db3d8a6e --- /dev/null +++ b/Test/ManyBlockImps.m @@ -0,0 +1,187 @@ +#include "../objc/runtime.h" +#include "../objc/blocks_runtime.h" +#include +#include +#include +#include + +/* + * Sets and unsets a few thousand blocks as IMPs. One set of trampolines holds + * one page worth of them (128 on AArch64 and 256 on x86-64 with 4 KiB pages), + * so the runtime needs many sets and must reuse the trampolines that are freed + * in any of them. + */ + +#define COUNT 5000 + +struct big +{ + int a, b, c, d, e; +}; + +/* + * Makes an IMP from a heap copy of a block, and leaves the IMP with the only + * reference to that copy. + */ +static IMP heapBlockIMP(id stackBlock) +{ + id block = Block_copy(stackBlock); + IMP imp = imp_implementationWithBlock(block); + assert(imp != NULL); + assert(imp_getBlock(imp) == block); + Block_release(block); + return imp; +} + +/* + * Returns an IMP that returns value, or a struct big that starts with value. + */ +static IMP makeIMP(int value, BOOL sret) +{ + if (sret) + { + struct big (^block)(id) = ^(id self) + { + struct big b = { value, value + 1, value + 2, value + 3, value + 4 }; + return b; + }; + return heapBlockIMP(block); + } + int (^block)(id) = ^(id self) { return value; }; + return heapBlockIMP(block); +} + +static int checkBig(struct big b) +{ + assert(b.b == b.a + 1); + assert(b.c == b.a + 2); + assert(b.d == b.a + 3); + assert(b.e == b.a + 4); + return b.a; +} + +static void checkIMP(IMP imp, int value, BOOL sret) +{ + id block = imp_getBlock(imp); + assert(block != nil); + if (sret) + { + assert(checkBig(((struct big(*)(id, SEL))imp)(nil, @selector(big))) == value); + assert(checkBig(((struct big(^)(id))block)(nil)) == value); + } + else + { + assert(((int(*)(id, SEL))imp)(nil, @selector(value)) == value); + assert(((int(^)(id))block)(nil) == value); + } +} + +static void removeIMP(IMP imp) +{ + assert(imp_removeBlock(imp)); + assert(imp_getBlock(imp) == nil); + // Removing it twice must not put the trampoline on the free list twice. + assert(!imp_removeBlock(imp)); +} + +static int compareIMPs(const void *a, const void *b) +{ + uintptr_t x = (uintptr_t)*(const IMP *)a; + uintptr_t y = (uintptr_t)*(const IMP *)b; + return (x > y) - (x < y); +} + +static void assertDistinct(IMP *imps) +{ + IMP *sorted = calloc(COUNT, sizeof(IMP)); + memcpy(sorted, imps, COUNT * sizeof(IMP)); + qsort(sorted, COUNT, sizeof(IMP), compareIMPs); + for (int i=1 ; i= freedCount / 2); + assertDistinct(imps); + for (int i=0 ; iregion = VirtualAlloc(NULL, trampoline_region_size, MEM_COMMIT | MEM_RESERVE, PAGE_READWRITE); #else metadata->region = mmap(NULL, trampoline_region_size, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0); + if (metadata->region == MAP_FAILED) + { + metadata->region = NULL; + } #endif + if (metadata->region == NULL) + { + free(metadata); + return NULL; + } struct block_header *headers_start = REGION_HEADERS_START(metadata); char *rx_buffer_start = REGION_RX_BUFFER_START(metadata); for (int i=0 ; i= trampolineSize) { return 0; } - block = Block_copy(block); - // Allocate some trampolines if this is the first time that we need to do this. - if (*setptr == NULL) + struct trampoline_set *set; + for (set=*setptr ; set!=NULL ; set=set->next) { - *setptr = alloc_trampolines(start, end); + if (set->first_free != -1) + { + break; + } } - for (struct trampoline_set *set=*setptr ; set!=NULL ; set=set->next) + // If this is the first time that we need trampolines, or all of the sets + // are full, then allocate a new set. Put it at the start of the list, so + // that the next allocations find a free trampoline straight away, but keep + // the older sets in the list: imp_getBlock() and imp_removeBlock() must + // still find their IMPs, and trampolines freed in them must be reused. + if (set == NULL) { - if (set->first_free != -1) + set = alloc_trampolines(start, end); + if (set == NULL) { - int i = set->first_free; - struct block_header *headers_start = REGION_HEADERS_START(set); - char *rx_buffer_start = REGION_RX_BUFFER_START(set); - struct block_header *h = &headers_start[i]; - struct block_header *next = h->block; - set->first_free = next ? (next - headers_start) : -1; - assert(set->first_free < trampoline_header_per_page); - assert(set->first_free >= -1); - h->fnptr = (void(*)(void))b->invoke; - h->block = b; - uintptr_t addr = (uintptr_t)&rx_buffer_start[i*sizeof(struct block_header)]; -#if (__ARM_ARCH_ISA_THUMB == 2) - // If the trampoline is Thumb-2 code, then we must set the low bit - // to 1 so that b[l]x instructions put the CPU in the correct mode. - addr |= 1; -#endif - return (IMP)addr; + return 0; } + set->next = *setptr; + *setptr = set; } - UNREACHABLE("Failed to allocate block"); + int i = set->first_free; + struct block_header *headers_start = REGION_HEADERS_START(set); + char *rx_buffer_start = REGION_RX_BUFFER_START(set); + struct block_header *h = &headers_start[i]; + struct block_header *next = h->block; + set->first_free = next ? (next - headers_start) : -1; + assert(set->first_free < (int)trampoline_header_per_page); + assert(set->first_free >= -1); + // Store the copy, not the block that we were passed: if that one is on the + // stack, then it is gone once the caller returns. + b = Block_copy(b); + h->fnptr = (void(*)(void))b->invoke; + h->block = b; + uintptr_t addr = (uintptr_t)&rx_buffer_start[i*sizeof(struct block_header)]; +#if (__ARM_ARCH_ISA_THUMB == 2) + // If the trampoline is Thumb-2 code, then we must set the low bit + // to 1 so that b[l]x instructions put the CPU in the correct mode. + addr |= 1; +#endif + return (IMP)addr; } static int indexForIMP(IMP anIMP, struct trampoline_set **setptr) @@ -379,7 +405,13 @@ id imp_getBlock(IMP anImp) { return NULL; } - return REGION_HEADERS_START(set)[idx].block; + struct block_header *h = ®ION_HEADERS_START(set)[idx]; + // A free trampoline's block field is a free list link, not a block. + if (h->fnptr == (void(*)(void))invalid) + { + return NULL; + } + return h->block; } BOOL imp_removeBlock(IMP anImp) @@ -398,6 +430,11 @@ BOOL imp_removeBlock(IMP anImp) } struct block_header *header_start = REGION_HEADERS_START(set); struct block_header *h = &header_start[idx]; + // Removing a free trampoline again would corrupt the free list. + if (h->fnptr == (void(*)(void))invalid) + { + return NO; + } Block_release(h->block); h->fnptr = (void(*)(void))invalid; h->block = set->first_free == -1 ? NULL : &header_start[set->first_free];