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

Enable analyzegc checks for try catch and fix found issues #53527

Merged
merged 9 commits into from
Mar 15, 2024
2 changes: 1 addition & 1 deletion Make.inc
Original file line number Diff line number Diff line change
Expand Up @@ -1487,7 +1487,7 @@ endif
CLANGSA_FLAGS :=
CLANGSA_CXXFLAGS :=
ifeq ($(OS), Darwin) # on new XCode, the files are hidden
CLANGSA_FLAGS += -isysroot $(shell xcode-select -p)/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk
CLANGSA_FLAGS += -isysroot $(shell xcrun --show-sdk-path -sdk macosx)
endif
ifeq ($(USEGCC),1)
# try to help clang find the c++ files for CC by guessing the value for --prefix
Expand Down
1 change: 1 addition & 0 deletions src/jl_exported_funcs.inc
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@
XX(jl_egal__bits) \
XX(jl_egal__bitstag) \
XX(jl_eh_restore_state) \
XX(jl_eh_restore_state_noexcept) \
XX(jl_enter_handler) \
XX(jl_enter_threaded_region) \
XX(jl_environ) \
Expand Down
2 changes: 1 addition & 1 deletion src/jloptions.c
Original file line number Diff line number Diff line change
Expand Up @@ -859,7 +859,7 @@ JL_DLLEXPORT void jl_parse_opts(int *argcp, char ***argvp)
if (isnan(sz) || sz < 0) {
jl_errorf("julia: invalid argument to --heap-size-hint (%s)", optarg);
}
jl_options.heap_size_hint = sz < UINT64_MAX ? (uint64_t)sz : UINT64_MAX;
jl_options.heap_size_hint = (uint64_t)sz < UINT64_MAX ? (uint64_t)sz : UINT64_MAX;
gbaraldi marked this conversation as resolved.
Show resolved Hide resolved
}
if (jl_options.heap_size_hint == 0)
jl_errorf("julia: invalid memory size specified in --heap-size-hint");
Expand Down
2 changes: 1 addition & 1 deletion src/jltypes.c
Original file line number Diff line number Diff line change
Expand Up @@ -1846,7 +1846,7 @@ static jl_value_t *normalize_unionalls(jl_value_t *t)
else if (jl_is_unionall(t)) {
jl_unionall_t *u = (jl_unionall_t*)t;
jl_value_t *body = normalize_unionalls(u->body);
JL_GC_PUSH1(&body);
JL_GC_PUSH2(&body, &t);
if (body != u->body) {
t = jl_new_struct(jl_unionall_type, u->var, body);
u = (jl_unionall_t*)t;
Expand Down
27 changes: 19 additions & 8 deletions src/julia.h
Original file line number Diff line number Diff line change
Expand Up @@ -2282,8 +2282,9 @@ extern JL_DLLIMPORT int jl_task_ptls_offset;

#include "julia_locks.h" // requires jl_task_t definition

JL_DLLEXPORT void jl_enter_handler(jl_handler_t *eh);
JL_DLLEXPORT void jl_enter_handler(jl_handler_t *eh) JL_NOTSAFEPOINT ;
JL_DLLEXPORT void jl_eh_restore_state(jl_handler_t *eh);
JL_DLLEXPORT void jl_eh_restore_state_noexcept(jl_handler_t *eh) JL_NOTSAFEPOINT;
JL_DLLEXPORT void jl_pop_handler(int n);
JL_DLLEXPORT size_t jl_excstack_state(void) JL_NOTSAFEPOINT;
JL_DLLEXPORT void jl_restore_excstack(size_t state) JL_NOTSAFEPOINT;
Expand Down Expand Up @@ -2337,24 +2338,34 @@ extern void (*real_siglongjmp)(jmp_buf _Buf, int _Value);

#ifdef __clang_gcanalyzer__

// This is hard. Ideally we'd teach the static analyzer about the extra control
// flow edges. But for now, just hide this as best we can
extern int had_exception;
#define JL_TRY if (1)
#define JL_CATCH if (had_exception)

// The analyzer assumes that the TRY block always executes to completion.
// This can lead to both false positives and false negatives, since it doesn't model the fact that throwing always leaves the try block early.
#define JL_TRY \
int i__try, i__catch; jl_handler_t __eh; \
size_t __excstack_state = jl_excstack_state(); \
jl_enter_handler(&__eh); \
if (1)
/* TRY BLOCK; */
#define JL_CATCH \
if (!had_exception) \
jl_eh_restore_state_noexcept(&__eh); \
else \
for (i__catch=1, jl_eh_restore_state(&__eh); i__catch; i__catch=0, /* CATCH BLOCK; */ jl_restore_excstack(__excstack_state))

#else

#define JL_TRY \
int i__tr, i__ca; jl_handler_t __eh; \
int i__try, i__catch; jl_handler_t __eh; \
size_t __excstack_state = jl_excstack_state(); \
jl_enter_handler(&__eh); \
if (!jl_setjmp(__eh.eh_ctx,0)) \
for (i__tr=1; i__tr; i__tr=0, jl_eh_restore_state(&__eh))
for (i__try=1; i__try; i__try=0, /* TRY BLOCK; */ jl_eh_restore_state_noexcept(&__eh))

#define JL_CATCH \
else \
for (i__ca=1, jl_eh_restore_state(&__eh); i__ca; i__ca=0, jl_restore_excstack(__excstack_state))
for (i__catch=1, jl_eh_restore_state(&__eh); i__catch; i__catch=0, /* CATCH BLOCK; */ jl_restore_excstack(__excstack_state))

#endif

Expand Down
1 change: 1 addition & 0 deletions src/method.c
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,7 @@ static jl_value_t *resolve_globals(jl_value_t *expr, jl_module_t *module, jl_sve
val = jl_interpret_toplevel_expr_in(module, (jl_value_t*)e, NULL, sparam_vals);
}
JL_CATCH {
val = NULL; // To make the analyzer happy see #define JL_TRY
}
if (val)
return val;
Expand Down
10 changes: 10 additions & 0 deletions src/rtutils.c
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,16 @@ JL_DLLEXPORT void jl_eh_restore_state(jl_handler_t *eh)
}
}

JL_DLLEXPORT void jl_eh_restore_state_noexcept(jl_handler_t *eh)
{
jl_task_t *ct = jl_current_task;
ct->eh = eh->prev;
ct->gcstack = eh->gcstack;
ct->world_age = eh->world_age;
ct->ptls->defer_signal = eh->defer_signal;
return;
vtjnash marked this conversation as resolved.
Show resolved Hide resolved
}

JL_DLLEXPORT void jl_pop_handler(int n)
vtjnash marked this conversation as resolved.
Show resolved Hide resolved
{
jl_task_t *ct = jl_current_task;
Expand Down
6 changes: 3 additions & 3 deletions src/signals-mach.c
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ static void jl_mach_gc_wait(jl_ptls_t ptls2, mach_port_t thread, int16_t tid)
// Eventually, we should probably release this signal to the original
// thread, (return KERN_FAILURE instead of KERN_SUCCESS) so that it
// triggers a SIGSEGV and gets handled by the usual codepath for unix.
int8_t gc_state = ptls2->gc_state;
int8_t gc_state = jl_atomic_load_acquire(&ptls2->gc_state);
jl_atomic_store_release(&ptls2->gc_state, JL_GC_STATE_WAITING);
uintptr_t item = tid | (((uintptr_t)gc_state) << 16);
arraylist_push(&suspended_threads, (void*)item);
Expand Down Expand Up @@ -338,7 +338,7 @@ kern_return_t catch_mach_exception_raise(
// jl_throw_in_thread(ptls2, thread, jl_stackovf_exception);
// return KERN_SUCCESS;
// }
if (ptls2->gc_state == JL_GC_STATE_WAITING)
if (jl_atomic_load_acquire(&ptls2->gc_state) == JL_GC_STATE_WAITING)
return KERN_FAILURE;
if (exception == EXC_ARITHMETIC) {
jl_throw_in_thread(ptls2, thread, jl_diverror_exception);
Expand All @@ -363,7 +363,7 @@ kern_return_t catch_mach_exception_raise(
}
return KERN_SUCCESS;
}
if (ptls2->current_task->eh == NULL)
if (jl_atomic_load_relaxed(&ptls2->current_task)->eh == NULL)
return KERN_FAILURE;
jl_value_t *excpt;
if (is_addr_on_stack(jl_atomic_load_relaxed(&ptls2->current_task), (void*)fault_addr)) {
Expand Down