Fix pthreads normal-mutex debug implementation

AUTHORS

@@ -602,3 +602,4 @@ a license to everyone to use it as detailed in LICENSE.)
 * Christian Lloyd <clloyd@teladochealth.com> (copyright owned by Teladoc Health, Inc.)
 * Sean Morris <sean@seanmorr.is>
 * Mitchell Wills <mwills@google.com> (copyright owned by Google, Inc.)
+* Eugene Hopkinson <slowriot@armchair.software>

system/lib/libc/musl/src/internal/pthread_impl.h

@@ -165,6 +165,46 @@ enum {
 #define _b_waiters2 __u.__vi[4]
 #define _b_inst __u.__p[3]
 
+#if defined(__EMSCRIPTEN__) && !defined(NDEBUG)
+extern _Thread_local pthread_mutex_t* __emscripten_debug_normal_mutex_list;
+
+static inline void __emscripten_debug_normal_mutex_note_locked(pthread_mutex_t *m)
+{
+	m->_m_prev = 0;
+	m->_m_next = __emscripten_debug_normal_mutex_list;
+	if (__emscripten_debug_normal_mutex_list) {
+		__emscripten_debug_normal_mutex_list->_m_prev = m;
+	}
+	__emscripten_debug_normal_mutex_list = m;
+}
+
+static inline void __emscripten_debug_normal_mutex_note_unlocked(pthread_mutex_t *m)
+{
+	pthread_mutex_t *prev = (pthread_mutex_t*)m->_m_prev;
+	pthread_mutex_t *next = (pthread_mutex_t*)m->_m_next;
+	if (prev) {
+		prev->_m_next = next;
+	} else {
+		__emscripten_debug_normal_mutex_list = next;
+	}
+	if (next) {
+		next->_m_prev = prev;
+	}
+	m->_m_prev = 0;
+	m->_m_next = 0;
+}
+
+static inline int __emscripten_debug_normal_mutex_owned(pthread_mutex_t *m)
+{
+	for (pthread_mutex_t *it = __emscripten_debug_normal_mutex_list; it; it = (pthread_mutex_t*)it->_m_next) {
+		if (it == m) {
+			return 1;
+		}
+	}
+	return 0;
+}
+#endif
+
 #ifndef TP_OFFSET
 #define TP_OFFSET 0
 #endif

system/lib/libc/musl/src/thread/pthread_mutex_timedlock.c

@@ -86,13 +86,22 @@ int __pthread_mutex_timedlock(pthread_mutex_t *restrict m, const struct timespec
 		int own = r & 0x3fffffff;
 		if (!own && (!r || (type&4)))
 			continue;
+#if defined(__EMSCRIPTEN__) && !defined(NDEBUG)
+		// Extra check for deadlock in debug builds, but only if we would block
+		// forever (at == NULL). For normal mutexes in debug Emscripten builds,
+		// _m_lock preserves the historical 0/EBUSY encoding, so ownership is
+		// tracked separately in a per-thread list of held normal mutexes.
+		assert(at || (type & 15) != PTHREAD_MUTEX_NORMAL ||
+		              !__emscripten_debug_normal_mutex_owned(m) &&
+		              "pthread mutex deadlock detected");
+#endif
 		if ((type&3) == PTHREAD_MUTEX_ERRORCHECK
 		    && own == __pthread_self()->tid)
 			return EDEADLK;
 #if defined(__EMSCRIPTEN__) && !defined(NDEBUG)
-		// Extra check for deadlock in debug builds, but only if we would block
-		// forever (at == NULL).
-		assert(at || own != __pthread_self()->tid && "pthread mutex deadlock detected");
+		assert(at || (type & 15) == PTHREAD_MUTEX_NORMAL ||
+		              own != __pthread_self()->tid &&
+		              "pthread mutex deadlock detected");
 #endif
 
 		a_inc(&m->_m_waiters);

system/lib/libc/musl/src/thread/pthread_mutex_trylock.c

@@ -1,5 +1,9 @@
 #include "pthread_impl.h"
 
+#if defined(__EMSCRIPTEN__) && !defined(NDEBUG)
+_Thread_local pthread_mutex_t* __emscripten_debug_normal_mutex_list;
+#endif
+
 int __pthread_mutex_trylock_owner(pthread_mutex_t *m)
 {
 	int old, own;
@@ -35,7 +39,15 @@ int __pthread_mutex_trylock_owner(pthread_mutex_t *m)
 		if (m->_m_waiters) tid |= 0x80000000;
 		self->robust_list.pending = &m->_m_next;
 	}
+#if defined(__EMSCRIPTEN__) && !defined(NDEBUG)
+	if ((type & 15) == PTHREAD_MUTEX_NORMAL) {
+		tid = EBUSY;
+	} else {
+		tid |= old & 0x40000000;
+	}
+#else
 	tid |= old & 0x40000000;
+#endif
 
 	if (a_cas(&m->_m_lock, old, tid) != old) {
 		self->robust_list.pending = 0;
@@ -53,7 +65,16 @@ int __pthread_mutex_trylock_owner(pthread_mutex_t *m)
 	}
 #endif
 
-#if defined(__EMSCRIPTEN__) || !defined(NDEBUG)
+#if defined(__EMSCRIPTEN__) && !defined(NDEBUG)
+	// In debug Emscripten builds, keep normal mutexes encoded the same way as
+	// the fast path (0/EBUSY) so internal users such as dlmalloc still see the
+	// historical lock-word semantics, and track ownership separately in a
+	// per-thread list for the deadlock assertion in pthread_mutex_timedlock.
+	if ((type & 15) == PTHREAD_MUTEX_NORMAL) {
+		__emscripten_debug_normal_mutex_note_locked(m);
+		return 0;
+	}
+#elif defined(__EMSCRIPTEN__) || !defined(NDEBUG)
 	// We can get here for normal mutexes too, but only in debug builds
 	// (where we track ownership purely for debug purposes).
 	if ((type & 15) == PTHREAD_MUTEX_NORMAL) return 0;

system/lib/libc/musl/src/thread/pthread_mutex_unlock.c

@@ -31,6 +31,11 @@ int __pthread_mutex_unlock(pthread_mutex_t *m)
 			((char *)next - sizeof(void *)) = prev;
 	}
 #ifdef __EMSCRIPTEN__
+#if !defined(NDEBUG)
+	if (type == PTHREAD_MUTEX_NORMAL) {
+		__emscripten_debug_normal_mutex_note_unlocked(m);
+	}
+#endif
 	cont = a_swap(&m->_m_lock, new);
 #else
 	if (type&8) {

test/test_browser.py

@@ -5243,6 +5243,16 @@ def test_wasm_worker_proxied_function(self):
     # Test that code does not crash in ASSERTIONS-disabled builds
     self.btest('wasm_worker/proxied_function.c', expected='0', cflags=['--js-library', test_file('wasm_worker/proxied_function.js'), '-sWASM_WORKERS', '-sASSERTIONS=0'])
 
+  def test_wasm_worker_pthread_mutex_debug_allocator_regression(self):
+    self.set_setting('ASSERTIONS')
+    self.btest('wasm_worker/pthread_mutex_debug_allocator_regression.cpp',
+               expected='0', cflags=['-pthread', '-sWASM_WORKERS'])
+
+  def test_wasm_worker_pthread_mutex_debug_reporting_teardown(self):
+    self.set_setting('ASSERTIONS')
+    self.btest('wasm_worker/pthread_mutex_debug_reporting_teardown.cpp',
+               expected='0', cflags=['-pthread', '-sWASM_WORKERS'])
+
   @no_firefox('no 4GB support yet')
   @no_2gb('uses MAXIMUM_MEMORY')
   @no_4gb('uses MAXIMUM_MEMORY')

test/wasm_worker/pthread_mutex_debug_allocator_regression.cpp

@@ -0,0 +1,25 @@
+#include <cstdint>
+#include <emscripten.h>
+#include <emscripten/wasm_worker.h>
+
+static void worker_loop() {
+  for (;;) {
+    delete new std::uint8_t{0};
+  }
+}
+
+static void main_loop() {
+  static unsigned ticks;
+  static bool reported;
+  new std::uint8_t{0};
+  if (!reported && ++ticks == 120) {
+    reported = true;
+    REPORT_RESULT(0);
+  }
+}
+
+int main() {
+  emscripten_wasm_worker_post_function_v(emscripten_malloc_wasm_worker(1024 * 1024), worker_loop);
+  emscripten_set_main_loop(main_loop, 0, false);
+  return 0;
+}

test/wasm_worker/pthread_mutex_debug_reporting_teardown.cpp

@@ -0,0 +1,25 @@
+#include <cstdint>
+#include <emscripten.h>
+#include <emscripten/wasm_worker.h>
+
+static void worker_loop() {
+  for (;;) {
+    delete new std::uint8_t{0};
+  }
+}
+
+static void main_loop() {
+  static unsigned ticks;
+  static bool reported;
+  new std::uint8_t{0};
+  if (!reported && ++ticks == 30) {
+    reported = true;
+    REPORT_RESULT(0);
+  }
+}
+
+int main() {
+  emscripten_wasm_worker_post_function_v(emscripten_malloc_wasm_worker(1024 * 1024), worker_loop);
+  emscripten_set_main_loop(main_loop, 0, false);
+  return 0;
+}