Fix nsync_mu_unlock_slow_() on Apple Silicon

We torture test dlmalloc() in test/libc/stdio/memory_test.c. That test was crashing on occasion on Apple M1 microprocessors when dlmalloc was using *NSYNC locks. It was relatively easy to spot the cause, which is this one particular compare and swap operation, which needed to change to use sequentially-consistent ordering rather than an acquire barrier
jart · Nov 13, 2023 · 751d20d · 751d20d
1 parent 3b15d31
commit 751d20d
Show file tree

Hide file tree

Showing 5 changed files with 36 additions and 2 deletions.
diff --git a/third_party/dlmalloc/dlmalloc.c b/third_party/dlmalloc/dlmalloc.c
@@ -23,6 +23,7 @@
 #include "libc/thread/thread.h"
 #include "libc/thread/tls.h"
 #include "third_party/dlmalloc/vespene.internal.h"
+#include "third_party/nsync/mu.h"
 // clang-format off
 
 #define FOOTERS 0

diff --git a/third_party/dlmalloc/locks.inc b/third_party/dlmalloc/locks.inc
@@ -30,6 +30,8 @@
 
 */
 
+#ifdef USE_SPIN_LOCKS
+
 #define MLOCK_T atomic_uint
 
 static int malloc_wipe(MLOCK_T *lk) {
@@ -51,6 +53,27 @@ static int malloc_unlock(MLOCK_T *lk) {
   return 0;
 }
 
+#else
+
+#define MLOCK_T nsync_mu
+
+static int malloc_wipe(MLOCK_T *lk) {
+  bzero(lk, sizeof(*lk));
+  return 0;
+}
+
+static int malloc_lock(MLOCK_T *lk) {
+  nsync_mu_lock(lk);
+  return 0;
+}
+
+static int malloc_unlock(MLOCK_T *lk) {
+  nsync_mu_unlock(lk);
+  return 0;
+}
+
+#endif
+
 #define ACQUIRE_LOCK(lk) malloc_lock(lk)
 #define RELEASE_LOCK(lk) malloc_unlock(lk)
 #define INITIAL_LOCK(lk) malloc_wipe(lk)

diff --git a/third_party/nsync/README.cosmo b/third_party/nsync/README.cosmo
@@ -15,6 +15,8 @@ ORIGIN
 
 LOCAL CHANGES
 
+  - Fix nsync_mu_unlock() on Apple Silicon
+
   - Time APIs were so good that they're now in libc
 
   - Double linked list API was so good that it's now in libc

diff --git a/third_party/nsync/atomic.internal.h b/third_party/nsync/atomic.internal.h
@@ -86,13 +86,21 @@ static inline int atm_cas_relacq_u32_(nsync_atomic_uint32_ *p, uint32_t o,
                                                  memory_order_relaxed);
 }
 
+static inline int atm_cas_seqcst_u32_(nsync_atomic_uint32_ *p, uint32_t o,
+                                      uint32_t n) {
+  return atomic_compare_exchange_strong_explicit(NSYNC_ATOMIC_UINT32_PTR_(p),
+                                                 &o, n, memory_order_seq_cst,
+                                                 memory_order_relaxed);
+}
+
 #define ATM_CAS_HELPER_(barrier, p, o, n) \
   (atm_cas_##barrier##_u32_((p), (o), (n)))
 
 #define ATM_CAS(p, o, n)        ATM_CAS_HELPER_(nomb, (p), (o), (n))
 #define ATM_CAS_ACQ(p, o, n)    ATM_CAS_HELPER_(acq, (p), (o), (n))
 #define ATM_CAS_REL(p, o, n)    ATM_CAS_HELPER_(rel, (p), (o), (n))
 #define ATM_CAS_RELACQ(p, o, n) ATM_CAS_HELPER_(relacq, (p), (o), (n))
+#define ATM_CAS_SEQCST(p, o, n) ATM_CAS_HELPER_(seqcst, (p), (o), (n))
 
 /* Need a cast to remove "const" from some uses. */
 #define ATM_LOAD(p)                                                          \

diff --git a/third_party/nsync/mu.c b/third_party/nsync/mu.c
@@ -298,8 +298,8 @@ void nsync_mu_unlock_slow_ (nsync_mu *mu, lock_type *l_type) {
 				return;
 			}
 		} else if ((old_word&MU_SPINLOCK) == 0 &&
-			   ATM_CAS_ACQ (&mu->word, old_word,
-					(old_word-early_release_mu)|MU_SPINLOCK|MU_DESIG_WAKER)) {
+			   ATM_CAS_SEQCST (&mu->word, old_word, /* [jart] fixes issues on apple silicon */
+                                           (old_word-early_release_mu)|MU_SPINLOCK|MU_DESIG_WAKER)) {
 			struct Dll *wake;
 			lock_type *wake_type;
 			uint32_t clear_on_release;