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 Jul 23, 2019
1 parent f2513a8 commit 4af4411
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 18 deletions.
45 changes: 28 additions & 17 deletions src/gc.c
Original file line number Diff line number Diff line change
Expand Up @@ -3034,23 +3034,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);
ptls->gc_num.allocd += sz;
ptls->gc_num.malloc++;
if (ptls && ptls->safepoint) {
maybe_collect(ptls);
ptls->gc_num.allocd += sz;
ptls->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);
ptls->gc_num.allocd += nm*sz;
ptls->gc_num.malloc++;
if (ptls && ptls->safepoint) {
maybe_collect(ptls);
ptls->gc_num.allocd += nm*sz;
ptls->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 @@ -3059,8 +3065,10 @@ JL_DLLEXPORT void jl_gc_counted_free_with_size(void *p, size_t sz)
{
jl_ptls_t ptls = jl_get_ptls_states();
free(p);
ptls->gc_num.freed += sz;
ptls->gc_num.freecall++;
if (ptls && ptls->safepoint) {
ptls->gc_num.freed += sz;
ptls->gc_num.freecall++;
}
}

// older name for jl_gc_counted_free_with_size
Expand All @@ -3072,14 +3080,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 (sz < old)
ptls->gc_num.freed += (old - sz);
else
ptls->gc_num.allocd += (sz - old);
ptls->gc_num.realloc++;
if (ptls && ptls->safepoint) {
maybe_collect(ptls);
if (sz < old)
ptls->gc_num.freed += (old - sz);
else
ptls->gc_num.allocd += (sz - old);
ptls->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 @@ -240,7 +240,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
6 changes: 6 additions & 0 deletions test/threads_exec.jl
Original file line number Diff line number Diff line change
Expand Up @@ -652,6 +652,12 @@ function pfib(n::Int)
end
@test pfib(20) == 6765

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()

# scheduling wake/sleep test (#32511)
let timeout = 300 # this test should take about 1-10 seconds
Expand Down

0 comments on commit 4af4411

Please sign in to comment.