From 396e94666ba1646cb0fd1459eeae3f2e7ddd2658 Mon Sep 17 00:00:00 2001 From: KJ Tsanaktsidis Date: Fri, 12 Jan 2024 17:32:16 +1100 Subject: [PATCH] Revert "Pass down "stack start" variables from closer to the top of the stack" This reverts commit 4ba8f0dc993953d3ddda6328e3ef17a2fc2cbde5. --- eval.c | 4 ++-- include/ruby/internal/interpreter.h | 2 +- internal/inits.h | 2 +- thread.c | 4 ++-- thread_none.c | 4 ++-- thread_pthread.c | 21 +++++---------------- thread_win32.c | 12 ++++++------ vm.c | 4 ++-- vm_core.h | 2 +- 9 files changed, 22 insertions(+), 33 deletions(-) diff --git a/eval.c b/eval.c index 6424aed1dc..aa0eae0872 100644 --- a/eval.c +++ b/eval.c @@ -70,7 +70,7 @@ ruby_setup(void) if (GET_VM()) return 0; - ruby_init_stack(&state); + ruby_init_stack((void *)&state); /* * Disable THP early before mallocs happen because we want this to @@ -79,7 +79,7 @@ ruby_setup(void) #if defined(__linux__) && defined(PR_SET_THP_DISABLE) prctl(PR_SET_THP_DISABLE, 1, 0, 0, 0); #endif - Init_BareVM(&state); + Init_BareVM(); Init_heap(); rb_vm_encoded_insn_data_table_init(); Init_vm_objects(); diff --git a/include/ruby/internal/interpreter.h b/include/ruby/internal/interpreter.h index d36d61ba54..662d39c0ec 100644 --- a/include/ruby/internal/interpreter.h +++ b/include/ruby/internal/interpreter.h @@ -141,7 +141,7 @@ void ruby_show_copyright(void); * * @param[in] addr A pointer somewhere on the stack, near its bottom. */ -void ruby_init_stack(volatile void *addr); +void ruby_init_stack(volatile VALUE *addr); /** * Initializes the VM and builtin libraries. diff --git a/internal/inits.h b/internal/inits.h index a2b2eea96f..03e180f77b 100644 --- a/internal/inits.h +++ b/internal/inits.h @@ -29,7 +29,7 @@ int Init_enc_set_filesystem_encoding(void); void Init_newline(void); /* vm.c */ -void Init_BareVM(void *local_in_parent_frame); +void Init_BareVM(void); void Init_vm_objects(void); /* vm_backtrace.c */ diff --git a/thread.c b/thread.c index e7a9d7cd9b..070d1bbdfa 100644 --- a/thread.c +++ b/thread.c @@ -522,9 +522,9 @@ static VALUE rb_threadptr_raise(rb_thread_t *, int, VALUE *); static VALUE rb_thread_to_s(VALUE thread); void -ruby_thread_init_stack(rb_thread_t *th, void *local_in_parent_frame) +ruby_thread_init_stack(rb_thread_t *th) { - native_thread_init_stack(th, local_in_parent_frame); + native_thread_init_stack(th); } const VALUE * diff --git a/thread_none.c b/thread_none.c index 767b929cf9..4d53d3bf4d 100644 --- a/thread_none.c +++ b/thread_none.c @@ -140,12 +140,12 @@ ruby_mn_threads_params(void) } void -ruby_init_stack(volatile void *addr) +ruby_init_stack(volatile VALUE *addr) { } static int -native_thread_init_stack(rb_thread_t *th, void *local_in_parent_frame) +native_thread_init_stack(rb_thread_t *th) { #if defined(__wasm__) && !defined(__EMSCRIPTEN__) th->ec->machine.stack_start = (VALUE *)rb_wasm_stack_get_base(); diff --git a/thread_pthread.c b/thread_pthread.c index a6a6c9d127..6d2f55a957 100644 --- a/thread_pthread.c +++ b/thread_pthread.c @@ -1964,7 +1964,7 @@ reserve_stack(volatile char *limit, size_t size) #undef ruby_init_stack void -ruby_init_stack(volatile void *addr) +ruby_init_stack(volatile VALUE *addr) { native_main_thread.id = pthread_self(); @@ -2049,7 +2049,7 @@ ruby_init_stack(volatile void *addr) {int err = (expr); if (err) {rb_bug_errno(#expr, err);}} static int -native_thread_init_stack(rb_thread_t *th, void *local_in_parent_frame) +native_thread_init_stack(rb_thread_t *th) { rb_nativethread_id_t curr = pthread_self(); @@ -2064,8 +2064,8 @@ native_thread_init_stack(rb_thread_t *th, void *local_in_parent_frame) size_t size; if (get_stack(&start, &size) == 0) { - uintptr_t diff = (uintptr_t)start - (uintptr_t)local_in_parent_frame; - th->ec->machine.stack_start = (uintptr_t)local_in_parent_frame; + uintptr_t diff = (uintptr_t)start - (uintptr_t)&curr; + th->ec->machine.stack_start = (VALUE *)&curr; th->ec->machine.stack_maxsize = size - diff; } } @@ -2185,19 +2185,8 @@ native_thread_create_dedicated(rb_thread_t *th) static void call_thread_start_func_2(rb_thread_t *th) { - /* Capture the address of a local in this stack frame to mark the beginning of the - machine stack for this thread. This is required even if we can tell the real - stack beginning from the pthread API in native_thread_init_stack, because - glibc stores some of its own data on the stack before calling into user code - on a new thread, and replacing that data on fiber-switch would break it (see - bug #13887) */ - VALUE stack_start = 0; - VALUE *stack_start_addr = &stack_start; - native_thread_init_stack(th, stack_start_addr); + native_thread_init_stack(th); thread_start_func_2(th, th->ec->machine.stack_start); - - /* Ensure that stack_start really was spilled to the stack */ - RB_GC_GUARD(stack_start) } static void * diff --git a/thread_win32.c b/thread_win32.c index e92472cea9..bd983e0bd9 100644 --- a/thread_win32.c +++ b/thread_win32.c @@ -582,7 +582,7 @@ rb_native_cond_destroy(rb_nativethread_cond_t *cond) } void -ruby_init_stack(volatile void *addr) +ruby_init_stack(volatile VALUE *addr) { } @@ -594,20 +594,20 @@ COMPILER_WARNING_PUSH COMPILER_WARNING_IGNORED(-Wmaybe-uninitialized) #endif static inline SIZE_T -query_memory_basic_info(PMEMORY_BASIC_INFORMATION mi, void *local_in_parent_frame) +query_memory_basic_info(PMEMORY_BASIC_INFORMATION mi) { - return VirtualQuery(local_in_parent_frame, mi, sizeof(*mi)); + return VirtualQuery(mi, mi, sizeof(*mi)); } COMPILER_WARNING_POP static void -native_thread_init_stack(rb_thread_t *th, void *local_in_parent_frame) +native_thread_init_stack(rb_thread_t *th) { MEMORY_BASIC_INFORMATION mi; char *base, *end; DWORD size, space; - CHECK_ERR(query_memory_basic_info(&mi, local_in_parent_frame)); + CHECK_ERR(query_memory_basic_info(&mi)); base = mi.AllocationBase; end = mi.BaseAddress; end += mi.RegionSize; @@ -638,7 +638,7 @@ thread_start_func_1(void *th_ptr) rb_thread_t *th = th_ptr; volatile HANDLE thread_id = th->nt->thread_id; - native_thread_init_stack(th, &th); + native_thread_init_stack(th); th->nt->interrupt_event = CreateEvent(0, TRUE, FALSE, 0); /* run */ diff --git a/vm.c b/vm.c index 945e7b91f7..37d631116a 100644 --- a/vm.c +++ b/vm.c @@ -4174,7 +4174,7 @@ rb_vm_set_progname(VALUE filename) extern const struct st_hash_type rb_fstring_hash_type; void -Init_BareVM(void *local_in_parent_frame) +Init_BareVM(void) { /* VM bootstrap: phase 1 */ rb_vm_t * vm = ruby_mimmalloc(sizeof(*vm)); @@ -4204,7 +4204,7 @@ Init_BareVM(void *local_in_parent_frame) th_init(th, 0, vm); rb_ractor_set_current_ec(th->ractor, th->ec); - ruby_thread_init_stack(th, local_in_parent_frame); + ruby_thread_init_stack(th); // setup ractor system rb_native_mutex_initialize(&vm->ractor.sync.lock); diff --git a/vm_core.h b/vm_core.h index 91cfae77e9..354603514e 100644 --- a/vm_core.h +++ b/vm_core.h @@ -1842,7 +1842,7 @@ rb_control_frame_t *rb_vm_get_binding_creatable_next_cfp(const rb_execution_cont VALUE *rb_vm_svar_lep(const rb_execution_context_t *ec, const rb_control_frame_t *cfp); int rb_vm_get_sourceline(const rb_control_frame_t *); void rb_vm_stack_to_heap(rb_execution_context_t *ec); -void ruby_thread_init_stack(rb_thread_t *th, void *local_in_parent_frame); +void ruby_thread_init_stack(rb_thread_t *th); rb_thread_t * ruby_thread_from_native(void); int ruby_thread_set_native(rb_thread_t *th); int rb_vm_control_frame_id_and_class(const rb_control_frame_t *cfp, ID *idp, ID *called_idp, VALUE *klassp);