Bug #22477
closedrb_ec_clear_vm_stack() can still clear the VM stack before ec->cfp = NULL on aarch64
Description
The fix for #21441 (e3ec101cc2) sets ec->cfp = NULL before rb_ec_set_vm_stack(ec, NULL, 0) in rb_ec_clear_vm_stack().
That way, thread_profile_frames() called from a signal handler returns early instead of walking a stack that is no longer set on the ec.
However, these are plain stores to distinct fields of the same struct, and rb_ec_set_vm_stack() is inlined, so the compiler is free to reorder them.
On aarch64, clang and GCC both merge the two stack stores into a single stp and emit it before the store to ec->cfp.
Here is rb_ec_clear_vm_stack() on master (vm_stack is at offset 0, vm_stack_size at 0x8 and cfp at 0x10):
stp xzr, xzr, [x0] ; ec->vm_stack = NULL, vm_stack_size = 0
str xzr, [x0, #0x10] ; ec->cfp = NULL
ret
This was observed with Apple clang 21 and GCC 16 on macOS arm64, and with clang 18 and GCC 13 on Linux aarch64. GCC 13 on x86_64 happens to emit the ec->cfp store first.
Reproduction¶
I stopped a terminating thread in lldb right after the stp, then called rb_profile_frames(), which is what a signal landing on that instruction would do:
$ lldb -- ./ruby --disable-gems -e 'Thread.new { 1 }.join'
(lldb) breakpoint set -n rb_threadptr_root_fiber_terminate -o true
(lldb) run
(lldb) breakpoint set -a rb_ec_clear_vm_stack+4 -o true
(lldb) continue
(lldb) p ((rb_execution_context_t *)$x0)->vm_stack
(VALUE *) 0x0000000000000000
(lldb) p ((rb_execution_context_t *)$x0)->cfp
(rb_control_frame_t *) 0x0000000828a4bfc8
(lldb) p (rb_execution_context_t *)$x0 == (rb_execution_context_t *)rb_current_ec_noinline()
(bool) true
(lldb) expr -- VALUE b[256]; int l[256]; (int)rb_profile_frames(0, 256, b, l)
error: Expression execution was interrupted: EXC_BAD_ACCESS (code=1, address=0x0).
It crashes in thread_profile_frames() (vm_backtrace.c:2137). Since ec->cfp is still set, the function does not return early.
end_cfp is then computed from the NULL ec->vm_stack, so the loop never reaches it and walks past the control frames.
Fix¶
I'll make a PR for master and backport PRs.
Add a compiler signal fence, as vm_push_frame() already does for the same reason:
ec->cfp = NULL;
+
+ // Ensure the store to ec->cfp above is not reordered after the stores below.
+#if defined HAVE_DECL_ATOMIC_SIGNAL_FENCE && HAVE_DECL_ATOMIC_SIGNAL_FENCE
+ atomic_signal_fence(memory_order_seq_cst);
+#endif
+
rb_ec_set_vm_stack(ec, NULL, 0);
With the fence, all the compilers above emit the store to ec->cfp no later than the stores clearing the stack, and the rest of vm.o is unchanged.
Under the same lldb steps, ec->cfp is already NULL at rb_ec_clear_vm_stack+4, and rb_profile_frames() returns 0.