Robert Foley <robert.fo...@linaro.org> writes:
> From: Lingfeng Yang <l...@google.com> > > We tried running QEMU under tsan in 2016, but tsan's lack of support for > longjmp-based fibers was a blocker: > https://groups.google.com/forum/#!topic/thread-sanitizer/se0YuzfWazw > > Fortunately, thread sanitizer gained fiber support in early 2019: > https://reviews.llvm.org/D54889 > > This patch brings tsan support upstream by importing the patch that annotated > QEMU's coroutines as tsan fibers in Android's QEMU fork: > https://android-review.googlesource.com/c/platform/external/qemu/+/844675 > > Tested with '--enable-tsan --cc=clang-9 --cxx=clang++-9 --disable-werror' > configure flags. > > Signed-off-by: Lingfeng Yang <l...@google.com> > Signed-off-by: Emilio G. Cota <c...@braap.org> > [cota: minor modifications + configure changes] > Signed-off-by: Robert Foley <robert.fo...@linaro.org> > [RF: configure changes for warnings, erorr handling + minor modifications] <snip> > > +#define UC_DEBUG 0 > +#if UC_DEBUG && defined(CONFIG_TSAN) > +#define UC_TRACE(fmt, ...) fprintf(stderr, "%s:%d:%p " fmt "\n", \ > + __func__, __LINE__, __tsan_get_current_fiber(), ##__VA_ARGS__); > +#else > +#define UC_TRACE(fmt, ...) > +#endif > + We shouldn't be introducing new debug printfs if we can avoid it. I suspect a separate patch could introduce some relevant trace points that are outside the #if CONFIG_TSAN chunks. > /** > * Per-thread coroutine bookkeeping > */ > @@ -65,7 +80,20 @@ union cc_arg { > int i[2]; > }; > > -static void finish_switch_fiber(void *fake_stack_save) > +/* QEMU_ALWAYS_INLINE only does so if __OPTIMIZE__, so we cannot use it. */ > +static inline __attribute__((always_inline)) > +void on_new_fiber(CoroutineUContext *co) > +{ We could put a tracepoint here at something like trace_new_fibre() but I suspect for following what's going on you could probably just have tracepoints in the higher coroutine functions and leave the fibre stuff as purely a CONFIG_TSAN detail. Please we wouldn't have to ague about American vs British spelling for the tracepoints ;-) <snip> Otherwise without the UC_TRACE verbiage: Reviewed-by: Alex Bennée <alex.ben...@linaro.org> -- Alex Bennée
