Skip to content

Re-correct TSAN integration #42447

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

Re-correct TSAN integration #42447

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
22d25b1
Revert "Update references to tsan state (#42440)"
Keno Oct 1, 2021
c9369ad
Re-correct TSAN integration
Keno Oct 1, 2021
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
6 changes: 3 additions & 3 deletions src/gc-stacks.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,9 +234,9 @@ void sweep_stack_pools(void)
_jl_free_stack(ptls2, stkbuf, bufsz);
}
#ifdef _COMPILER_TSAN_ENABLED_
if (t->tsan_state) {
__tsan_destroy_fiber(t->tsan_state);
t->tsan_state = NULL;
if (t->ctx.tsan_state) {
__tsan_destroy_fiber(t->ctx.tsan_state);
t->ctx.tsan_state = NULL;
}
#endif
}
Expand Down
3 changes: 0 additions & 3 deletions src/julia.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1851,9 +1851,6 @@ typedef struct _jl_task_t {
struct jl_stack_context_t copy_stack_ctx;
#endif
};
#if defined(_COMPILER_TSAN_ENABLED_)
void *tsan_state;
#endif
void *stkbuf; // malloc'd memory (either copybuf or stack)
size_t bufsz; // actual sizeof stkbuf
unsigned int copy_stack:31; // sizeof stack for copybuf
Expand Down
12 changes: 11 additions & 1 deletion src/julia_threads.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ typedef win32_ucontext_t jl_ucontext_t;

struct jl_stack_context_t {
jl_jmp_buf uc_mcontext;
#if defined(_COMPILER_TSAN_ENABLED_)
void *tsan_state;
#endif
};

#if (!defined(JL_HAVE_UNW_CONTEXT) && defined(JL_HAVE_ASM)) || defined(JL_HAVE_SIGALTSTACK)
Expand All @@ -84,7 +87,14 @@ typedef unw_context_t jl_ucontext_t;
#endif
#if defined(JL_HAVE_UCONTEXT)
#include <ucontext.h>
typedef ucontext_t jl_ucontext_t;
typedef struct {
ucontext_t ctx;
#if defined(_COMPILER_TSAN_ENABLED_)
void *tsan_state;
#else
uint8_t tsan_state[0]
#endif
} jl_ucontext_t;
#endif
#endif

Expand Down
78 changes: 58 additions & 20 deletions src/task.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,29 @@ static inline void sanitizer_finish_switch_fiber(void) {}
#endif

#if defined(_COMPILER_TSAN_ENABLED_)
static inline void tsan_destroy_ctx(jl_ptls_t ptls, void **state) {
if (state != &ptls->root_task->tsan_state) {
__tsan_destroy_fiber(*state);
static inline void tsan_destroy_ctx(jl_ptls_t ptls, jl_ucontext_t *ctx) {
if (ctx != &ptls->root_task->ctx) {
__tsan_destroy_fiber(ctx->tsan_state);
}
*state = NULL;
ctx->tsan_state = NULL;
}
static inline void tsan_switch_to_ctx(void *state) {
__tsan_switch_to_fiber(state, 0);
static inline void tsan_destroy_copyctx(jl_ptls_t ptls, struct jl_stack_context_t *ctx) {
if (ctx != &ptls->root_task->copy_stack_ctx) {
__tsan_destroy_fiber(ctx->tsan_state);
}
ctx->tsan_state = NULL;
}
static inline void tsan_switch_to_ctx(jl_ucontext_t *ctx) {
__tsan_switch_to_fiber(ctx->tsan_state, 0);
}
static inline void tsan_switch_to_copyctx(struct jl_stack_context_t *ctx) {
__tsan_switch_to_fiber(ctx->tsan_state, 0);
}
#else
static inline void tsan_destroy_ctx(jl_ptls_t ptls, jl_ucontext_t *ctx) {}
static inline void tsan_destroy_copyctx(jl_ptls_t ptls, struct jl_stack_context_t *ctx) {}
static inline void tsan_switch_to_ctx(jl_ucontext_t *ctx) {}
static inline void tsan_switch_to_copyctx(struct jl_stack_context_t *ctx) {}
#endif

// empirically, jl_finish_task needs about 64k stack space to infer/run
Expand Down Expand Up @@ -180,6 +194,7 @@ static void restore_stack2(jl_task_t *t, jl_ptls_t ptls, jl_task_t *lastt)
#error COPY_STACKS is incompatible with this platform
#endif
sanitizer_start_switch_fiber(t->stkbuf, t->bufsz);
tsan_switch_to_copyctx(&t->copy_stack_ctx);
#if defined(_OS_WINDOWS_)
jl_setcontext(&t->ctx);
#else
Expand Down Expand Up @@ -336,7 +351,7 @@ static void ctx_switch(jl_task_t *lastt)
assert(ptls->locks.len == 0);

#ifdef _COMPILER_TSAN_ENABLED_
if (lastt->tsan_state != __tsan_get_current_fiber()) {
if (lastt->ctx.tsan_state != __tsan_get_current_fiber()) {
// Something went really wrong - don't even assume that we can
// use assert/abort which involve lots of signal handling that
// looks at the tsan state.
Expand All @@ -357,9 +372,9 @@ static void ctx_switch(jl_task_t *lastt)
t->sticky = 1;
t->bufsz = 0;
if (always_copy_stacks)
memcpy(&t->copy_stack_ctx, &ptls->copy_stack_ctx, sizeof(t->copy_stack_ctx));
memcpy(&t->copy_stack_ctx, &ptls->copy_stack_ctx, sizeof(t->copy_stack_ctx) - sizeof(t->copy_stack_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.

Suggested change
memcpy(&t->copy_stack_ctx, &ptls->copy_stack_ctx, sizeof(t->copy_stack_ctx) - sizeof(t->copy_stack_ctx.tsan_state));
memcpy(&t->copy_stack_ctx.uc_mcontext, &ptls->copy_stack_ctx.uc_mcontext, sizeof(t->copy_stack_ctx.uc_mcontext));

But I'm not sure if uc_mcontext is something C can deal with, or if it needed to be wrapped in a struct to be something that C could create access.

Copy link
Member

Choose a reason for hiding this comment

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

But note that this is in the COPY_STACKS code path, so currently we disable this code path entirely if TSAN is enabled

else
memcpy(&t->ctx, &ptls->base_ctx, sizeof(t->ctx));
memcpy(&t->ctx, &ptls->base_ctx, sizeof(t->ctx) - sizeof(t->ctx.tsan_state));
Copy link
Member

@vtjnash vtjnash Oct 3, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
memcpy(&t->ctx, &ptls->base_ctx, sizeof(t->ctx) - sizeof(t->ctx.tsan_state));
memcpy(&t->ctx.uc_mcontext, &ptls->base_ctx.uc_mcontext, sizeof(t->ctx.uc_mcontext));

#else
jl_throw(jl_memory_exception);
#endif
Expand Down Expand Up @@ -401,35 +416,39 @@ static void ctx_switch(jl_task_t *lastt)
#endif
jl_set_pgcstack(&t->gcstack);

#if defined(_COMPILER_TSAN_ENABLED_)
tsan_switch_to_ctx(t->tsan_state);
if (killed)
tsan_destroy_ctx(ptls, &lastt->tsan_state);
#endif
if (t->started) {
#ifdef COPY_STACKS
if (t->copy_stack) {
if (!killed && !lastt->copy_stack)
restore_stack2(t, ptls, lastt);
else if (lastt->copy_stack) {
restore_stack(t, ptls, NULL); // (doesn't return)
}
else {
restore_stack(t, ptls, (char*)1); // (doesn't return)
tsan_switch_to_copyctx(&t->copy_stack_ctx);
if (killed)
tsan_destroy_copyctx(ptls, &lastt->copy_stack_ctx);

if (lastt->copy_stack) {
restore_stack(t, ptls, NULL); // (doesn't return)
}
else {
restore_stack(t, ptls, (char*)1); // (doesn't return)
}
}
}
else
#endif
{
sanitizer_start_switch_fiber(t->stkbuf, t->bufsz);
if (killed) {
tsan_switch_to_ctx(&t->ctx);
tsan_destroy_ctx(ptls, &lastt->ctx);
jl_set_fiber(&t->ctx); // (doesn't return)
abort(); // unreachable
}
else {
if (lastt->copy_stack) {
// Resume at the jl_setjmp earlier in this function,
// don't do a full task swap
tsan_switch_to_ctx(&t->ctx);
jl_set_fiber(&t->ctx); // (doesn't return)
}
else {
Expand All @@ -441,6 +460,10 @@ static void ctx_switch(jl_task_t *lastt)
else {
sanitizer_start_switch_fiber(t->stkbuf, t->bufsz);
if (t->copy_stack && always_copy_stacks) {
tsan_switch_to_ctx(&t->ctx);
if (killed) {
tsan_destroy_ctx(ptls, &lastt->ctx);
}
#ifdef COPY_STACKS
#if defined(_OS_WINDOWS_)
jl_setcontext(&t->ctx);
Expand All @@ -452,11 +475,14 @@ static void ctx_switch(jl_task_t *lastt)
}
else {
if (killed) {
tsan_switch_to_ctx(&t->ctx);
tsan_destroy_ctx(ptls, &lastt->ctx);
jl_start_fiber_set(&t->ctx); // (doesn't return)
abort();
}
else if (lastt->copy_stack) {
// Resume at the jl_setjmp earlier in this function
tsan_switch_to_ctx(&t->ctx);
jl_start_fiber_set(&t->ctx); // (doesn't return)
abort();
}
Expand Down Expand Up @@ -764,7 +790,10 @@ JL_DLLEXPORT jl_task_t *jl_new_task(jl_function_t *start, jl_value_t *completion
}
#endif
#ifdef _COMPILER_TSAN_ENABLED_
t->tsan_state = __tsan_create_fiber(0);
if (always_copy_stacks)
t->copy_stack_ctx.tsan_state = __tsan_create_fiber(0);
else
t->ctx.tsan_state = __tsan_create_fiber(0);
#endif
return t;
}
Expand Down Expand Up @@ -924,10 +953,12 @@ static void jl_start_fiber_set(jl_ucontext_t *t)
static void jl_start_fiber_swap(jl_ucontext_t *lastt, jl_ucontext_t *t)
{
assert(lastt);
tsan_switch_to_ctx(t);
swapcontext(lastt, t);
}
static void jl_swap_fiber(jl_ucontext_t *lastt, jl_ucontext_t *t)
{
tsan_switch_to_ctx(t);
swapcontext(lastt, t);
}
static void jl_set_fiber(jl_ucontext_t *t)
Expand Down Expand Up @@ -982,6 +1013,7 @@ static void jl_swap_fiber(jl_ucontext_t *lastt, jl_ucontext_t *t)
{
if (jl_setjmp(lastt->uc_mcontext, 0))
return;
tsan_switch_to_ctx(t);
jl_set_fiber(t); // doesn't return
}
static void jl_set_fiber(jl_ucontext_t *t)
Expand Down Expand Up @@ -1069,6 +1101,7 @@ static void jl_start_fiber_swap(jl_ucontext_t *lastt, jl_ucontext_t *t)
if (jl_setjmp(lastt->uc_mcontext, 0))
return;
#endif
tsan_switch_to_ctx(t);
jl_start_fiber_set(t); // doesn't return
}
static void jl_start_fiber_set(jl_ucontext_t *t)
Expand Down Expand Up @@ -1219,12 +1252,14 @@ static void jl_start_fiber_swap(jl_ucontext_t *lastt, jl_ucontext_t *t)
assert(lastt);
if (lastt && jl_setjmp(lastt->uc_mcontext, 0))
return;
tsan_switch_to_ctx(t);
jl_start_fiber_set(t);
}
static void jl_swap_fiber(jl_ucontext_t *lastt, jl_ucontext_t *t)
{
if (jl_setjmp(lastt->uc_mcontext, 0))
return;
tsan_switch_to_ctx(t);
jl_start_fiber_set(t); // doesn't return
}
static void jl_set_fiber(jl_ucontext_t *t)
Expand Down Expand Up @@ -1311,7 +1346,10 @@ jl_task_t *jl_init_root_task(jl_ptls_t ptls, void *stack_lo, void *stack_hi)
assert(jl_current_task == ct);

#ifdef _COMPILER_TSAN_ENABLED_
ct->tsan_state = __tsan_get_current_fiber();
if (always_copy_stacks)
ct->copy_stack_ctx.tsan_state = __tsan_get_current_fiber();
else
ct->ctx.tsan_state = __tsan_get_current_fiber();
#endif

#ifdef COPY_STACKS
Expand Down