From 91709ac2fe84a2e4440f65e6cd2fa986a5ed0b77 Mon Sep 17 00:00:00 2001 From: Stefan Berger Date: Mon, 29 Jun 2020 10:38:56 -0400 Subject: [PATCH] [LibOS,Pal] Set `weak` parameter in atomics operations to false The `weak` parameter has no influence on x86, but on ppc64 it causes spurious faults due to a missing check on flags (missing `bne` instruction). --- LibOS/shim/src/bookkeep/shim_signal.c | 4 ++-- LibOS/shim/src/sys/shim_futex.c | 2 +- Pal/include/lib/spinlock.h | 4 ++-- Pal/src/host/Linux-SGX/db_events.c | 2 +- Pal/src/host/Linux-SGX/db_mutex.c | 4 ++-- Pal/src/host/Linux-SGX/enclave_ocalls.c | 3 ++- Pal/src/host/Linux/db_events.c | 2 +- Pal/src/host/Linux/db_mutex.c | 2 +- 8 files changed, 12 insertions(+), 11 deletions(-) diff --git a/LibOS/shim/src/bookkeep/shim_signal.c b/LibOS/shim/src/bookkeep/shim_signal.c index 8927f585..169ee752 100644 --- a/LibOS/shim/src/bookkeep/shim_signal.c +++ b/LibOS/shim/src/bookkeep/shim_signal.c @@ -130,7 +130,7 @@ static bool append_rt_signal(struct shim_rt_signal_queue* queue, if (put_idx - get_idx >= ARRAY_SIZE(queue->queue)) { return false; } - } while (!__atomic_compare_exchange_n(&queue->put_idx, &put_idx, put_idx + 1, /*weak=*/true, + } while (!__atomic_compare_exchange_n(&queue->put_idx, &put_idx, put_idx + 1, /*weak=*/false, __ATOMIC_RELEASE, __ATOMIC_ACQUIRE)); queue->queue[put_idx % ARRAY_SIZE(queue->queue)] = signal; @@ -179,7 +179,7 @@ static struct shim_signal* pop_rt_signal(struct shim_rt_signal_queue* queue) { if (put_idx == get_idx) { return NULL; } - } while (!__atomic_compare_exchange_n(&queue->get_idx, &get_idx, get_idx + 1, /*weak=*/true, + } while (!__atomic_compare_exchange_n(&queue->get_idx, &get_idx, get_idx + 1, /*weak=*/false, __ATOMIC_RELEASE, __ATOMIC_ACQUIRE)); return queue->queue[get_idx % ARRAY_SIZE(queue->queue)]; diff --git a/LibOS/shim/src/sys/shim_futex.c b/LibOS/shim/src/sys/shim_futex.c index fb16a2a8..b975cbb2 100644 --- a/LibOS/shim/src/sys/shim_futex.c +++ b/LibOS/shim/src/sys/shim_futex.c @@ -853,7 +853,7 @@ static bool handle_futex_death(uint32_t* uaddr) { uint32_t new_val = (val & FUTEX_WAITERS) | FUTEX_OWNER_DIED; if (__atomic_compare_exchange_n(uaddr, &val, new_val, - /*weak=*/true, __ATOMIC_RELAXED, __ATOMIC_RELAXED)) { + /*weak=*/false, __ATOMIC_RELAXED, __ATOMIC_RELAXED)) { /* Successfully set the new value, end the loop. */ break; } diff --git a/Pal/include/lib/spinlock.h b/Pal/include/lib/spinlock.h index 67b2ff55..8c731a68 100644 --- a/Pal/include/lib/spinlock.h +++ b/Pal/include/lib/spinlock.h @@ -108,7 +108,7 @@ static inline void spinlock_lock(spinlock_t* lock) { /* Seen lock as free, check if it still is, this time with acquire semantics (but only * if we really take it). */ val = SPINLOCK_UNLOCKED; - } while (!__atomic_compare_exchange_n(&lock->lock, &val, SPINLOCK_LOCKED, /*weak=*/true, + } while (!__atomic_compare_exchange_n(&lock->lock, &val, SPINLOCK_LOCKED, /*weak=*/false, __ATOMIC_ACQUIRE, __ATOMIC_RELAXED)); out: @@ -141,7 +141,7 @@ static inline int spinlock_lock_timeout(spinlock_t* lock, unsigned long iteratio /* Seen lock as free, check if it still is, this time with acquire semantics (but only * if we really take it). */ val = SPINLOCK_UNLOCKED; - } while (!__atomic_compare_exchange_n(&lock->lock, &val, SPINLOCK_LOCKED, /*weak=*/true, + } while (!__atomic_compare_exchange_n(&lock->lock, &val, SPINLOCK_LOCKED, /*weak=*/false, __ATOMIC_ACQUIRE, __ATOMIC_RELAXED)); out_success: diff --git a/Pal/src/host/Linux-SGX/db_events.c b/Pal/src/host/Linux-SGX/db_events.c index 2b9dbc2a..e65397e6 100644 --- a/Pal/src/host/Linux-SGX/db_events.c +++ b/Pal/src/host/Linux-SGX/db_events.c @@ -42,7 +42,7 @@ int _DkEventSet(PAL_HANDLE event, int wakeup) { if (event->event.isnotification) { // Leave it signaled, wake all uint32_t t = 0; - if (__atomic_compare_exchange_n(event->event.signaled, &t, 1, /*weak=*/true, + if (__atomic_compare_exchange_n(event->event.signaled, &t, 1, /*weak=*/false, __ATOMIC_ACQUIRE, __ATOMIC_RELAXED)) { int nwaiters = __atomic_load_n(&event->event.nwaiters.counter, __ATOMIC_SEQ_CST); if (nwaiters) { diff --git a/Pal/src/host/Linux-SGX/db_mutex.c b/Pal/src/host/Linux-SGX/db_mutex.c index cdcf1136..2d244520 100644 --- a/Pal/src/host/Linux-SGX/db_mutex.c +++ b/Pal/src/host/Linux-SGX/db_mutex.c @@ -48,7 +48,7 @@ int _DkMutexLockTimeout(struct mutex_handle* m, int64_t timeout_us) { int ret = 0; uint32_t t = MUTEX_UNLOCKED; - if (__atomic_compare_exchange_n(m->locked, &t, MUTEX_LOCKED, /*weak=*/true, + if (__atomic_compare_exchange_n(m->locked, &t, MUTEX_LOCKED, /*weak=*/false, __ATOMIC_ACQUIRE, __ATOMIC_RELAXED)) goto success; @@ -62,7 +62,7 @@ int _DkMutexLockTimeout(struct mutex_handle* m, int64_t timeout_us) { while (true) { uint32_t t = MUTEX_UNLOCKED; - if (__atomic_compare_exchange_n(m->locked, &t, MUTEX_LOCKED, /*weak=*/true, + if (__atomic_compare_exchange_n(m->locked, &t, MUTEX_LOCKED, /*weak=*/false, __ATOMIC_ACQUIRE, __ATOMIC_RELAXED)) break; /* diff --git a/Pal/src/host/Linux-SGX/enclave_ocalls.c b/Pal/src/host/Linux-SGX/enclave_ocalls.c index 85080dda..478987e2 100644 --- a/Pal/src/host/Linux-SGX/enclave_ocalls.c +++ b/Pal/src/host/Linux-SGX/enclave_ocalls.c @@ -215,7 +215,8 @@ static int ocall_mmap_untrusted_cache(uint64_t size, void** mem, bool* need_munm *need_munmap = false; struct untrusted_area* cache = &get_tcb_trts()->untrusted_area_cache; uint64_t in_use = 0; - if (!__atomic_compare_exchange_n(&cache->in_use, &in_use, 1, false, __ATOMIC_RELAXED, __ATOMIC_RELAXED)) { + if (!__atomic_compare_exchange_n(&cache->in_use, &in_use, 1, /*weak=*/false, + __ATOMIC_RELAXED, __ATOMIC_RELAXED)) { /* AEX signal handling case: cache is in use, so make explicit mmap/munmap */ int retval = ocall_mmap_untrusted(-1, 0, size, PROT_READ | PROT_WRITE, mem); if (IS_ERR(retval)) { diff --git a/Pal/src/host/Linux/db_events.c b/Pal/src/host/Linux/db_events.c index 16f634f1..9bffd6e0 100644 --- a/Pal/src/host/Linux/db_events.c +++ b/Pal/src/host/Linux/db_events.c @@ -37,7 +37,7 @@ int _DkEventSet(PAL_HANDLE event, int wakeup) { if (event->event.isnotification) { // Leave it signaled, wake all uint32_t t = 0; - if (__atomic_compare_exchange_n(&event->event.signaled, &t, 1, /*weak=*/true, + if (__atomic_compare_exchange_n(&event->event.signaled, &t, 1, /*weak=*/false, __ATOMIC_ACQUIRE, __ATOMIC_RELAXED)) { int nwaiters = __atomic_load_n(&event->event.nwaiters.counter, __ATOMIC_SEQ_CST); if (nwaiters) { diff --git a/Pal/src/host/Linux/db_mutex.c b/Pal/src/host/Linux/db_mutex.c index 776cb19e..42fc2ba3 100644 --- a/Pal/src/host/Linux/db_mutex.c +++ b/Pal/src/host/Linux/db_mutex.c @@ -97,7 +97,7 @@ int _DkMutexLockTimeout(struct mutex_handle* m, int64_t timeout_us) { while (true) { uint32_t t = MUTEX_UNLOCKED; - if (__atomic_compare_exchange_n(&m->locked, &t, MUTEX_LOCKED, /*weak=*/true, + if (__atomic_compare_exchange_n(&m->locked, &t, MUTEX_LOCKED, /*weak=*/false, __ATOMIC_ACQUIRE, __ATOMIC_RELAXED)) break;