Message ID | 20180104160523.22995-16-marcandre.lureau@redhat.com |
---|---|
State | New |
Headers | show |
Series | Various build-sys and sanitizer related fixes | expand |
Hi Marc-André, On 01/04/2018 01:05 PM, Marc-André Lureau wrote: > It helps ASAN to detect more leaks on coroutine stacks, as found in > the following patch. > > 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. > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> > --- > include/qemu/compiler.h | 4 ++++ > util/coroutine-ucontext.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 50 insertions(+) > > diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h > index 340e5fdc09..5fcc4f7ec7 100644 > --- a/include/qemu/compiler.h > +++ b/include/qemu/compiler.h > @@ -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 */ > diff --git a/util/coroutine-ucontext.c b/util/coroutine-ucontext.c > index 6621f3f692..e78eae8766 100644 > --- a/util/coroutine-ucontext.c > +++ b/util/coroutine-ucontext.c > @@ -31,6 +31,11 @@ > #include <valgrind/valgrind.h> > #endif > > +#if defined(__SANITIZE_ADDRESS__) || __has_feature(address_sanitizer) > +#define CONFIG_ASAN 1 > +#include <sanitizer/asan_interface.h> > +#endif Sadly this fails on Travis: $ export CONFIG="--enable-debug --enable-debug-tcg --enable-trace-backends=log" $ export CC=gcc $ ./configure ${CONFIG} C compiler gcc Host C compiler cc C++ compiler c++ Objective-C compiler clang CFLAGS -Wno-maybe-uninitialized -Og -fsanitize=address -g [...] CC util/qemu-coroutine.o CC util/qemu-coroutine-lock.o CC util/qemu-coroutine-io.o CC util/qemu-coroutine-sleep.o CC util/coroutine-ucontext.o util/coroutine-ucontext.c:36:38: fatal error: sanitizer/asan_interface.h: No such file or directory #include <sanitizer/asan_interface.h> ^ compilation terminated. make: *** [util/coroutine-ucontext.o] Error 1 You simply need to update the .travis.yml, but does that mean your previous patch "Enable ASAN/UBSan by default if the compiler supports it." isn't correct? > + > typedef struct { > Coroutine base; > void *stack; > @@ -59,11 +64,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 +103,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 +122,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 +158,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 +209,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; > } > >
> On 01/04/2018 01:05 PM, Marc-André Lureau wrote: >> It helps ASAN to detect more leaks on coroutine stacks, as found in >> the following patch. >> >> 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. >> >> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> >> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> >> --- >> include/qemu/compiler.h | 4 ++++ >> util/coroutine-ucontext.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 50 insertions(+) >> >> diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h >> index 340e5fdc09..5fcc4f7ec7 100644 >> --- a/include/qemu/compiler.h >> +++ b/include/qemu/compiler.h >> @@ -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 */ >> diff --git a/util/coroutine-ucontext.c b/util/coroutine-ucontext.c >> index 6621f3f692..e78eae8766 100644 >> --- a/util/coroutine-ucontext.c >> +++ b/util/coroutine-ucontext.c >> @@ -31,6 +31,11 @@ >> #include <valgrind/valgrind.h> >> #endif >> >> +#if defined(__SANITIZE_ADDRESS__) || __has_feature(address_sanitizer) >> +#define CONFIG_ASAN 1 >> +#include <sanitizer/asan_interface.h> >> +#endif > > Sadly this fails on Travis: > > $ export CONFIG="--enable-debug --enable-debug-tcg > --enable-trace-backends=log" > $ export CC=gcc > $ ./configure ${CONFIG} > C compiler gcc > Host C compiler cc > C++ compiler c++ > Objective-C compiler clang > CFLAGS -Wno-maybe-uninitialized -Og -fsanitize=address -g > [...] > CC util/qemu-coroutine.o > CC util/qemu-coroutine-lock.o > CC util/qemu-coroutine-io.o > CC util/qemu-coroutine-sleep.o > CC util/coroutine-ucontext.o > util/coroutine-ucontext.c:36:38: fatal error: > sanitizer/asan_interface.h: No such file or directory > #include <sanitizer/asan_interface.h> > ^ > compilation terminated. > make: *** [util/coroutine-ucontext.o] Error 1 > > You simply need to update the .travis.yml, but does that mean your > previous patch "Enable ASAN/UBSan by default if the compiler supports > it." isn't correct? Err, the previous patch subject is "build-sys: add some sanitizers when --enable-debug if possible" >> + >> typedef struct { >> Coroutine base; >> void *stack; >> @@ -59,11 +64,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 +103,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 +122,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 +158,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 +209,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; >> } >> >> >
Hi On Tue, Jan 9, 2018 at 12:09 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > Hi Marc-André, > > On 01/04/2018 01:05 PM, Marc-André Lureau wrote: >> It helps ASAN to detect more leaks on coroutine stacks, as found in >> the following patch. >> >> 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. >> >> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> >> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> >> --- >> include/qemu/compiler.h | 4 ++++ >> util/coroutine-ucontext.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 50 insertions(+) >> >> diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h >> index 340e5fdc09..5fcc4f7ec7 100644 >> --- a/include/qemu/compiler.h >> +++ b/include/qemu/compiler.h >> @@ -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 */ >> diff --git a/util/coroutine-ucontext.c b/util/coroutine-ucontext.c >> index 6621f3f692..e78eae8766 100644 >> --- a/util/coroutine-ucontext.c >> +++ b/util/coroutine-ucontext.c >> @@ -31,6 +31,11 @@ >> #include <valgrind/valgrind.h> >> #endif >> >> +#if defined(__SANITIZE_ADDRESS__) || __has_feature(address_sanitizer) >> +#define CONFIG_ASAN 1 >> +#include <sanitizer/asan_interface.h> >> +#endif > > Sadly this fails on Travis: > > $ export CONFIG="--enable-debug --enable-debug-tcg > --enable-trace-backends=log" > $ export CC=gcc > $ ./configure ${CONFIG} > C compiler gcc > Host C compiler cc > C++ compiler c++ > Objective-C compiler clang > CFLAGS -Wno-maybe-uninitialized -Og -fsanitize=address -g > [...] > CC util/qemu-coroutine.o > CC util/qemu-coroutine-lock.o > CC util/qemu-coroutine-io.o > CC util/qemu-coroutine-sleep.o > CC util/coroutine-ucontext.o > util/coroutine-ucontext.c:36:38: fatal error: > sanitizer/asan_interface.h: No such file or directory > #include <sanitizer/asan_interface.h> > ^ > compilation terminated. > make: *** [util/coroutine-ucontext.o] Error 1 > > You simply need to update the .travis.yml, but does that mean your I think we need libgcc-6-dev. > previous patch "Enable ASAN/UBSan by default if the compiler supports > it." isn't correct? No, the problematic patch is "ucontext: annotate coroutine stack for ASAN", it should probably check for asan_interface.h before using it. Print a warning if it's not there during configure, as ASAN experience/reports will be inferior. > >> + >> typedef struct { >> Coroutine base; >> void *stack; >> @@ -59,11 +64,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 +103,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 +122,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 +158,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 +209,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; >> } >> >> >
Hi, On Tue, Jan 9, 2018 at 12:23 PM, Marc-André Lureau <marcandre.lureau@gmail.com> wrote: > Hi > > On Tue, Jan 9, 2018 at 12:09 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: >> Hi Marc-André, >> >> On 01/04/2018 01:05 PM, Marc-André Lureau wrote: >>> It helps ASAN to detect more leaks on coroutine stacks, as found in >>> the following patch. >>> >>> 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. >>> >>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> >>> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >>> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> >>> --- >>> include/qemu/compiler.h | 4 ++++ >>> util/coroutine-ucontext.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ >>> 2 files changed, 50 insertions(+) >>> >>> diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h >>> index 340e5fdc09..5fcc4f7ec7 100644 >>> --- a/include/qemu/compiler.h >>> +++ b/include/qemu/compiler.h >>> @@ -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 */ >>> diff --git a/util/coroutine-ucontext.c b/util/coroutine-ucontext.c >>> index 6621f3f692..e78eae8766 100644 >>> --- a/util/coroutine-ucontext.c >>> +++ b/util/coroutine-ucontext.c >>> @@ -31,6 +31,11 @@ >>> #include <valgrind/valgrind.h> >>> #endif >>> >>> +#if defined(__SANITIZE_ADDRESS__) || __has_feature(address_sanitizer) >>> +#define CONFIG_ASAN 1 >>> +#include <sanitizer/asan_interface.h> >>> +#endif >> >> Sadly this fails on Travis: >> >> $ export CONFIG="--enable-debug --enable-debug-tcg >> --enable-trace-backends=log" >> $ export CC=gcc >> $ ./configure ${CONFIG} >> C compiler gcc >> Host C compiler cc >> C++ compiler c++ >> Objective-C compiler clang >> CFLAGS -Wno-maybe-uninitialized -Og -fsanitize=address -g >> [...] >> CC util/qemu-coroutine.o >> CC util/qemu-coroutine-lock.o >> CC util/qemu-coroutine-io.o >> CC util/qemu-coroutine-sleep.o >> CC util/coroutine-ucontext.o >> util/coroutine-ucontext.c:36:38: fatal error: >> sanitizer/asan_interface.h: No such file or directory >> #include <sanitizer/asan_interface.h> >> ^ >> compilation terminated. >> make: *** [util/coroutine-ucontext.o] Error 1 >> >> You simply need to update the .travis.yml, but does that mean your > > I think we need libgcc-6-dev. > >> previous patch "Enable ASAN/UBSan by default if the compiler supports >> it." isn't correct? > > No, the problematic patch is "ucontext: annotate coroutine stack for > ASAN", it should probably check for asan_interface.h before using it. > > Print a warning if it's not there during configure, as ASAN > experience/reports will be inferior. > This should fix it: (Paolo, would you like me to resend or will you review the diff & squash? thanks) diff --git a/util/coroutine-ucontext.c b/util/coroutine-ucontext.c index e78eae8766..62fce888fd 100644 --- a/util/coroutine-ucontext.c +++ b/util/coroutine-ucontext.c @@ -32,9 +32,11 @@ #endif #if defined(__SANITIZE_ADDRESS__) || __has_feature(address_sanitizer) +#if HAVE_ASAN_IFACE_H #define CONFIG_ASAN 1 #include <sanitizer/asan_interface.h> #endif +#endif typedef struct { Coroutine base; diff --git a/configure b/configure index d033286b48..406ccd7cb4 100755 --- a/configure +++ b/configure @@ -5185,6 +5185,13 @@ if compile_prog "" "" ; then have_utmpx=yes fi +########################################## +# check for asan header +have_asan_iface_h=yes +if ! check_include "sanitizer/asan_interface.h" ; then + have_asan_iface_h=no +fi + ########################################## # End of CC checks # After here, no more $cc or $ld runs @@ -5198,6 +5205,10 @@ elif test "$debug" = "yes"; then write_c_skeleton; if compile_prog "-fsanitize=address" ""; then CFLAGS="-fsanitize=address $CFLAGS" + if test "$have_asan_iface_h" = "no" ; then + print_error "ASAN build enabled, but ASAN header missing." \ + "Without code annotation, the report may be inferior." + fi fi if compile_prog "-fsanitize=undefined" ""; then CFLAGS="-fsanitize=undefined $CFLAGS" @@ -6320,6 +6331,10 @@ if test "$have_utmpx" = "yes" ; then echo "HAVE_UTMPX=y" >> $config_host_mak fi +if test "$have_asan_iface_h" = "yes" ; then + echo "HAVE_ASAN_IFACE_H=y" >> $config_host_mak +fi + if test "$ivshmem" = "yes" ; then echo "CONFIG_IVSHMEM=y" >> $config_host_mak fi diff --git a/.travis.yml b/.travis.yml index f583839755..f2291e87a6 100644 --- a/.travis.yml +++ b/.travis.yml @@ -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 > > >> >>> + >>> typedef struct { >>> Coroutine base; >>> void *stack; >>> @@ -59,11 +64,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 +103,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 +122,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 +158,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 +209,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; >>> } >>> >>> >> > > > > -- > Marc-André Lureau
On 12/01/2018 12:15, Marc-André Lureau wrote: >>> You simply need to update the .travis.yml, but does that mean your >> I think we need libgcc-6-dev. >> >>> previous patch "Enable ASAN/UBSan by default if the compiler supports >>> it." isn't correct? >> No, the problematic patch is "ucontext: annotate coroutine stack for >> ASAN", it should probably check for asan_interface.h before using it. >> >> Print a warning if it's not there during configure, as ASAN >> experience/reports will be inferior. >> > This should fix it: > > (Paolo, would you like me to resend or will you review the diff & > squash? thanks) I'll integrate it, thanks. Paolo
diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h index 340e5fdc09..5fcc4f7ec7 100644 --- a/include/qemu/compiler.h +++ b/include/qemu/compiler.h @@ -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 */ diff --git a/util/coroutine-ucontext.c b/util/coroutine-ucontext.c index 6621f3f692..e78eae8766 100644 --- a/util/coroutine-ucontext.c +++ b/util/coroutine-ucontext.c @@ -31,6 +31,11 @@ #include <valgrind/valgrind.h> #endif +#if defined(__SANITIZE_ADDRESS__) || __has_feature(address_sanitizer) +#define CONFIG_ASAN 1 +#include <sanitizer/asan_interface.h> +#endif + typedef struct { Coroutine base; void *stack; @@ -59,11 +64,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 +103,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 +122,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 +158,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 +209,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; }