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

Author: yamt

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 += \

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

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;

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