Commit d83414e1 authored by Marc-André Lureau's avatar Marc-André Lureau Committed by Paolo Bonzini
Browse files

ucontext: annotate coroutine stack for ASAN 

It helps ASAN to detect more leaks on coroutine stacks, and to get rid
of some extra warnings.

Before:

tests/test-coroutine -p
/basic/lifecycle
/basic/lifecycle: ==20781==WARNING: ASan doesn't fully support
makecontext/swapcontext functions and may produce false positives in
some cases!
==20781==WARNING: ASan is ignoring requested __asan_handle_no_return:
stack top: 0x7ffcb184d000; bottom 0x7ff6c4cfd000; size: 0x0005ecb50000
(25446121472)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189


OK

After:

tests/test-coroutine -p /basic/lifecycle
/basic/lifecycle: ==21110==WARNING: ASan doesn't fully support
makecontext/swapcontext functions and may produce false positives in
some cases!
OK

A similar work would need to be done for sigaltstack & windows fibers
to have similar coverage. Since ucontext is preferred, I didn't bother
checking the other coroutine implementations for now.

Update travis to fix the build with ASAN annotations.

Signed-off-by: default avatarMarc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20180116151152.4040-4-marcandre.lureau@redhat.com>
Signed-off-by: default avatarPaolo Bonzini <pbonzini@redhat.com>
parent 247724cb

.travis.yml

+2 −1
Original line number Diff line number Diff line
@@ -13,12 +13,13 @@ addons: 
      - libattr1-dev

      - libbrlapi-dev

      - libcap-ng-dev

      - libgcc-6-dev

      - libgnutls-dev

      - libgtk-3-dev

      - libiscsi-dev

      - liblttng-ust-dev

      - libnfs-dev

      - libncurses5-dev

      - libnfs-dev

      - libnss3-dev

      - libpixman-1-dev

      - libpng12-dev

configure

+30 −0
Original line number Diff line number Diff line
@@ -5213,6 +5213,8 @@ write_c_skeleton 


have_asan=no

have_ubsan=no

have_asan_iface_h=no

have_asan_iface_fiber=no



if test "$sanitizers" = "yes" ; then

  if compile_prog "$CPU_CFLAGS -Werror -fsanitize=address" ""; then
@@ -5221,12 +5223,29 @@ if test "$sanitizers" = "yes" ; then 
  if compile_prog "$CPU_CFLAGS -Werror -fsanitize=undefined" ""; then

      have_ubsan=yes

  fi



  if check_include "sanitizer/asan_interface.h" ; then

      have_asan_iface_h=yes

  fi



  cat > $TMPC << EOF

#include <sanitizer/asan_interface.h>

int main(void) {

  __sanitizer_start_switch_fiber(0, 0, 0);

  return 0;

}

EOF

  if compile_prog "$CPU_CFLAGS -Werror -fsanitize=address" "" ; then

      have_asan_iface_fiber=yes

  fi

fi



##########################################

# End of CC checks

# After here, no more $cc or $ld runs



write_c_skeleton



if test "$gcov" = "yes" ; then

  CFLAGS="-fprofile-arcs -ftest-coverage -g $CFLAGS"

  LDFLAGS="-fprofile-arcs -ftest-coverage $LDFLAGS"
@@ -5249,6 +5268,13 @@ fi 


if test "$have_asan" = "yes"; then

  CFLAGS="-fsanitize=address $CFLAGS"

  if test "$have_asan_iface_h" = "no" ; then

      echo "ASAN build enabled, but ASAN header missing." \

           "Without code annotation, the report may be inferior."

  elif test "$have_asan_iface_fiber" = "no" ; then

      echo "ASAN build enabled, but ASAN header is too old." \

           "Without code annotation, the report may be inferior."

  fi

fi

if test "$have_ubsan" = "yes"; then

  CFLAGS="-fsanitize=undefined $CFLAGS"
@@ -6237,6 +6263,10 @@ if test "$valgrind_h" = "yes" ; then 
  echo "CONFIG_VALGRIND_H=y" >> $config_host_mak

fi



if test "$have_asan_iface_fiber" = "yes" ; then

    echo "CONFIG_ASAN_IFACE_FIBER=y" >> $config_host_mak

fi



if test "$has_environ" = "yes" ; then

  echo "CONFIG_HAS_ENVIRON=y" >> $config_host_mak

fi

include/qemu/compiler.h

+4 −0
Original line number Diff line number Diff line
@@ -111,4 +111,8 @@ 
#define GCC_FMT_ATTR(n, m)

#endif



#ifndef __has_feature

#define __has_feature(x) 0 /* compatibility with non-clang compilers */

#endif



#endif /* COMPILER_H */

util/coroutine-ucontext.c

+48 −0
Original line number Diff line number Diff line
@@ -31,6 +31,13 @@ 
#include <valgrind/valgrind.h>

#endif



#if defined(__SANITIZE_ADDRESS__) || __has_feature(address_sanitizer)

#ifdef CONFIG_ASAN_IFACE_FIBER

#define CONFIG_ASAN 1

#include <sanitizer/asan_interface.h>

#endif

#endif



typedef struct {

    Coroutine base;

    void *stack;
@@ -59,11 +66,37 @@ union cc_arg { 
    int i[2];

};



static void finish_switch_fiber(void *fake_stack_save)

{

#ifdef CONFIG_ASAN

    const void *bottom_old;

    size_t size_old;



    __sanitizer_finish_switch_fiber(fake_stack_save, &bottom_old, &size_old);



    if (!leader.stack) {

        leader.stack = (void *)bottom_old;

        leader.stack_size = size_old;

    }

#endif

}



static void start_switch_fiber(void **fake_stack_save,

                               const void *bottom, size_t size)

{

#ifdef CONFIG_ASAN

    __sanitizer_start_switch_fiber(fake_stack_save, bottom, size);

#endif

}



static void coroutine_trampoline(int i0, int i1)

{

    union cc_arg arg;

    CoroutineUContext *self;

    Coroutine *co;

    void *fake_stack_save = NULL;



    finish_switch_fiber(NULL);



    arg.i[0] = i0;

    arg.i[1] = i1;
@@ -72,9 +105,13 @@ static void coroutine_trampoline(int i0, int i1) 


    /* Initialize longjmp environment and switch back the caller */

    if (!sigsetjmp(self->env, 0)) {

        start_switch_fiber(&fake_stack_save,

                           leader.stack, leader.stack_size);

        siglongjmp(*(sigjmp_buf *)co->entry_arg, 1);

    }



    finish_switch_fiber(fake_stack_save);



    while (true) {

        co->entry(co->entry_arg);

        qemu_coroutine_switch(co, co->caller, COROUTINE_TERMINATE);
@@ -87,6 +124,7 @@ Coroutine *qemu_coroutine_new(void) 
    ucontext_t old_uc, uc;

    sigjmp_buf old_env;

    union cc_arg arg = {0};

    void *fake_stack_save = NULL;



    /* The ucontext functions preserve signal masks which incurs a

     * system call overhead.  sigsetjmp(buf, 0)/siglongjmp() does not
@@ -122,8 +160,12 @@ Coroutine *qemu_coroutine_new(void) 


    /* swapcontext() in, siglongjmp() back out */

    if (!sigsetjmp(old_env, 0)) {

        start_switch_fiber(&fake_stack_save, co->stack, co->stack_size);

        swapcontext(&old_uc, &uc);

    }



    finish_switch_fiber(fake_stack_save);



    return &co->base;

}
@@ -169,13 +211,19 @@ qemu_coroutine_switch(Coroutine *from_, Coroutine *to_, 
    CoroutineUContext *from = DO_UPCAST(CoroutineUContext, base, from_);

    CoroutineUContext *to = DO_UPCAST(CoroutineUContext, base, to_);

    int ret;

    void *fake_stack_save = NULL;



    current = to_;



    ret = sigsetjmp(from->env, 0);

    if (ret == 0) {

        start_switch_fiber(action == COROUTINE_TERMINATE ?

                           NULL : &fake_stack_save, to->stack, to->stack_size);

        siglongjmp(to->env, action);

    }



    finish_switch_fiber(fake_stack_save);



    return ret;

}