diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ad2a0e5..294448e 100644 --- a/.github/workflows/ci.yml +++ b/.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 }} diff --git a/README.md b/README.md index db50e65..e73eae6 100644 --- a/README.md +++ b/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. diff --git a/examples/threads-detached/mcpp.toml b/examples/threads-detached/mcpp.toml new file mode 100644 index 0000000..9183586 --- /dev/null +++ b/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" diff --git a/examples/threads-detached/src/main.c b/examples/threads-detached/src/main.c new file mode 100644 index 0000000..121ed16 --- /dev/null +++ b/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 +#include +#include +#include + +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; +} diff --git a/mcpp.toml b/mcpp.toml index 19c362e..98c7a24 100644 --- a/mcpp.toml +++ b/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 diff --git a/musl/PATCHES.md b/musl/PATCHES.md index 15c637f..26a292b 100644 --- a/musl/PATCHES.md +++ b/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. diff --git a/port/src/okm_thread.c b/port/src/okm_thread.c index 6e0149c..39a0bf4 100644 --- a/port/src/okm_thread.c +++ b/port/src/okm_thread.c @@ -30,6 +30,7 @@ #include #include #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) diff --git a/tools/cross-build-macos.sh b/tools/cross-build-macos.sh index 97c32bf..f5e13cd 100755 --- a/tools/cross-build-macos.sh +++ b/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 diff --git a/tools/probe-cross-macos.sh b/tools/probe-cross-macos.sh index 3ed6ba6..3f4e98f 100755 --- a/tools/probe-cross-macos.sh +++ b/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%.*}