Skip to content

Commit

Permalink
coro: fix coro_{init,startup} unwind information
Browse files Browse the repository at this point in the history
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/scylladb#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 tarantool#4002

NO_DOC=internal bug fix
NO_CHANGELOG=internal bug fix
NO_TEST=unwind information annotation in inline assembly
  • Loading branch information
CuriousGeorgiy committed Mar 18, 2022
1 parent 7dc9fe4 commit 6c4b430
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 14 deletions.
49 changes: 36 additions & 13 deletions third_party/coro/coro.c
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@

#include "coro.h"

#include "trivia/config.h"

#include <stddef.h>
#include <string.h>

Expand Down Expand Up @@ -103,9 +105,20 @@ coro_init (void)

coro_transfer (new_coro, create_coro);

#if __GCC_HAVE_DWARF2_CFI_ASM && __amd64
asm (".cfi_undefined rip");
#endif
/*
* Call-chain ends here: we need to invalidate this frame's return
* address to make unwinding stop here.
* 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
*/
#ifdef ENABLE_BACKTRACE
__asm__(".cfi_undefined rip\n"
".cfi_undefined rbp\n"
".cfi_return_column rbp\n");
#endif /* ENABLE_BACKTRACE */

func ((void *)arg);

Expand Down Expand Up @@ -320,21 +333,32 @@ trampoline (int sig)
".fnend\n"

#elif __aarch64__

#ifdef ENABLE_BACKTRACE
".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__
#ifdef __APPLE__
"\tb _abort\n"
# else
#else /* __APPLE__ */
"\tb abort\n"
# endif
#endif /* __APPLE__ */
".cfi_endproc\n"
#else /* ENABLE_BACKTRACE */
"\tmov x0, x20\n"
"\tblr x19\n"
#ifdef __APPLE__
"\tb _abort\n"
#else /* __APPLE__ */
"\tb abort\n"
#endif /* __APPLE__ */
#endif /* ENABLE_BACKTRACE */

#else
#error unsupported architecture
Expand Down Expand Up @@ -834,4 +858,3 @@ coro_stack_free (struct coro_stack *stack)
}

#endif

1 change: 0 additions & 1 deletion third_party/coro/coro.h
Original file line number Diff line number Diff line change
Expand Up @@ -420,4 +420,3 @@ void coro_destroy (coro_context *ctx);
#endif

#endif

0 comments on commit 6c4b430

Please sign in to comment.