Skip to content

Commit 16d06d0

Browse files
committed
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.
1 parent ee39300 commit 16d06d0

File tree

2 files changed

+22
-19
lines changed

2 files changed

+22
-19
lines changed

src/BlocksRuntime/runtime.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include <stdlib.h>
1313
#include <string.h>
1414
#include <stdint.h>
15+
#include <stdatomic.h>
1516
#if HAVE_OBJC
1617
#define __USE_GNU
1718
#include <dlfcn.h>
@@ -32,9 +33,9 @@
3233
#define __has_builtin(builtin) 0
3334
#endif
3435

35-
#if __has_builtin(__sync_bool_compare_and_swap)
36+
#if __has_builtin(__atomic_compare_exchange_n)
3637
#define OSAtomicCompareAndSwapInt(_Old, _New, _Ptr) \
37-
__sync_bool_compare_and_swap(_Ptr, _Old, _New)
38+
__atomic_compare_exchange_n(_Ptr, &_Old, _New, true, memory_order_seq_cst, memory_order_seq_cst)
3839
#else
3940
#define _CRT_SECURE_NO_WARNINGS 1
4041
#include <Windows.h>

src/allocator.c

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -542,31 +542,33 @@ _dispatch_alloc_maybe_madvise_page(dispatch_continuation_t c)
542542
}
543543
// They are all unallocated, so we could madvise the page. Try to
544544
// take ownership of them all.
545-
int last_locked = 0;
546-
do {
547-
if (!os_atomic_cmpxchg(&page_bitmaps[last_locked], BITMAP_C(0),
545+
for (i = 0; i < BITMAPS_PER_PAGE; i++) {
546+
if (!os_atomic_cmpxchg(&page_bitmaps[i], BITMAP_C(0),
548547
BITMAP_ALL_ONES, relaxed)) {
549548
// We didn't get one; since there is a cont allocated in
550549
// the page, we can't madvise. Give up and unlock all.
551-
goto unlock;
550+
break;
552551
}
553-
} while (++last_locked < (signed)BITMAPS_PER_PAGE);
552+
}
553+
554+
if (i >= BITMAPS_PER_PAGE) {
554555
#if DISPATCH_DEBUG
555-
//fprintf(stderr, "%s: madvised page %p for cont %p (next = %p), "
556-
// "[%u+1]=%u bitmaps at %p\n", __func__, page, c, c->do_next,
557-
// last_locked-1, BITMAPS_PER_PAGE, &page_bitmaps[0]);
558-
// Scribble to expose use-after-free bugs
559-
// madvise (syscall) flushes these stores
560-
memset(page, DISPATCH_ALLOCATOR_SCRIBBLE, DISPATCH_ALLOCATOR_PAGE_SIZE);
556+
// fprintf(stderr, "%s: madvised page %p for cont %p (next = %p), "
557+
// "[%u+1]=%u bitmaps at %p\n", __func__, page, c, c->do_next,
558+
// last_locked-1, BITMAPS_PER_PAGE, &page_bitmaps[0]);
559+
// Scribble to expose use-after-free bugs
560+
// madvise (syscall) flushes these stores
561+
memset(page, DISPATCH_ALLOCATOR_SCRIBBLE, DISPATCH_ALLOCATOR_PAGE_SIZE);
561562
#endif
562-
(void)dispatch_assume_zero(madvise(page, DISPATCH_ALLOCATOR_PAGE_SIZE,
563-
MADV_FREE));
563+
// madvise the page
564+
(void)dispatch_assume_zero(madvise(page, DISPATCH_ALLOCATOR_PAGE_SIZE,
565+
MADV_FREE));
566+
}
564567

565-
unlock:
566-
while (last_locked > 1) {
567-
page_bitmaps[--last_locked] = BITMAP_C(0);
568+
while (i > 1) {
569+
page_bitmaps[--i] = BITMAP_C(0);
568570
}
569-
if (last_locked) {
571+
if (i) {
570572
os_atomic_store(&page_bitmaps[0], BITMAP_C(0), relaxed);
571573
}
572574
return;

0 commit comments

Comments
 (0)