From 17ad7330bd1c4a248fecc06be11910da84e978fd Mon Sep 17 00:00:00 2001 From: Georgiy Lebedev Date: Wed, 9 Feb 2022 09:12:09 +0300 Subject: [PATCH] coro: fix `coro_{init,startup}` unwind information MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fiber call-chains end at `coro_{init, startup}`, but unwinders don't stop there, trying to use `coro_{init, startup}` stack frame's return address (which points to some garbage) and, in turn, failing. A similar issue was experienced by seastar and julia (see JuliaLang/julia#23074 and scylladb/scylla#1909). In order to make unwinding stop at `coro_{init, startup}`'s stack frame we need to annotate it with CFI assembly: previously, annotation was provided only for GCC on x86_64 — also provide it for clang. For some reason unwinders ignore platform ABIs regarding ending of call-chains: instead of trying to follow platform ABIs, explicitly invalidate the topmost (`coro_{init, startup}`) stack frame information for both x86_64 and AARCH64. References: 1. glibc: * clone: https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/ sysv/linux/x86_64/clone.S;h=31ac12da0cc08a934d514fed1de9eba1cb3e8ec5;hb =ebbb8c9f64c3486603ef4ccee4dd2a5574e41039 * start: https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/x86_6 4/start.S;h=9edd17b60cd54ec9eef11c76ab02322dcb5d057a;hb=5b736bc9b55115e 67129e77db4de6cf193054cd2 2. seastar: * thread_context::main(): https://github.com/scylladb/seastar/blob/d27b f8b5a14e5b9e9c9df18fd1306489b651aa42/src/core/thread.cc#L278-L293 3. julia: * https://github.com/JuliaLang/julia/blob/2e2b1d2ad50fe12999cbde d0b5acd3f0a36ec8c5/src/julia_internal.h#L90-L106 4. android: * https://cs.android.com/android/platform/superproject/+/master:bionic/ libc/platform/bionic/macros.h;l=52-60;drc=2528dab7419a63f57fe20027886ba 7dd3857aba8 Needed for #4002 NO_DOC=bug fix NO_CHANGELOG=bug fix --- third_party/coro/coro.c | 26 ++++++++++++++++---------- third_party/coro/coro.h | 1 - 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/third_party/coro/coro.c b/third_party/coro/coro.c index ca490dc268a9d..1702489474391 100644 --- a/third_party/coro/coro.c +++ b/third_party/coro/coro.c @@ -103,9 +103,15 @@ coro_init (void) coro_transfer (new_coro, create_coro); -#if __GCC_HAVE_DWARF2_CFI_ASM && __amd64 - asm (".cfi_undefined rip"); -#endif + /* Some nuances on x86_64, see: + * https://github.com/libunwind/libunwind/blob/ec171c9ba7ea3abb2a1383cee2988a7 + * abd483a1f/src/x86_64/Gstep.c#L91-L92 + * https://github.com/libunwind/libunwind/blob/ec171c9ba7ea3abb2a1383cee2988a7 + * abd483a1f/src/dwarf/Gparser.c#L877 + */ + __asm__(".cfi_undefined rip\n" + ".cfi_undefined rbp\n" + ".cfi_return_column rbp\n"); func ((void *)arg); @@ -320,13 +326,14 @@ trampoline (int sig) ".fnend\n" #elif __aarch64__ - ".cfi_startproc\n" - "\tmov x30, #0\n" - "\tsub sp, sp, #16\n" - "\tstr x30, [sp, #0]\n" - ".cfi_def_cfa_offset 16\n" - ".cfi_offset 30, -16\n" + /* + * Call-chain ends here: we need to invalidate this frame's return + * address to make unwinding stop here. + * No nuances: AARCH64 has a special link register for storing + * return addresses. + */ + ".cfi_undefined x30\n" "\tmov x0, x20\n" "\tblr x19\n" # ifdef __APPLE__ @@ -834,4 +841,3 @@ coro_stack_free (struct coro_stack *stack) } #endif - diff --git a/third_party/coro/coro.h b/third_party/coro/coro.h index cc277e2786403..1c9fb66f36fae 100644 --- a/third_party/coro/coro.h +++ b/third_party/coro/coro.h @@ -420,4 +420,3 @@ void coro_destroy (coro_context *ctx); #endif #endif -