Skip to content

Commit

Permalink
ASAN: Support coroutine context switching and stack tracing (#4153)
Browse files Browse the repository at this point in the history
For coroutine, we should use `__sanitizer_start_switch_fiber` which
similar to`VALGRIND_STACK_REGISTER`, see
google/sanitizers#189 (comment)
for details. If not fix this, asan will output warning:

```
==72269==WARNING: ASan is ignoring requested __asan_handle_no_return: stack type: default top: 0x00016f638000; bottom 0x000106bec000; size: 0x000068a4c000 (1755627520)
False positive error reports may follow
For details see google/sanitizers#189
```

It will cause asan failed to get the stack, see
`research/st/asan-switch.cpp` for example:

```
==71611==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x000103600733 at pc 0x0001009d3d7c bp 0x000100b4bd40 sp 0x000100b4bd38
WRITE of size 1 at 0x000103600733 thread T0
    #0 0x1009d3d78 in foo(void*) asan-switch.cpp:13
```

After fix this issue, it should provide the full stack when crashing:

```
==73437==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x000103300733 at pc 0x000100693d7c bp 0x00016f76f550 sp 0x00016f76f548
WRITE of size 1 at 0x000103300733 thread T0
    #0 0x100693d78 in foo(void*) asan-switch.cpp:13
    #1 0x100693df4 in main asan-switch.cpp:23
    #2 0x195aa20dc  (<unknown module>)
```

For primordial coroutine, if not set the stack by
`st_set_primordial_stack`, then the stack is NULL and asan can't get the
stack tracing. Note that it's optional and only make it fail to display
the stack information, no other errors.

---

Co-authored-by: john <hondaxiao@tencent.com>
  • Loading branch information
winlinvip and xiaozhihong authored Aug 22, 2024
1 parent 55610cf commit 8f48a0e
Show file tree
Hide file tree
Showing 15 changed files with 236 additions and 15 deletions.
6 changes: 6 additions & 0 deletions trunk/3rdparty/st-srs/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,12 @@ endif
# or cache the stack and reuse it:
# make EXTRA_CFLAGS=-DMD_CACHE_STACK
#
# or enable support for valgrind:
# make EXTRA_CFLAGS="-DMD_VALGRIND"
#
# or enable support for asan:
# make EXTRA_CFLAGS="-DMD_ASAN -fsanitize=address -fno-omit-frame-pointer"
#
# or enable the coverage for utest:
# make UTEST_FLAGS="-fprofile-arcs -ftest-coverage"
#
Expand Down
6 changes: 6 additions & 0 deletions trunk/3rdparty/st-srs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,12 @@ Linux with valgrind and epoll:
make linux-debug EXTRA_CFLAGS="-DMD_HAVE_EPOLL -DMD_VALGRIND"
```

Linux with ASAN(Google Address Sanitizer):

```bash
make linux-debug EXTRA_CFLAGS="-DMD_ASAN"
```

## Mac: Usage

Get code:
Expand Down
9 changes: 9 additions & 0 deletions trunk/3rdparty/st-srs/common.c
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,12 @@

#include "common.h"

void *_st_primordial_stack_bottom = NULL;
size_t _st_primordial_stack_size = 0;

void st_set_primordial_stack(void *top, void *bottom)
{
_st_primordial_stack_bottom = bottom;
_st_primordial_stack_size = (char *)top - (char *)bottom;
}

76 changes: 73 additions & 3 deletions trunk/3rdparty/st-srs/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,10 @@ struct _st_thread {
_st_clist_t tlink; /* For putting on thread queue */
#endif

#ifdef MD_ASAN
void *fake_stack; /* Fake stack for ASAN */
#endif

st_utime_t due; /* Wakeup time when thread is sleeping */
_st_thread_t *left; /* For putting in timeout heap */
_st_thread_t *right; /* -- see docs/timeout_heap.txt for details */
Expand Down Expand Up @@ -314,7 +318,7 @@ extern __thread _st_eventsys_t *_st_eventsys;
* Forward declarations
*/

void _st_vp_schedule(void);
void _st_vp_schedule(_st_thread_t *from);
void _st_vp_check_clock(void);
void *_st_idle_thread_start(void *arg);
void _st_thread_main(void);
Expand Down Expand Up @@ -365,6 +369,64 @@ _st_thread_t *st_thread_create(void *(*start)(void *arg), void *arg, int joinabl
#define ST_SWITCH_IN_CB(_thread)
#endif

#ifdef MD_ASAN
/*
* Fiber annotation interface.
*
* Before switching to a different stack, one must call
* __sanitizer_start_switch_fiber with a pointer to the bottom of the
* destination stack and its size. When code starts running on the new stack,
* it must call __sanitizer_finish_switch_fiber to finalize the switch.
* The start_switch function takes a void** to store the current fake stack if
* there is one (it is needed when detect_stack_use_after_return is enabled).
* When restoring a stack, this pointer must be given to the finish_switch
* function. In most cases, this void* can be stored on the stack just before
* switching. When leaving a fiber definitely, null must be passed as first
* argument to the start_switch function so that the fake stack is destroyed.
* If you do not want support for stack use-after-return detection, you can
* always pass null to these two functions.
* Note that the fake stack mechanism is disabled during fiber switch, so if a
* signal callback runs during the switch, it will not benefit from the stack
* use-after-return detection.
*
* See https://github.com/google/sanitizers/issues/189#issuecomment-1346243598
*/
extern void __sanitizer_start_switch_fiber(void **fake_stack_save,
const void *bottom, size_t size);

extern void __sanitizer_finish_switch_fiber(void *fake_stack_save,
const void **bottom_old,
size_t *size_old);

/* The stack for primoridal thread. */
extern void *_st_primordial_stack_bottom;
extern size_t _st_primordial_stack_size;

static inline void _st_asan_start_switch(_st_thread_t *from, _st_thread_t *thread)
{
/* For primordial thread, the stack is NULL, so asan can not capture it. */
const void *stk_bottom = thread->stack ? thread->stack->stk_bottom : NULL;
size_t stk_size = thread->stack ? thread->stack->stk_size : 0;

/* For primordial thread, user should setup the stack information. */
if (!stk_bottom && (thread->flags & _ST_FL_PRIMORDIAL)) {
stk_bottom = _st_primordial_stack_bottom;
stk_size = _st_primordial_stack_size;
}

/*
* Save the current stack to fake_stack of from, tell asan the target stack
* we are targeting to switch to.
*/
__sanitizer_start_switch_fiber(&from->fake_stack, stk_bottom, stk_size);
}

static inline void _st_asan_finish_switch(_st_thread_t *thread)
{
__sanitizer_finish_switch_fiber(thread->fake_stack, NULL, NULL);
}
#endif

/*
* Switch away from the current thread context by saving its state and
* calling the thread scheduler
Expand All @@ -374,9 +436,14 @@ static inline void _st_switch_context(_st_thread_t *thread)
ST_SWITCH_OUT_CB(thread);

if (!_st_md_cxt_save(thread->context)) {
_st_vp_schedule();
_st_vp_schedule(thread);
}

#ifdef MD_ASAN
/* Switch from other thread to this running thread. */
_st_asan_finish_switch(thread);
#endif

ST_DEBUG_ITERATE_THREADS();
ST_SWITCH_IN_CB(thread);
}
Expand All @@ -385,8 +452,11 @@ static inline void _st_switch_context(_st_thread_t *thread)
* Restore a thread context that was saved by _st_switch_context or
* initialized by _ST_INIT_CONTEXT
*/
static inline void _st_restore_context(_st_thread_t *thread)
static inline void _st_restore_context(_st_thread_t *from, _st_thread_t *thread)
{
#ifdef MD_ASAN
_st_asan_start_switch(from, thread);
#endif
_st_this_thread = thread;
_st_md_cxt_restore(thread->context, 1);
}
Expand Down
2 changes: 2 additions & 0 deletions trunk/3rdparty/st-srs/public.h
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,8 @@ extern st_netfd_t st_open(const char *path, int oflags, mode_t mode);
extern void st_destroy(void);
extern int st_thread_setspecific2(st_thread_t thread, int key, void *value);

extern void st_set_primordial_stack(void *top, void *bottom);

#ifdef DEBUG
extern void _st_show_thread_stack(st_thread_t thread, const char *messg);
extern void _st_iterate_threads(void);
Expand Down
20 changes: 13 additions & 7 deletions trunk/3rdparty/st-srs/sched.c
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ int st_poll(struct pollfd *pds, int npds, st_utime_t timeout)
}


void _st_vp_schedule(void)
void _st_vp_schedule(_st_thread_t *from)
{
_st_thread_t *thread;

Expand All @@ -159,7 +159,7 @@ void _st_vp_schedule(void)

/* Resume the thread */
thread->state = _ST_ST_RUNNING;
_st_restore_context(thread);
_st_restore_context(from, thread);
}


Expand Down Expand Up @@ -368,6 +368,11 @@ int st_thread_join(_st_thread_t *thread, void **retvalp)
void _st_thread_main(void)
{
_st_thread_t *thread = _st_this_thread;

#ifdef MD_ASAN
/* Switch from other thread to this new created thread. */
_st_asan_finish_switch(thread);
#endif

/*
* Cap the stack by zeroing out the saved return address register
Expand Down Expand Up @@ -572,7 +577,7 @@ void _st_vp_check_clock(void)
/* Make thread runnable */
ST_ASSERT(!(thread->flags & _ST_FL_IDLE_THREAD));
thread->state = _ST_ST_RUNNABLE;
// Insert at the head of RunQ, to execute timer first.
/* Insert at the head of RunQ, to execute timer first. */
st_clist_insert_after(&thread->links, &_st_this_vp.run_q);
}
}
Expand All @@ -589,7 +594,7 @@ void st_thread_yield()
/* Check sleep queue for expired threads */
_st_vp_check_clock();

// If not thread in RunQ to yield to, ignore and continue to run.
/* If not thread in RunQ to yield to, ignore and continue to run. */
if (_st_this_vp.run_q.next == &_st_this_vp.run_q) {
return;
}
Expand All @@ -598,11 +603,11 @@ void st_thread_yield()
++_st_stat_thread_yield2;
#endif

// Append thread to the tail of RunQ, we will back after all threads executed.
/* Append thread to the tail of RunQ, we will back after all threads executed. */
me->state = _ST_ST_RUNNABLE;
st_clist_insert_before(&me->links, &_st_this_vp.run_q);

// Yield to other threads in the RunQ.
/* Yield to other threads in the RunQ. */
_st_switch_context(me);
}

Expand Down Expand Up @@ -664,8 +669,9 @@ _st_thread_t *st_thread_create(void *(*start)(void *arg), void *arg, int joinabl
thread->arg = arg;

/* Note that we must directly call rather than call any functions. */
if (_st_md_cxt_save(thread->context))
if (_st_md_cxt_save(thread->context)) {
_st_thread_main();
}
MD_GET_SP(thread) = (long)(stack->sp);

/* If thread is joinable, allocate a termination condition variable */
Expand Down
25 changes: 22 additions & 3 deletions trunk/auto/depends.sh
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ fi
#####################################################################################
# Check for address sanitizer, see https://github.com/google/sanitizers
#####################################################################################
if [[ $SRS_SANITIZER == YES && $OS_IS_X86_64 == YES ]]; then
if [[ $SRS_SANITIZER == YES ]]; then
echo 'int main() { return 0; }' > ${SRS_OBJS}/test_sanitizer.c &&
gcc -fsanitize=address -fno-omit-frame-pointer -g -O0 ${SRS_OBJS}/test_sanitizer.c \
-o ${SRS_OBJS}/test_sanitizer 1>/dev/null 2>&1;
Expand All @@ -217,7 +217,7 @@ if [[ $SRS_SANITIZER == YES && $OS_IS_X86_64 == YES ]]; then
fi
fi

if [[ $SRS_SANITIZER == YES && $OS_IS_X86_64 == YES && $SRS_SANITIZER_STATIC == NO ]]; then
if [[ $SRS_SANITIZER == YES && $SRS_SANITIZER_STATIC == NO ]]; then
echo 'int main() { return 0; }' > ${SRS_OBJS}/test_sanitizer.c &&
gcc -fsanitize=address -fno-omit-frame-pointer -static-libasan -g -O0 ${SRS_OBJS}/test_sanitizer.c \
-o ${SRS_OBJS}/test_sanitizer 1>/dev/null 2>&1;
Expand All @@ -228,7 +228,7 @@ if [[ $SRS_SANITIZER == YES && $OS_IS_X86_64 == YES && $SRS_SANITIZER_STATIC ==
fi
fi

if [[ $SRS_SANITIZER == YES && $OS_IS_X86_64 == YES && $SRS_SANITIZER_LOG == NO ]]; then
if [[ $SRS_SANITIZER == YES && $SRS_SANITIZER_LOG == NO ]]; then
echo "#include <sanitizer/asan_interface.h>" > ${SRS_OBJS}/test_sanitizer.c &&
echo "int main() { return 0; }" >> ${SRS_OBJS}/test_sanitizer.c &&
gcc -fsanitize=address -fno-omit-frame-pointer -g -O0 ${SRS_OBJS}/test_sanitizer.c \
Expand All @@ -240,6 +240,18 @@ if [[ $SRS_SANITIZER == YES && $OS_IS_X86_64 == YES && $SRS_SANITIZER_LOG == NO
fi
fi

if [[ $SRS_SANITIZER == YES ]]; then
echo "#include <sanitizer/asan_interface.h>" > ${SRS_OBJS}/test_sanitizer.c &&
echo "int main() { __sanitizer_start_switch_fiber(NULL, NULL, 0); }" >> ${SRS_OBJS}/test_sanitizer.c &&
gcc -fsanitize=address -fno-omit-frame-pointer -g -O0 ${SRS_OBJS}/test_sanitizer.c \
-o ${SRS_OBJS}/test_sanitizer 1>/dev/null 2>&1;
ret=$?; rm -rf ${SRS_OBJS}/test_sanitizer*
if [[ $ret -eq 0 ]]; then
echo "libasan fiber switch api found ok!";
SRS_SANITIZER_FIBER_SWITCH=YES
fi
fi

#####################################################################################
# state-threads
#####################################################################################
Expand Down Expand Up @@ -267,6 +279,13 @@ fi
if [[ $SRS_DEBUG_STATS == YES ]]; then
_ST_EXTRA_CFLAGS="$_ST_EXTRA_CFLAGS -DDEBUG_STATS"
fi
# Whether to enable asan.
if [[ $SRS_SANITIZER_FIBER_SWITCH == YES ]]; then
_ST_EXTRA_CFLAGS="$_ST_EXTRA_CFLAGS -DMD_ASAN"
fi
if [[ $SRS_SANITIZER == YES ]]; then
_ST_EXTRA_CFLAGS="$_ST_EXTRA_CFLAGS -fsanitize=address -fno-omit-frame-pointer"
fi
# Pass the global extra flags.
if [[ $SRS_EXTRA_FLAGS != '' ]]; then
_ST_EXTRA_CFLAGS="$_ST_EXTRA_CFLAGS $SRS_EXTRA_FLAGS"
Expand Down
4 changes: 2 additions & 2 deletions trunk/configure
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ fi

# For Sanitizer
# @doc: https://github.com/google/sanitizers/wiki/AddressSanitizer
if [[ $SRS_SANITIZER == YES && $OS_IS_X86_64 == YES ]]; then
if [[ $SRS_SANITIZER == YES ]]; then
CXXFLAGS="${CXXFLAGS} -fsanitize=address -fno-omit-frame-pointer";
fi

Expand Down Expand Up @@ -244,7 +244,7 @@ fi

# For address sanitizer
# @doc: https://github.com/google/sanitizers/wiki/AddressSanitizer
if [[ $SRS_SANITIZER == YES && $OS_IS_X86_64 == YES ]]; then
if [[ $SRS_SANITIZER == YES ]]; then
SrsLinkOptions="${SrsLinkOptions} -fsanitize=address -fno-omit-frame-pointer";
if [[ $SRS_SANITIZER_STATIC == YES ]]; then
SrsLinkOptions="${SrsLinkOptions} -static-libasan";
Expand Down
2 changes: 2 additions & 0 deletions trunk/research/st/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,5 @@ huge-threads
exceptions
hello
pthreads
asan-switch
hello-thread
40 changes: 40 additions & 0 deletions trunk/research/st/asan-switch.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
g++ asan-switch.cpp ../../objs/st/libst.a -fsanitize=address -fno-omit-frame-pointer -g -O0 -o asan-switch && ./asan-switch
*/
#include <stdio.h>
#include <string.h>
#include <sys/resource.h>
#include "../../objs/st/st.h"

void* foo(void *args) {
for (int i = 0; ; i++) {
st_sleep(1);
if (i && (i % 2) == 0) {
char *p = new char[3];
p[3] = 'H';
}
printf("#%d: main: working\n", i);
}
return NULL;
}

int main(int argc, char **argv) {
register void* stack_top asm ("sp");
struct rlimit limit;
if (getrlimit (RLIMIT_STACK, &limit) == 0) {
void* stack_bottom = (char*)stack_top - limit.rlim_cur;
st_set_primordial_stack(stack_top, stack_bottom);
}

st_init();
if (argc > 1) {
// Directly call foo() to trigger ASAN, call the function in the primordial thread,
// note that asan can not capther the stack of primordial thread.
foo(NULL);
} else {
st_thread_create(foo, NULL, 0, 0);
st_thread_exit(NULL);
}
return 0;
}

21 changes: 21 additions & 0 deletions trunk/research/st/hello-thread.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/*
g++ hello-thread.cpp ../../objs/st/libst.a -g -O0 -o hello-thread && ./hello-thread
*/
#include <stdio.h>
#include "../../objs/st/st.h"

void* foo(void *args) {
for (int i = 0; ; i++) {
st_sleep(1);
printf("#%d: main: working\n", i);
}

return NULL;
}

int main() {
st_init();
st_thread_create(foo, NULL, 0, 0);
st_thread_exit(NULL);
return 0;
}
9 changes: 9 additions & 0 deletions trunk/research/st/hello.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#include <sys/resource.h>
#include <stdio.h>
int main (void)
{
struct rlimit limit;

getrlimit (RLIMIT_STACK, &limit);
printf ("\nStack Limit = %ld and %ld max\n", limit.rlim_cur, limit.rlim_max);
}
Loading

0 comments on commit 8f48a0e

Please sign in to comment.