0.13.3 --- a detached thread ends and the program goes on (#32)

musl's __unmapself moved to a 256-byte stack shared by every exiting thread
before the two calls that end a detached one. Here the thread runs on the stack
kal_task_start supplied, and those calls pass through the port's dispatcher,
context table and openkal: an unoptimized build reached 13,640 bytes below the
shared stack. On arm64 macOS the overflow overwrote the thread-specific key
table and the context table, and the exit jumped through the bytes it had just
written. The port releases the mapping from the stack the thread is on;
examples/threads-detached runs on every CI row.

Carries openkal-windows 0.7.3: a channel's ends reach only the program they are
placed in (0.7.2), and a started program receives its arguments and directory
as given (0.7.3).

Author: Sunrisepeak

.github/workflows/ci.yml

@@ -606,6 +606,19 @@ jobs:
           bash tools/run-probe.sh examples/threads-cxx threads-cxx
           grep -q 'value 42, result is the argument' examples/threads-cxx/run.log
 
+      # ⭐ A DETACHED THREAD ENDS, AND THE PROGRAM THAT STARTED IT GOES ON.
+      #
+      # musl's `__unmapself` left for a 256-byte shared stack before the calls
+      # that end a detached thread; this port's path for those calls needs far
+      # more, and on macOS the overflow overwrote the context table the exit then
+      # read. musl/PATCHES.md, `src/thread/__unmapself.c`.
+      - name: A detached thread ends and the program goes on
+        env:
+          MCPP_TARGET: ${{ matrix.target }}
+        run: |
+          bash tools/run-probe.sh examples/threads-detached threads-detached
+          grep -q 'detached: 8 started, 8 ended' examples/threads-detached/run.log
+
       - name: A program may use the names the internal overlay defines
         env:
           MCPP_TARGET: ${{ matrix.target }}

README.md

@@ -41,6 +41,11 @@ consumer needs to answer it without asking.
 
 | this package | is carried by |
 | --- | --- |
+| 0.13.3 | `openkal-llvm-runtime = "0.9.4"` |
+| 0.13.2 | `openkal-llvm-runtime = "0.9.3"` |
+| 0.13.1 | `openkal-llvm-runtime = "0.9.2"` |
+| 0.13.0 | `openkal-llvm-runtime = "0.9.0"` |
+| 0.12.0 | `openkal-llvm-runtime = "0.8.0"` |
 | 0.11.0 | `openkal-llvm-runtime = "0.7.0"` |
 | 0.10.0 | `openkal-llvm-runtime = "0.6.0"` |
 | 0.9.0 | `openkal-llvm-runtime = "0.5.0"` |
@@ -76,8 +81,8 @@ architecture. Replacing that one header is the whole of the redirection; the
 
 `musl/PATCHES.md` lists the whole of what is not unmodified: **five patched
 lines**, all of one kind — a machine word carried through a variable declared
-`long`, which is not a machine word on one of the three targets — and **eleven
-replaced sources**. Five of the eleven are replaced for the same reason: each
+`long`, which is not a machine word on one of the three targets — and **twelve
+replaced sources**. Five of the twelve are replaced for the same reason: each
 reads the shape of one particular environment rather than asking a kernel for
 something.
 

examples/threads-detached/mcpp.toml

@@ -0,0 +1,13 @@
+[package]
+name    = "threads-detached"
+version = "0.1.0"
+
+[dependencies]
+openkal-musl = { path = "../.." }
+
+[targets.threads-detached]
+kind = "bin"
+main = "src/main.c"
+
+[build]
+cxx_runtime = "host-coupled"

examples/threads-detached/src/main.c

@@ -0,0 +1,99 @@
+/* A detached thread ends, and the program that started it goes on.
+ *
+ * musl releases a detached thread's mapping from inside the thread, and on Linux
+ * the thread is standing on that mapping --- so `__unmapself' first moves to a
+ * 256-byte stack every exiting thread shares, and makes the two system calls
+ * that end the thread from there. In this port neither call is a system call:
+ * each passes through the port's dispatcher, the context table and openkal,
+ * and an unoptimized build of that path needs several times 256 bytes. The
+ * overflow wrote over whatever the linker placed beneath the shared stack. On
+ * macOS that is musl's table of thread-specific keys and this port's context
+ * table, and the thread then looked up its own record in the table it had just
+ * overwritten and jumped into it: every program whose detached thread ended
+ * stopped with an access violation.
+ *
+ * ⭐ WHAT IS OBSERVED. Detached threads that end one after another, then the
+ * two tables beneath the shared stack in use: a key with a destructor, and a
+ * joinable thread's own error value, which is reached through the context table.
+ */
+#include <errno.h>
+#include <pthread.h>
+#include <stdio.h>
+#include <time.h>
+
+enum { DETACHED = 8 };
+
+static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER;
+static pthread_cond_t reported = PTHREAD_COND_INITIALIZER;
+static int started;
+static int ended;
+
+static void* detached(void* arg)
+{
+	(void)arg;
+	pthread_mutex_lock(&lock);
+	++ended;
+	pthread_cond_broadcast(&reported);
+	pthread_mutex_unlock(&lock);
+	return 0;
+}
+
+static int destroyed;
+static void destroy(void* value) { if (value == &destroyed) ++destroyed; }
+
+static pthread_key_t key;
+static void* keyed(void* arg)
+{
+	(void)arg;
+	errno = 0;
+	pthread_setspecific(key, &destroyed);
+	errno = 7;
+	return (void*)(long)errno;
+}
+
+static void pause_briefly(void)
+{
+	struct timespec ts = { 0, 50 * 1000 * 1000 };
+	nanosleep(&ts, 0);
+}
+
+int main(void)
+{
+	int failures = 0;
+
+	pthread_attr_t attr;
+	pthread_attr_init(&attr);
+	pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED);
+	for (int i = 0; i < DETACHED; ++i) {
+		pthread_t thread;
+		if (pthread_create(&thread, &attr, detached, 0) != 0) {
+			printf("FAIL: detached thread %d was not created\n", i);
+			++failures;
+			break;
+		}
+		++started;
+		pthread_mutex_lock(&lock);
+		while (ended < started) pthread_cond_wait(&reported, &lock);
+		pthread_mutex_unlock(&lock);
+		/* Reporting precedes ending; the pause lets the thread finish ending
+		 * before the next one starts, which is where the fault was. */
+		pause_briefly();
+	}
+	pthread_attr_destroy(&attr);
+	printf("detached: %d started, %d ended\n", started, ended);
+	if (ended != DETACHED) { printf("FAIL: %d of %d detached threads ended\n", ended, DETACHED); ++failures; }
+
+	if (pthread_key_create(&key, destroy) != 0) { puts("FAIL: pthread_key_create"); ++failures; }
+	pthread_t joinable;
+	void* result = 0;
+	if (pthread_create(&joinable, 0, keyed, 0) != 0 || pthread_join(joinable, &result) != 0) {
+		puts("FAIL: the joinable thread did not run");
+		++failures;
+	}
+	printf("joinable: its error value %ld, destructor ran %d time(s)\n", (long)result, destroyed);
+	if ((long)result != 7) { puts("FAIL: the joinable thread's error value"); ++failures; }
+	if (destroyed != 1) { puts("FAIL: the key's destructor"); ++failures; }
+
+	printf("-- failures: %d --\n", failures);
+	return failures == 0 ? 0 : 1;
+}

mcpp.toml

@@ -1,7 +1,7 @@
 [package]
 namespace   = "mcpplibs"
 name        = "openkal-musl"
-version     = "0.13.2"
+version     = "0.13.3"
 description = "musl 1.2.5 redirected onto openkal: one C library, ported once, above every implementation of the specification rather than above one kernel."
 license     = "Apache-2.0"
 
@@ -69,7 +69,7 @@ openkal-opensbi = { version = "0.6.0", features = ["standalone"] }
 defines = ["OKM_HAS_FS=0", "OKM_HAS_PROCESS=0", "OKM_HAS_TASK=0"]
 
 [target.'cfg(windows)'.dependencies]
-openkal-windows = { version = "0.7.0", features = ["standalone"] }
+openkal-windows = { version = "0.7.3", features = ["standalone"] }
 
 # The feature macros musl's own build establishes.
 #
@@ -115,6 +115,10 @@ sources = [
   "!musl/src/env/__init_tls.c",
   "!musl/src/thread/__set_thread_area.c",
   "!musl/src/thread/clone.c",
+  # Replaced in port/src/okm_thread.c: musl leaves for a 256-byte shared stack
+  # before the calls that end a detached thread, and this port's path for those
+  # calls needs far more --- it overwrote the context table on macOS.
+  "!musl/src/thread/__unmapself.c",
   "!musl/src/process/posix_spawn.c",
   # ⭐ AND ITS SIBLING, WHICH IS EXCLUDED BECAUSE THE ONE ABOVE IS.
   # musl's posix_spawnp does not search a PATH: it stores `__execvpe' in the

musl/PATCHES.md

@@ -93,9 +93,9 @@ carry it in a `long`.
 
 ## The sources this port replaces, and why each
 
-Eleven, and the list in the manifest carries the same reasons. Five read the shape
+Twelve, and the list in the manifest carries the same reasons. Five read the shape
 of one environment directly. Two carry a machine word through a variable
-declared `long`. Two more were found only by running the result. And one is
+declared `long`. Three more were found only by running the result. And one is
 replaced because another already was:
 
 `src/process/posix_spawnp.c` does not search a PATH. It stores `__execvpe` in
@@ -117,6 +117,18 @@ one in musl's own source, and having the two ways of searching for one program
 disagree with each other is worse for a caller than having both wrong the same
 way. Nothing on that target searches a PATH today: its CI row declares no shell.
 
+⚠️⚠️ `src/thread/__unmapself.c` moves to a 256-byte stack shared by every exiting
+thread before it makes the two calls that end a detached one. On Linux the thread
+stands on the mapping it is about to release, and two raw system calls fit in
+256 bytes. Here the thread stands on the stack `kal_task_start` supplied, and
+the calls pass through this port's dispatcher, its context table and openkal:
+an unoptimized build reached **13,640 bytes** below the shared stack, measured on
+x86_64 Linux. The overflow lands on whatever the linker placed beneath it. On
+arm64 macOS that is the table of thread-specific keys and then the context
+table, so the exiting thread read its own record out of the bytes it had just
+written and jumped through them. `port/src/okm_thread.c` releases the mapping
+from the stack the thread is on; `examples/threads-detached` is the probe.
+
 `src/mman/mmap.c` returns a pointer through a `long`. It is replaced rather than
 patched because the replacement is also better where a `long` does hold a
 pointer: the value never becomes an integer at all.

port/src/okm_thread.c

@@ -30,6 +30,7 @@
 #include <string.h>
 #include <time.h>
 #include "futex.h"
+#include "syscall.h"
 
 void      __okm_set_tp(uintptr_t value);
 void*     __okm_get_self(void);
@@ -153,6 +154,36 @@ syscall_arg_t __okm_task_exit(int code)
 	return 0;
 }
 
+/* ⚠️⚠️ A DETACHED CONTEXT RELEASES ITS MAPPING FROM WHERE IT STANDS, AND THE
+ * STACK MUSL MOVED TO FIRST WAS OVERRUN FIFTY TIMES OVER.
+ *
+ * A detached thread releases its own mapping as the last thing it does. On Linux
+ * the thread is standing on that mapping, so musl's `__unmapself' first moves to
+ * a 256-byte stack shared by every exiting thread and makes the two system calls
+ * that end the thread from there --- two instructions each, which is what the
+ * size was chosen for.
+ *
+ * Neither premise holds here. `__clone' above never runs a context on the stack
+ * musl allocated, because kal_task_start supplies its own, so the move protects
+ * nothing. And the two calls are this port's: each passes through the
+ * dispatcher, the context table and openkal. Measured in an unoptimized build for
+ * x86_64 Linux, that path reached 13,640 bytes below the shared stack, writing
+ * over whatever the linker had placed beneath it. What that is depends on the
+ * target. On arm64 macOS it is musl's table of thread-specific keys and then this
+ * port's context table, so the thread looked up its own record in the table it
+ * had just overwritten and jumped into it: every program whose detached thread
+ * ended stopped there with an access violation (examples/threads-detached).
+ *
+ * ⇒ The mapping is released from the stack the context is on, and the end is
+ * reached the ordinary way. What musl's version guaranteed --- that nothing runs
+ * upon the mapping once it is released --- holds without the move, because
+ * nothing here ever ran upon it. */
+void __unmapself(void* base, size_t size)
+{
+	__syscall(SYS_munmap, base, size);
+	__syscall(SYS_exit, 0);
+}
+
 /* --- the suspension primitive ------------------------------------------------ */
 
 syscall_arg_t __okm_futex(const int* addr, int op, int val, const struct timespec* t)

tools/cross-build-macos.sh

@@ -65,7 +65,7 @@ cd "$here"
 
 # Kept in step with mcpp.toml, INCLUDING that system's own exclusions:
 # okm_phdr.c answers dl_iterate_phdr from an ELF header and that format has none.
-skip='__libc_start_main|__init_tls|__set_thread_area|clone|posix_spawn|posix_spawnp|mmap|syscall_ret|getcwd|fcntl|dl_iterate_phdr|okm_phdr|cache'
+skip='__libc_start_main|__init_tls|__set_thread_area|__unmapself|clone|posix_spawn|posix_spawnp|mmap|syscall_ret|getcwd|fcntl|dl_iterate_phdr|okm_phdr|cache'
 for f in musl/src/*/*.c musl/src/malloc/mallocng/*.c port/src/*.c port/src/*.S; do
     base=$(basename "$f"); base=${base%.*}
     [[ "$base" =~ ^($skip)$ ]] && continue

tools/probe-cross-macos.sh

@@ -86,7 +86,7 @@ cd "$here"
 # SECOND statement of what mcpp.toml already states, and a second statement is
 # a thing that falls behind the first. It fell behind on the release that added
 # the tenth entry, and it is this job that said so.
-skip='__libc_start_main|__init_tls|__set_thread_area|clone|posix_spawn|posix_spawnp|mmap|syscall_ret|getcwd|fcntl|dl_iterate_phdr|okm_phdr|cache'
+skip='__libc_start_main|__init_tls|__set_thread_area|__unmapself|clone|posix_spawn|posix_spawnp|mmap|syscall_ret|getcwd|fcntl|dl_iterate_phdr|okm_phdr|cache'
 units=0
 for f in musl/src/*/*.c musl/src/malloc/mallocng/*.c port/src/*.c port/src/*.S; do
     base=$(basename "$f"); base=${base%.*}