Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correct TSAN tasking integration #36929

Merged
merged 1 commit into from
Aug 7, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Make.inc
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,10 @@ CXX_DISABLE_ASSERTION := -DJL_NDEBUG
DISABLE_ASSERTIONS := -DNDEBUG -DJL_NDEBUG
endif

ifeq ($(LLVM_ASSERTIONS),0)
CXX_DISABLE_ASSERTION += -DNDEBUG
endif

Comment on lines +434 to +437
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this variable does not have the actual runtime value (which should get set from llvm-config), we have a header (julia_assert.h) that is supposed to already deal with this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like llvm-config no longer sets NDEBUG, but it still needs to match what LLVM expects for ABI reasons.

# Compiler specific stuff

ifeq ($(USEMSVC), 1)
Expand Down
4 changes: 0 additions & 4 deletions src/aotcompile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,8 @@
#include <llvm/Transforms/IPO.h>
#include <llvm/Transforms/Scalar.h>
#include <llvm/Transforms/Vectorize.h>
#if defined(JL_ASAN_ENABLED)
#include <llvm/Transforms/Instrumentation/AddressSanitizer.h>
#endif
#if defined(JL_TSAN_ENABLED)
#include <llvm/Transforms/Instrumentation/ThreadSanitizer.h>
#endif
#include <llvm/Transforms/Scalar/GVN.h>
#include <llvm/Transforms/IPO/AlwaysInliner.h>
#include <llvm/Transforms/InstCombine/InstCombine.h>
Expand Down
6 changes: 6 additions & 0 deletions src/gc-stacks.c
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,12 @@ void sweep_stack_pools(void)
t->stkbuf = NULL;
_jl_free_stack(ptls2, stkbuf, bufsz);
}
#ifdef JL_TSAN_ENABLED
if (t->ctx.tsan_state) {
__tsan_destroy_fiber(t->ctx.tsan_state);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a NULL pointer?

Suggested change
__tsan_destroy_fiber(t->ctx.tsan_state);
__tsan_destroy_fiber(NULL);

t->ctx.tsan_state = NULL;
}
#endif
}
if (n >= l - ndel)
break;
Expand Down
8 changes: 3 additions & 5 deletions src/julia.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@
# define JL_THREAD_LOCAL
#endif

// Duplicated from options.h
#if defined(__has_feature) // Clang flavor
#if __has_feature(address_sanitizer)
#define JL_ASAN_ENABLED
Expand All @@ -74,6 +73,9 @@
#define JL_MSAN_ENABLED
#endif
#if __has_feature(thread_sanitizer)
#if __clang_major__ < 11
#error Thread sanitizer runtime libraries in clang < 11 leak memory and cannot be used
#endif
#define JL_TSAN_ENABLED
#endif
#else // GCC flavor
Expand Down Expand Up @@ -1810,10 +1812,6 @@ typedef struct _jl_task_t {
unsigned int copy_stack:31; // sizeof stack for copybuf
unsigned int started:1;

#if defined(JL_TSAN_ENABLED)
void *tsan_state;
#endif

// current exception handler
jl_handler_t *eh;
// saved gc stack top for context switches
Expand Down
3 changes: 2 additions & 1 deletion src/julia_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ void __sanitizer_finish_switch_fiber(void*, const void**, size_t*);
#endif
#ifdef JL_TSAN_ENABLED
void *__tsan_create_fiber(unsigned flags);
// void __tsan_destroy_fiber(void *fiber);
void *__tsan_get_current_fiber(void);
void __tsan_destroy_fiber(void *fiber);
void __tsan_switch_to_fiber(void *fiber, unsigned flags);
#endif
#ifdef __cplusplus
Expand Down
13 changes: 12 additions & 1 deletion src/julia_threads.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,15 @@ typedef win32_ucontext_t jl_ucontext_t;
#if defined(JL_HAVE_ASM) || defined(JL_HAVE_SIGALTSTACK)
typedef struct {
jl_jmp_buf uc_mcontext;
#if defined(JL_TSAN_ENABLED)
void *tsan_state;
#endif
} jl_ucontext_t;
#endif
#if defined(JL_HAVE_ASYNCIFY)
#if defined(JL_TSAN_ENABLED)
#error TSAN not currently supported with asyncify
#endif
typedef struct {
// This is the extent of the asyncify stack, but because the top of the
// asyncify stack (stacktop) is also the bottom of the C stack, we can
Expand All @@ -62,7 +68,12 @@ typedef struct {
#if defined(JL_HAVE_UCONTEXT) || defined(JL_HAVE_UNW_CONTEXT)
#define UNW_LOCAL_ONLY
#include <libunwind.h>
typedef ucontext_t jl_ucontext_t;
typedef struct {
ucontext_t ctx;
#if defined(JL_TSAN_ENABLED)
void *tsan_state;
#endif
} jl_ucontext_t;
#endif
#endif

Expand Down
3 changes: 3 additions & 0 deletions src/support/win32_ucontext.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ typedef struct {
size_t ss_size;
} uc_stack;
jmp_buf uc_mcontext;
#ifdef JL_TSAN_ENABLED
void *tsan_state;
#endif
} win32_ucontext_t;
void jl_makecontext(win32_ucontext_t *ucp, void (*func)(void));
void jl_swapcontext(win32_ucontext_t *oucp, const win32_ucontext_t *ucp);
Expand Down
Loading