Implement the critical part of wasi_thread_start in asm (#376)

* Implement the critical part of wasi_thread_start in asm

It's fragile to set up the critical part of C environment in C.

* Specify --target for asm files as well

* wasi_thread_start: Move __tls_base initialization to asm as well

diff --git a/Makefile b/Makefile
index 674b7c11a36c2eada9e1713e82c154601f5c2338..46229f930af8a28fc5e033932072c713e158a967 100644 (file)
--- a/Makefile
+++ b/Makefile
@@ -269,6 +269,7 @@ LIBC_TOP_HALF_MUSL_SOURCES += \
         thread/sem_timedwait.c \
         thread/sem_trywait.c \
         thread/sem_wait.c \
+        thread/wasm32/wasi_thread_start.s \
     )
 endif
 
@@ -287,12 +288,13 @@ LIBC_TOP_HALF_HEADERS_PRIVATE = $(LIBC_TOP_HALF_DIR)/headers/private
 LIBC_TOP_HALF_SOURCES = $(LIBC_TOP_HALF_DIR)/sources
 LIBC_TOP_HALF_ALL_SOURCES = \
     $(LIBC_TOP_HALF_MUSL_SOURCES) \
-    $(sort $(shell find $(LIBC_TOP_HALF_SOURCES) -name \*.c))
+    $(sort $(shell find $(LIBC_TOP_HALF_SOURCES) -name \*.[cs]))
 
 # Add any extra flags
 CFLAGS = $(EXTRA_CFLAGS)
 # Set the target.
 CFLAGS += --target=$(TARGET_TRIPLE)
+ASMFLAGS += --target=$(TARGET_TRIPLE)
 # WebAssembly floating-point match doesn't trap.
 # TODO: Add -fno-signaling-nans when the compiler supports it.
 CFLAGS += -fno-trapping-math
@@ -339,10 +341,11 @@ CFLAGS += -isystem "$(SYSROOT_INC)"
 # These variables describe the locations of various files and directories in
 # the build tree.
 objs = $(patsubst $(CURDIR)/%.c,$(OBJDIR)/%.o,$(1))
+asmobjs = $(patsubst $(CURDIR)/%.s,$(OBJDIR)/%.o,$(1))
 DLMALLOC_OBJS = $(call objs,$(DLMALLOC_SOURCES))
 EMMALLOC_OBJS = $(call objs,$(EMMALLOC_SOURCES))
 LIBC_BOTTOM_HALF_ALL_OBJS = $(call objs,$(LIBC_BOTTOM_HALF_ALL_SOURCES))
-LIBC_TOP_HALF_ALL_OBJS = $(call objs,$(LIBC_TOP_HALF_ALL_SOURCES))
+LIBC_TOP_HALF_ALL_OBJS = $(call asmobjs,$(call objs,$(LIBC_TOP_HALF_ALL_SOURCES)))
 ifeq ($(MALLOC_IMPL),dlmalloc)
 LIBC_OBJS += $(DLMALLOC_OBJS)
 else ifeq ($(MALLOC_IMPL),emmalloc)
@@ -517,6 +520,10 @@ $(OBJDIR)/%.o: $(CURDIR)/%.c include_dirs
        @mkdir -p "$(@D)"
        $(CC) $(CFLAGS) -MD -MP -o $@ -c $<
 
+$(OBJDIR)/%.o: $(CURDIR)/%.s include_dirs
+       @mkdir -p "$(@D)"
+       $(CC) $(ASMFLAGS) -o $@ -c $<
+
 -include $(shell find $(OBJDIR) -name \*.d)
 
 $(DLMALLOC_OBJS): CFLAGS += \

diff --git a/expected/wasm32-wasi-pthread/defined-symbols.txt b/expected/wasm32-wasi-pthread/defined-symbols.txt
index a999a0803bacc68cecb40441b262271f740be7c4..080c80fc7c56d66545e0a38b7e17afbace363206 100644 (file)
--- a/expected/wasm32-wasi-pthread/defined-symbols.txt
+++ b/expected/wasm32-wasi-pthread/defined-symbols.txt
@@ -46,6 +46,7 @@ __do_cleanup_pop
 __do_cleanup_push
 __do_des
 __do_orphaned_stdio_locks
+__dummy_reference
 __duplocale
 __env_rm_add
 __errno_location
@@ -354,6 +355,7 @@ __wasi_sock_recv
 __wasi_sock_send
 __wasi_sock_shutdown
 __wasi_thread_spawn
+__wasi_thread_start_C
 __wasilibc_access
 __wasilibc_cwd
 __wasilibc_cwd_lock

diff --git a/libc-top-half/musl/src/thread/pthread_create.c b/libc-top-half/musl/src/thread/pthread_create.c
index 602d503d7d20770a324415fb502cd4ddce2c24dd..676e2ccf95d2a2b54398da61f83c1716df20de85 100644 (file)
--- a/libc-top-half/musl/src/thread/pthread_create.c
+++ b/libc-top-half/musl/src/thread/pthread_create.c
@@ -238,9 +238,14 @@ struct start_args {
        volatile int control;
        unsigned long sig_mask[_NSIG/8/sizeof(long)];
 #else
+       /*
+        * Note: the offset of the "stack" and "tls_base" members
+        * in this structure is hardcoded in wasi_thread_start.
+        */
+       void *stack;
+       void *tls_base;
        void *(*start_func)(void *);
        void *start_arg;
-       void *tls_base;
 #endif
 };
 
@@ -274,30 +279,25 @@ static int start_c11(void *p)
        return 0;
 }
 #else
-__attribute__((export_name("wasi_thread_start")))
-void wasi_thread_start(int tid, void *p)
+
+/*
+ * We want to ensure wasi_thread_start is linked whenever
+ * pthread_create is used. The following reference is to ensure that.
+ * Otherwise, the linker doesn't notice the dependency because
+ * wasi_thread_start is used indirectly via a wasm export.
+ */
+void wasi_thread_start(int tid, void *p);
+hidden void *__dummy_reference = wasi_thread_start;
+
+hidden void __wasi_thread_start_C(int tid, void *p)
 {
-       /*
-        * Note: it's fragile to implement wasi_thread_start in C.
-        * On entry, we don't even have C stack (__stack_pointer)
-        * set up. Be careful when modifying this function.
-        */
        struct start_args *args = p;
-       __asm__(".globaltype __tls_base, i32\n"
-                       "local.get %0\n"
-                       "global.set __tls_base\n"
-                       :: "r"(args->tls_base));
        pthread_t self = __pthread_self();
        // Set the thread ID (TID) on the pthread structure. The TID is stored
        // atomically since it is also stored by the parent thread; this way,
        // whichever thread (parent or child) reaches this point first can proceed
        // without waiting.
        atomic_store((atomic_int *) &(self->tid), tid);
-       // Set the stack pointer.
-       __asm__(".globaltype __stack_pointer, i32\n"
-                       "local.get %0\n"
-                       "global.set __stack_pointer\n"
-                       :: "r"(self->stack));
        // Execute the user's start function.
        __pthread_exit(args->start_func(args->start_arg));
 }
@@ -501,6 +501,7 @@ int __pthread_create(pthread_t *restrict res, const pthread_attr_t *restrict att
        /* Correct the stack size */
        new->stack_size = stack - stack_limit;
 
+       args->stack = new->stack; /* just for convenience of asm trampoline */
        args->start_func = entry;
        args->start_arg = arg;
        args->tls_base = (void*)new_tls_base;

diff --git a/libc-top-half/musl/src/thread/wasm32/wasi_thread_start.s b/libc-top-half/musl/src/thread/wasm32/wasi_thread_start.s
new file mode 100644 (file)
index 0000000..0fe9854
--- /dev/null
+++ b/libc-top-half/musl/src/thread/wasm32/wasi_thread_start.s
@@ -0,0 +1,31 @@
+       .text
+
+       .export_name    wasi_thread_start, wasi_thread_start
+
+       .globaltype     __stack_pointer, i32
+       .globaltype     __tls_base, i32
+       .functype       __wasi_thread_start_C (i32, i32) -> ()
+
+       .hidden wasi_thread_start
+       .globl  wasi_thread_start
+       .type   wasi_thread_start,@function
+
+wasi_thread_start:
+       .functype       wasi_thread_start (i32, i32) -> ()
+
+       # Set up the minimum C environment.
+       # Note: offsetof(start_arg, stack) == 0
+       local.get   1  # start_arg
+       i32.load    0  # stack
+       global.set  __stack_pointer
+
+       local.get   1  # start_arg
+       i32.load    4  # tls_base
+       global.set  __tls_base
+
+       # Make the C function do the rest of work.
+       local.get   0  # tid
+       local.get   1  # start_arg
+       call __wasi_thread_start_C
+
+       end_function