From 16d06d0640b60aface0f0ca73a404175b0592e35 Mon Sep 17 00:00:00 2001 From: Rose <83477269+AtariDreams@users.noreply.github.com> Date: Sat, 16 Dec 2023 11:13:38 -0500 Subject: [PATCH] Use proper atomics and ref counting Prefer __atomic_compare_exchange_n over __sync_bool_compare_and_swap. I chose weak because we are looping and reading the value of old_value constantly anyway, so it would be better to have it weak. Otherwise, it is equivalent to what it was before. --- src/BlocksRuntime/runtime.c | 5 +++-- src/allocator.c | 36 +++++++++++++++++++----------------- 2 files changed, 22 insertions(+), 19 deletions(-) diff --git a/src/BlocksRuntime/runtime.c b/src/BlocksRuntime/runtime.c index 4b7d4bfa2..a71b2c2df 100644 --- a/src/BlocksRuntime/runtime.c +++ b/src/BlocksRuntime/runtime.c @@ -12,6 +12,7 @@ #include #include #include +#include #if HAVE_OBJC #define __USE_GNU #include @@ -32,9 +33,9 @@ #define __has_builtin(builtin) 0 #endif -#if __has_builtin(__sync_bool_compare_and_swap) +#if __has_builtin(__atomic_compare_exchange_n) #define OSAtomicCompareAndSwapInt(_Old, _New, _Ptr) \ - __sync_bool_compare_and_swap(_Ptr, _Old, _New) + __atomic_compare_exchange_n(_Ptr, &_Old, _New, true, memory_order_seq_cst, memory_order_seq_cst) #else #define _CRT_SECURE_NO_WARNINGS 1 #include diff --git a/src/allocator.c b/src/allocator.c index 66284090f..05b6dc85b 100644 --- a/src/allocator.c +++ b/src/allocator.c @@ -542,31 +542,33 @@ _dispatch_alloc_maybe_madvise_page(dispatch_continuation_t c) } // They are all unallocated, so we could madvise the page. Try to // take ownership of them all. - int last_locked = 0; - do { - if (!os_atomic_cmpxchg(&page_bitmaps[last_locked], BITMAP_C(0), + for (i = 0; i < BITMAPS_PER_PAGE; i++) { + if (!os_atomic_cmpxchg(&page_bitmaps[i], BITMAP_C(0), BITMAP_ALL_ONES, relaxed)) { // We didn't get one; since there is a cont allocated in // the page, we can't madvise. Give up and unlock all. - goto unlock; + break; } - } while (++last_locked < (signed)BITMAPS_PER_PAGE); + } + + if (i >= BITMAPS_PER_PAGE) { #if DISPATCH_DEBUG - //fprintf(stderr, "%s: madvised page %p for cont %p (next = %p), " - // "[%u+1]=%u bitmaps at %p\n", __func__, page, c, c->do_next, - // last_locked-1, BITMAPS_PER_PAGE, &page_bitmaps[0]); - // Scribble to expose use-after-free bugs - // madvise (syscall) flushes these stores - memset(page, DISPATCH_ALLOCATOR_SCRIBBLE, DISPATCH_ALLOCATOR_PAGE_SIZE); + // fprintf(stderr, "%s: madvised page %p for cont %p (next = %p), " + // "[%u+1]=%u bitmaps at %p\n", __func__, page, c, c->do_next, + // last_locked-1, BITMAPS_PER_PAGE, &page_bitmaps[0]); + // Scribble to expose use-after-free bugs + // madvise (syscall) flushes these stores + memset(page, DISPATCH_ALLOCATOR_SCRIBBLE, DISPATCH_ALLOCATOR_PAGE_SIZE); #endif - (void)dispatch_assume_zero(madvise(page, DISPATCH_ALLOCATOR_PAGE_SIZE, - MADV_FREE)); + // madvise the page + (void)dispatch_assume_zero(madvise(page, DISPATCH_ALLOCATOR_PAGE_SIZE, + MADV_FREE)); + } -unlock: - while (last_locked > 1) { - page_bitmaps[--last_locked] = BITMAP_C(0); + while (i > 1) { + page_bitmaps[--i] = BITMAP_C(0); } - if (last_locked) { + if (i) { os_atomic_store(&page_bitmaps[0], BITMAP_C(0), relaxed); } return;