Skip to content

Commit 0e2382a

Browse files
loganekloganek
authored
Disable aux stack allocations for threads spawned by wasi_thread_start (bytecodealliance#1867)
This syscall doesn't need allocating stack or TLS and it's expected from the application to do that instead. E.g. WASI-libc already does this for `pthread_create`. Also fix some of the examples to allocate memory for stack and not use stack before the stack pointer is set to a correct value.
1 parent 2615646 commit 0e2382a

File tree

12 files changed

+159
-52
lines changed

12 files changed

+159
-52
lines changed

‎core/iwasm/common/wasm_exec_env.h‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,13 @@ wasm_exec_env_create(struct WASMModuleInstanceCommon *module_inst,
179179
void
180180
wasm_exec_env_destroy(WASMExecEnv *exec_env);
181181

182+
static inline bool
183+
wasm_exec_env_is_aux_stack_managed_by_runtime(WASMExecEnv *exec_env)
184+
{
185+
return exec_env->aux_stack_boundary.boundary != 0
186+
|| exec_env->aux_stack_bottom.bottom != 0;
187+
}
188+
182189
/**
183190
* Allocate a WASM frame from the WASM stack.
184191
*

‎core/iwasm/interpreter/wasm_interp_classic.c‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1783,14 +1783,18 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module,
17831783
global = globals + global_idx;
17841784
global_addr = get_global_addr(global_data, global);
17851785
aux_stack_top = *(uint32 *)(frame_sp - 1);
1786-
if (aux_stack_top <= exec_env->aux_stack_boundary.boundary) {
1787-
wasm_set_exception(module, "wasm auxiliary stack overflow");
1788-
goto got_exception;
1789-
}
1790-
if (aux_stack_top > exec_env->aux_stack_bottom.bottom) {
1791-
wasm_set_exception(module,
1792-
"wasm auxiliary stack underflow");
1793-
goto got_exception;
1786+
if (wasm_exec_env_is_aux_stack_managed_by_runtime(exec_env)) {
1787+
if (aux_stack_top
1788+
<= exec_env->aux_stack_boundary.boundary) {
1789+
wasm_set_exception(module,
1790+
"wasm auxiliary stack overflow");
1791+
goto got_exception;
1792+
}
1793+
if (aux_stack_top > exec_env->aux_stack_bottom.bottom) {
1794+
wasm_set_exception(module,
1795+
"wasm auxiliary stack underflow");
1796+
goto got_exception;
1797+
}
17941798
}
17951799
*(int32 *)global_addr = aux_stack_top;
17961800
frame_sp--;

‎core/iwasm/interpreter/wasm_interp_fast.c‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1576,14 +1576,18 @@ wasm_interp_call_func_bytecode(WASMModuleInstance *module,
15761576
global = globals + global_idx;
15771577
global_addr = get_global_addr(global_data, global);
15781578
aux_stack_top = frame_lp[GET_OFFSET()];
1579-
if (aux_stack_top <= exec_env->aux_stack_boundary.boundary) {
1580-
wasm_set_exception(module, "wasm auxiliary stack overflow");
1581-
goto got_exception;
1582-
}
1583-
if (aux_stack_top > exec_env->aux_stack_bottom.bottom) {
1584-
wasm_set_exception(module,
1585-
"wasm auxiliary stack underflow");
1586-
goto got_exception;
1579+
if (wasm_exec_env_is_aux_stack_managed_by_runtime(exec_env)) {
1580+
if (aux_stack_top
1581+
<= exec_env->aux_stack_boundary.boundary) {
1582+
wasm_set_exception(module,
1583+
"wasm auxiliary stack overflow");
1584+
goto got_exception;
1585+
}
1586+
if (aux_stack_top > exec_env->aux_stack_bottom.bottom) {
1587+
wasm_set_exception(module,
1588+
"wasm auxiliary stack underflow");
1589+
goto got_exception;
1590+
}
15871591
}
15881592
*(int32 *)global_addr = aux_stack_top;
15891593
#if WASM_ENABLE_MEMORY_PROFILING != 0

‎core/iwasm/libraries/lib-pthread/lib_pthread_wrapper.c‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -619,8 +619,9 @@ pthread_create_wrapper(wasm_exec_env_t exec_env,
619619
routine_args->module_inst = new_module_inst;
620620

621621
os_mutex_lock(&exec_env->wait_lock);
622-
ret = wasm_cluster_create_thread(
623-
exec_env, new_module_inst, pthread_start_routine, (void *)routine_args);
622+
ret =
623+
wasm_cluster_create_thread(exec_env, new_module_inst, true,
624+
pthread_start_routine, (void *)routine_args);
624625
if (ret != 0) {
625626
os_mutex_unlock(&exec_env->wait_lock);
626627
goto fail;

‎core/iwasm/libraries/lib-wasi-threads/lib_wasi_threads_wrapper.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,8 +126,8 @@ thread_spawn_wrapper(wasm_exec_env_t exec_env, uint32 start_arg)
126126
thread_start_arg->start_func = start_func;
127127

128128
os_mutex_lock(&exec_env->wait_lock);
129-
ret = wasm_cluster_create_thread(exec_env, new_module_inst, thread_start,
130-
thread_start_arg);
129+
ret = wasm_cluster_create_thread(exec_env, new_module_inst, false,
130+
thread_start, thread_start_arg);
131131
if (ret != 0) {
132132
LOG_ERROR("Failed to spawn a new thread");
133133
goto thread_spawn_fail;

‎core/iwasm/libraries/thread-mgr/thread_manager.c‎

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,10 @@ free_aux_stack(WASMExecEnv *exec_env, uint32 start)
125125
WASMModuleInstanceCommon *module_inst =
126126
wasm_exec_env_get_module_inst(exec_env);
127127

128+
if (!wasm_exec_env_is_aux_stack_managed_by_runtime(exec_env)) {
129+
return true;
130+
}
131+
128132
bh_assert(start >= cluster->stack_size);
129133

130134
wasm_runtime_module_free(module_inst, start - cluster->stack_size);
@@ -534,7 +538,7 @@ thread_manager_start_routine(void *arg)
534538

535539
int32
536540
wasm_cluster_create_thread(WASMExecEnv *exec_env,
537-
wasm_module_inst_t module_inst,
541+
wasm_module_inst_t module_inst, bool alloc_aux_stack,
538542
void *(*thread_routine)(void *), void *arg)
539543
{
540544
WASMCluster *cluster;
@@ -550,16 +554,18 @@ wasm_cluster_create_thread(WASMExecEnv *exec_env,
550554
if (!new_exec_env)
551555
return -1;
552556

553-
if (!allocate_aux_stack(exec_env, &aux_stack_start, &aux_stack_size)) {
554-
LOG_ERROR("thread manager error: "
555-
"failed to allocate aux stack space for new thread");
556-
goto fail1;
557-
}
557+
if (alloc_aux_stack) {
558+
if (!allocate_aux_stack(exec_env, &aux_stack_start, &aux_stack_size)) {
559+
LOG_ERROR("thread manager error: "
560+
"failed to allocate aux stack space for new thread");
561+
goto fail1;
562+
}
558563

559-
/* Set aux stack for current thread */
560-
if (!wasm_exec_env_set_aux_stack(new_exec_env, aux_stack_start,
561-
aux_stack_size)) {
562-
goto fail2;
564+
/* Set aux stack for current thread */
565+
if (!wasm_exec_env_set_aux_stack(new_exec_env, aux_stack_start,
566+
aux_stack_size)) {
567+
goto fail2;
568+
}
563569
}
564570

565571
if (!wasm_cluster_add_exec_env(cluster, new_exec_env))
@@ -581,7 +587,8 @@ wasm_cluster_create_thread(WASMExecEnv *exec_env,
581587
wasm_cluster_del_exec_env(cluster, new_exec_env);
582588
fail2:
583589
/* free the allocated aux stack space */
584-
free_aux_stack(exec_env, aux_stack_start);
590+
if (alloc_aux_stack)
591+
free_aux_stack(exec_env, aux_stack_start);
585592
fail1:
586593
wasm_exec_env_destroy(new_exec_env);
587594
return -1;

‎core/iwasm/libraries/thread-mgr/thread_manager.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ wasm_exec_env_get_cluster(WASMExecEnv *exec_env);
6464

6565
int32
6666
wasm_cluster_create_thread(WASMExecEnv *exec_env,
67-
wasm_module_inst_t module_inst,
67+
wasm_module_inst_t module_inst, bool alloc_aux_stack,
6868
void *(*thread_routine)(void *), void *arg);
6969

7070
int32

‎samples/wasi-threads/wasm-apps/CMakeLists.txt‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,13 @@ endif ()
1313

1414
set (CMAKE_SYSROOT "${WASI_SYSROOT}")
1515
set (CMAKE_C_COMPILER "${WASI_SDK_DIR}/bin/clang")
16+
set (CMAKE_ASM_COMPILER "${WASI_SDK_DIR}/bin/clang")
1617
set (CMAKE_C_COMPILER_TARGET "wasm32-wasi")
1718

1819
function (compile_sample SOURCE_FILE)
1920
get_filename_component (FILE_NAME ${SOURCE_FILE} NAME_WLE)
2021
set (WASM_MODULE ${FILE_NAME}.wasm)
21-
add_executable (${WASM_MODULE} ${SOURCE_FILE})
22+
add_executable (${WASM_MODULE} ${SOURCE_FILE} ${ARGN})
2223

2324
target_compile_options (${WASM_MODULE} PRIVATE
2425
-pthread -ftls-model=local-exec)
@@ -34,5 +35,5 @@ function (compile_sample SOURCE_FILE)
3435
)
3536
endfunction ()
3637

37-
compile_sample(no_pthread.c)
38-
compile_sample(exception_propagation.c)
38+
compile_sample(no_pthread.c wasi_thread_start.S)
39+
compile_sample(exception_propagation.c wasi_thread_start.S)

‎samples/wasi-threads/wasm-apps/exception_propagation.c‎

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,21 @@
99
#include <stdlib.h>
1010
#include <stdio.h>
1111
#include <assert.h>
12-
#include <wasi/api.h>
1312
#include <semaphore.h>
1413
#include <stdbool.h>
1514
#include <unistd.h>
1615

16+
#include "wasi_thread_start.h"
17+
1718
#define TIMEOUT_SECONDS 10
1819
#define NUM_THREADS 3
1920
static sem_t sem;
2021

22+
typedef struct {
23+
start_args_t base;
24+
bool throw_exception;
25+
} shared_t;
26+
2127
void
2228
run_long_task()
2329
{
@@ -26,12 +32,12 @@ run_long_task()
2632
sleep(1);
2733
}
2834

29-
__attribute__((export_name("wasi_thread_start"))) void
30-
wasi_thread_start(int thread_id, int *start_arg)
35+
void
36+
__wasi_thread_start_C(int thread_id, int *start_arg)
3137
{
32-
bool has_to_throw_exception = (bool)start_arg;
38+
shared_t *data = (shared_t *)start_arg;
3339

34-
if (has_to_throw_exception) {
40+
if (data->throw_exception) {
3541
// Wait for all other threads (including main thread) to be ready
3642
printf("Waiting before throwing exception\n");
3743
for (int i = 0; i < NUM_THREADS; i++)
@@ -52,26 +58,36 @@ wasi_thread_start(int thread_id, int *start_arg)
5258
int
5359
main(int argc, char **argv)
5460
{
55-
int thread_id = -1;
61+
int thread_id = -1, i;
62+
shared_t data[NUM_THREADS] = { 0 };
63+
5664
if (sem_init(&sem, 0, 0) != 0) {
5765
printf("Failed to init semaphore\n");
5866
return EXIT_FAILURE;
5967
}
6068

69+
for (i = 0; i < NUM_THREADS; i++) {
70+
// No graceful memory free to simplify the example
71+
if (!start_args_init(&data[i].base)) {
72+
printf("Failed to allocate thread's stack\n");
73+
return EXIT_FAILURE;
74+
}
75+
}
76+
6177
// Create a thread that throws an exception
62-
thread_id = __wasi_thread_spawn((void *)true);
78+
data[0].throw_exception = true;
79+
thread_id = __wasi_thread_spawn(&data[0]);
6380
if (thread_id < 0) {
6481
printf("Failed to create thread: %d\n", thread_id);
6582
return EXIT_FAILURE;
6683
}
67-
6884
// Create two additional threads to test exception propagation
69-
thread_id = __wasi_thread_spawn((void *)false);
85+
thread_id = __wasi_thread_spawn(&data[1]);
7086
if (thread_id < 0) {
7187
printf("Failed to create thread: %d\n", thread_id);
7288
return EXIT_FAILURE;
7389
}
74-
thread_id = __wasi_thread_spawn((void *)false);
90+
thread_id = __wasi_thread_spawn(&data[2]);
7591
if (thread_id < 0) {
7692
printf("Failed to create thread: %d\n", thread_id);
7793
return EXIT_FAILURE;

‎samples/wasi-threads/wasm-apps/no_pthread.c‎

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,18 +9,20 @@
99
#include <stdlib.h>
1010
#include <stdio.h>
1111
#include <assert.h>
12-
#include <wasi/api.h>
12+
13+
#include "wasi_thread_start.h"
1314

1415
static const int64_t SECOND = 1000 * 1000 * 1000;
1516

1617
typedef struct {
18+
start_args_t base;
1719
int th_ready;
1820
int value;
1921
int thread_id;
2022
} shared_t;
2123

22-
__attribute__((export_name("wasi_thread_start"))) void
23-
wasi_thread_start(int thread_id, int *start_arg)
24+
void
25+
__wasi_thread_start_C(int thread_id, int *start_arg)
2426
{
2527
shared_t *data = (shared_t *)start_arg;
2628

@@ -38,24 +40,35 @@ wasi_thread_start(int thread_id, int *start_arg)
3840
int
3941
main(int argc, char **argv)
4042
{
41-
shared_t data = { 0, 52, -1 };
43+
shared_t data = { { NULL }, 0, 52, -1 };
4244
int thread_id;
45+
int ret = EXIT_SUCCESS;
46+
47+
if (!start_args_init(&data.base)) {
48+
printf("Stack allocation for thread failed\n");
49+
return EXIT_FAILURE;
50+
}
4351

4452
thread_id = __wasi_thread_spawn(&data);
4553
if (thread_id < 0) {
4654
printf("Failed to create thread: %d\n", thread_id);
47-
return EXIT_FAILURE;
55+
ret = EXIT_FAILURE;
56+
goto final;
4857
}
4958

5059
if (__builtin_wasm_memory_atomic_wait32(&data.th_ready, 0, SECOND) == 2) {
5160
printf("Timeout\n");
52-
return EXIT_FAILURE;
61+
ret = EXIT_FAILURE;
62+
goto final;
5363
}
5464

5565
printf("Thread completed, new value: %d, thread id: %d\n", data.value,
5666
data.thread_id);
5767

5868
assert(thread_id == data.thread_id);
5969

60-
return EXIT_SUCCESS;
70+
final:
71+
start_args_deinit(&data.base);
72+
73+
return ret;
6174
}

0 commit comments

Comments
 (0)