Skip to content

Commit

Permalink
Avoid segfault when ptls->safepoint is NULL
Browse files Browse the repository at this point in the history
There are cases (eg: when __gmpz_init is called from python via PyCall in a
secondary thread) where ptls->safepoint is NULL when maybe_collect is called,
and *ptls->safepoint causes a NULL pointer dereference and a segfault.  This
change fixes that case.

Included test in test/threads.jl that will reproduce this issue.

See #27020 for backtrace and more details
  • Loading branch information
bluesmoon authored and benlorenz committed Jun 6, 2019
1 parent 9a3e1c7 commit 54e31f6
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 7 deletions.
21 changes: 15 additions & 6 deletions src/gc.c
Original file line number Diff line number Diff line change
Expand Up @@ -2998,23 +2998,29 @@ JL_DLLEXPORT void jl_throw_out_of_memory_error(void)
JL_DLLEXPORT void *jl_gc_counted_malloc(size_t sz)
{
jl_ptls_t ptls = jl_get_ptls_states();
maybe_collect(ptls);
if (ptls && ptls->safepoint) {
maybe_collect(ptls);
}
gc_num.allocd += sz;
gc_num.malloc++;
void *b = malloc(sz);
if (b == NULL)
// If ptls->safepoint is NULL, then let the caller deal with the failed malloc
if (b == NULL && ptls && ptls->safepoint)
jl_throw(jl_memory_exception);
return b;
}

JL_DLLEXPORT void *jl_gc_counted_calloc(size_t nm, size_t sz)
{
jl_ptls_t ptls = jl_get_ptls_states();
maybe_collect(ptls);
if (ptls && ptls->safepoint) {
maybe_collect(ptls);
}
gc_num.allocd += nm*sz;
gc_num.malloc++;
void *b = calloc(nm, sz);
if (b == NULL)
// If ptls->safepoint is NULL, then let the caller deal with the failed calloc
if (b == NULL && ptls && ptls->safepoint)
jl_throw(jl_memory_exception);
return b;
}
Expand All @@ -3035,14 +3041,17 @@ JL_DLLEXPORT void jl_gc_counted_free(void *p, size_t sz)
JL_DLLEXPORT void *jl_gc_counted_realloc_with_old_size(void *p, size_t old, size_t sz)
{
jl_ptls_t ptls = jl_get_ptls_states();
maybe_collect(ptls);
if (ptls && ptls->safepoint) {
maybe_collect(ptls);
}
if (sz < old)
gc_num.freed += (old - sz);
else
gc_num.allocd += (sz - old);
gc_num.realloc++;
void *b = realloc(p, sz);
if (b == NULL)
// If ptls->safepoint is NULL, then let the caller deal with the failed realloc
if (b == NULL && ptls && ptls->safepoint)
jl_throw(jl_memory_exception);
return b;
}
Expand Down
2 changes: 1 addition & 1 deletion src/julia_threads.h
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ JL_DLLEXPORT void (jl_cpu_wake)(void);

// gc safepoint and gc states
// This triggers a SegFault when we are in GC
// Assign it to a variable to make sure the compiler emit the load
// Assign it to a variable to make sure the compiler emits the load
// and to avoid Clang warning for -Wunused-volatile-lvalue
#define jl_gc_safepoint_(ptls) do { \
jl_signal_fence(); \
Expand Down
7 changes: 7 additions & 0 deletions test/threads.jl
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,13 @@ function test_thread_too_few_iters()
end
test_thread_too_few_iters()

function test_27020_SIGSEGV()
x = BigInt()
# We only care that this call does not throw a SIGSEGV, so no need to check return value
@threadcall((:__gmpz_init, :libgmp), Cvoid, (Ptr{BigInt}, ), Ref(x))
end
test_27020_SIGSEGV()

let e = Event(), started = Event()
done = false
t = @async (notify(started); wait(e); done = true)
Expand Down

0 comments on commit 54e31f6

Please sign in to comment.