> 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 <[email protected]> >> Reviewed-by: Philippe Mathieu-Daudé <[email protected]> >> Reviewed-by: Stefan Hajnoczi <[email protected]> >> --- >> 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;
>> }
>>
>>
>
signature.asc
Description: OpenPGP digital signature
