From 53d7027de075073d3bc1ce7b89a4b5ef94ca7caf Mon Sep 17 00:00:00 2001 From: Marcin Kolny Date: Thu, 31 Aug 2023 13:23:54 +0100 Subject: [PATCH] Implement strict validation of thread IDs according to the specification (#2521) --- .../lib-wasi-threads/test/create_threads_until_limit.c | 2 +- .../libraries/lib-wasi-threads/test/global_atomic.c | 2 +- .../libraries/lib-wasi-threads/test/global_lock.c | 2 +- .../test/trap_after_main_thread_finishes.c | 2 +- .../test/update_shared_data_and_alloc_heap.c | 2 +- samples/wasi-threads/wasm-apps/no_pthread.c | 10 ++-------- samples/wasi-threads/wasm-apps/wasi_thread_start.h | 4 ++++ 7 files changed, 11 insertions(+), 13 deletions(-) diff --git a/core/iwasm/libraries/lib-wasi-threads/test/create_threads_until_limit.c b/core/iwasm/libraries/lib-wasi-threads/test/create_threads_until_limit.c index 23ba5f627..94ffa0f67 100644 --- a/core/iwasm/libraries/lib-wasi-threads/test/create_threads_until_limit.c +++ b/core/iwasm/libraries/lib-wasi-threads/test/create_threads_until_limit.c @@ -65,7 +65,7 @@ main(int argc, char **argv) assert(start_args_init(&data[i].base)); thread_ids[i] = __wasi_thread_spawn(&data[i]); printf("Thread created with id=%d\n", thread_ids[i]); - assert(thread_ids[i] > 0 && "Thread creation failed"); + ASSERT_VALID_TID(thread_ids[i]); for (int j = 0; j < i; j++) { assert(thread_ids[i] != thread_ids[j] && "Duplicated TIDs"); diff --git a/core/iwasm/libraries/lib-wasi-threads/test/global_atomic.c b/core/iwasm/libraries/lib-wasi-threads/test/global_atomic.c index a38e75364..7e1e8e081 100644 --- a/core/iwasm/libraries/lib-wasi-threads/test/global_atomic.c +++ b/core/iwasm/libraries/lib-wasi-threads/test/global_atomic.c @@ -49,7 +49,7 @@ main(int argc, char **argv) for (int i = 0; i < NUM_THREADS; i++) { assert(start_args_init(&data[i].base)); thread_ids[i] = __wasi_thread_spawn(&data[i]); - assert(thread_ids[i] > 0 && "Thread creation failed"); + ASSERT_VALID_TID(thread_ids[i]); } printf("Wait for threads to finish\n"); diff --git a/core/iwasm/libraries/lib-wasi-threads/test/global_lock.c b/core/iwasm/libraries/lib-wasi-threads/test/global_lock.c index f81fca49b..fb33802fc 100644 --- a/core/iwasm/libraries/lib-wasi-threads/test/global_lock.c +++ b/core/iwasm/libraries/lib-wasi-threads/test/global_lock.c @@ -61,7 +61,7 @@ main(int argc, char **argv) for (int i = 0; i < NUM_THREADS; i++) { assert(start_args_init(&data[i].base)); thread_ids[i] = __wasi_thread_spawn(&data[i]); - assert(thread_ids[i] > 0 && "Thread creation failed"); + ASSERT_VALID_TID(thread_ids[i]); } printf("Wait for threads to finish\n"); diff --git a/core/iwasm/libraries/lib-wasi-threads/test/trap_after_main_thread_finishes.c b/core/iwasm/libraries/lib-wasi-threads/test/trap_after_main_thread_finishes.c index 69e125d40..5cf61338f 100644 --- a/core/iwasm/libraries/lib-wasi-threads/test/trap_after_main_thread_finishes.c +++ b/core/iwasm/libraries/lib-wasi-threads/test/trap_after_main_thread_finishes.c @@ -38,7 +38,7 @@ main(int argc, char **argv) assert(start_args_init(&data.base)); int thread_id = __wasi_thread_spawn(&data); - assert(thread_id > 0 && "Thread creation failed"); + ASSERT_VALID_TID(thread_id); return EXIT_SUCCESS; } diff --git a/core/iwasm/libraries/lib-wasi-threads/test/update_shared_data_and_alloc_heap.c b/core/iwasm/libraries/lib-wasi-threads/test/update_shared_data_and_alloc_heap.c index b7fb9afba..d6e34539b 100644 --- a/core/iwasm/libraries/lib-wasi-threads/test/update_shared_data_and_alloc_heap.c +++ b/core/iwasm/libraries/lib-wasi-threads/test/update_shared_data_and_alloc_heap.c @@ -69,7 +69,7 @@ main(int argc, char **argv) data[i].iteration = i; thread_ids[i] = __wasi_thread_spawn(&data[i]); - assert(thread_ids[i] > 0 && "Thread creation failed"); + ASSERT_VALID_TID(thread_ids[i]); } printf("Wait for threads to finish\n"); diff --git a/samples/wasi-threads/wasm-apps/no_pthread.c b/samples/wasi-threads/wasm-apps/no_pthread.c index dc3c95530..16661bd83 100644 --- a/samples/wasi-threads/wasm-apps/no_pthread.c +++ b/samples/wasi-threads/wasm-apps/no_pthread.c @@ -50,16 +50,11 @@ main(int argc, char **argv) } thread_id = __wasi_thread_spawn(&data); - if (thread_id < 0) { - printf("Failed to create thread: %d\n", thread_id); - ret = EXIT_FAILURE; - goto final; - } + ASSERT_VALID_TID(thread_id); if (__builtin_wasm_memory_atomic_wait32(&data.th_ready, 0, SECOND) == 2) { printf("Timeout\n"); - ret = EXIT_FAILURE; - goto final; + return EXIT_FAILURE; } printf("Thread completed, new value: %d, thread id: %d\n", data.value, @@ -67,7 +62,6 @@ main(int argc, char **argv) assert(thread_id == data.thread_id); -final: start_args_deinit(&data.base); return ret; diff --git a/samples/wasi-threads/wasm-apps/wasi_thread_start.h b/samples/wasi-threads/wasm-apps/wasi_thread_start.h index a46917d0a..2427fd2b9 100644 --- a/samples/wasi-threads/wasm-apps/wasi_thread_start.h +++ b/samples/wasi-threads/wasm-apps/wasi_thread_start.h @@ -7,6 +7,10 @@ #define STACK_SIZE 32 * 1024 // same as the main stack +/* See https://github.com/WebAssembly/wasi-threads#design-choice-thread-ids */ +#define ASSERT_VALID_TID(TID) \ + assert(TID >= 1 && TID <= 0x1FFFFFFF && "Invalid thread ID") + typedef struct { void *stack; } start_args_t;