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
29 changes: 27 additions & 2 deletions src/codegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1061,7 +1061,31 @@ static const auto jlleave_func = new JuliaFunction<>{
XSTR(jl_pop_handler),
[](LLVMContext &C) { return FunctionType::get(getVoidTy(C),
{getInt32Ty(C)}, false); },
nullptr,
[](LLVMContext &C) {
auto FnAttrs = AttrBuilder(C);
FnAttrs.addAttribute(Attribute::WillReturn);
FnAttrs.addAttribute(Attribute::NoUnwind);
auto RetAttrs = AttrBuilder(C);
return AttributeList::get(C,
AttributeSet::get(C, FnAttrs),
AttributeSet(),
None);
},
};
static const auto jlleave_noexcept_func = new JuliaFunction<>{
XSTR(jl_pop_handler_noexcept),
[](LLVMContext &C) { return FunctionType::get(getVoidTy(C),
{getInt32Ty(C)}, false); },
[](LLVMContext &C) {
auto FnAttrs = AttrBuilder(C);
FnAttrs.addAttribute(Attribute::WillReturn);
FnAttrs.addAttribute(Attribute::NoUnwind);
auto RetAttrs = AttrBuilder(C);
return AttributeList::get(C,
AttributeSet::get(C, FnAttrs),
AttributeSet(),
None);
},
};
static const auto jl_restore_excstack_func = new JuliaFunction<TypeFnContextAndSizeT>{
XSTR(jl_restore_excstack),
Expand Down Expand Up @@ -5961,7 +5985,7 @@ static void emit_stmtpos(jl_codectx_t &ctx, jl_value_t *expr, int ssaval_result)
hand_n_leave += 1;
}
}
ctx.builder.CreateCall(prepare_call(jlleave_func),
ctx.builder.CreateCall(prepare_call(jlleave_noexcept_func),
ConstantInt::get(getInt32Ty(ctx.builder.getContext()), hand_n_leave));
if (scope_to_restore) {
jl_aliasinfo_t scope_ai = jl_aliasinfo_t::fromTBAA(ctx, ctx.tbaa().tbaa_gcframe);
Expand Down Expand Up @@ -9800,6 +9824,7 @@ static void init_jit_functions(void)
add_named_global(jlgenericfunction_func, &jl_generic_function_def);
add_named_global(jlenter_func, &jl_enter_handler);
add_named_global(jl_current_exception_func, &jl_current_exception);
add_named_global(jlleave_noexcept_func, &jl_pop_handler_noexcept);
add_named_global(jlleave_func, &jl_pop_handler);
add_named_global(jl_restore_excstack_func, &jl_restore_excstack);
add_named_global(jl_excstack_state_func, &jl_excstack_state);
Expand Down
4 changes: 3 additions & 1 deletion src/interpreter.c
Original file line number Diff line number Diff line change
Expand Up @@ -540,13 +540,15 @@ static jl_value_t *eval_body(jl_array_t *stmts, interpreter_state *s, size_t ip,
jl_unreachable();
}
}
jl_eh_restore_state(&__eh);

if (s->continue_at) { // means we reached a :leave expression
jl_eh_restore_state_noexcept(&__eh);
ip = s->continue_at;
s->continue_at = 0;
continue;
}
else { // a real exception
jl_eh_restore_state(&__eh);
ip = catch_ip;
assert(jl_enternode_catch_dest(stmt) != 0);
continue;
Expand Down
2 changes: 2 additions & 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 Expand Up @@ -366,6 +367,7 @@
XX(jl_pointerref) \
XX(jl_pointerset) \
XX(jl_pop_handler) \
XX(jl_pop_handler_noexcept) \
XX(jl_preload_sysimg_so) \
XX(jl_prepend_cwd) \
XX(jl_printf) \
Expand Down
3 changes: 2 additions & 1 deletion src/jloptions.c
Original file line number Diff line number Diff line change
Expand Up @@ -859,7 +859,8 @@ 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;
const long double limit = ldexpl(1.0, 64); // UINT64_MAX + 1
jl_options.heap_size_hint = sz < limit ? (uint64_t)sz : UINT64_MAX;
}
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
30 changes: 21 additions & 9 deletions src/julia.h
Original file line number Diff line number Diff line change
Expand Up @@ -2282,9 +2282,11 @@ 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_pop_handler(int n);
JL_DLLEXPORT void jl_eh_restore_state_noexcept(jl_handler_t *eh) JL_NOTSAFEPOINT;
JL_DLLEXPORT void jl_pop_handler(int n) JL_NOTSAFEPOINT;
JL_DLLEXPORT void jl_pop_handler_noexcept(int n) JL_NOTSAFEPOINT;
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 +2339,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
2 changes: 1 addition & 1 deletion src/llvm-late-gc-lowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1621,7 +1621,7 @@ State LateLowerGCFrame::LocalScan(Function &F) {
callee == pgcstack_getter || callee->getName() == XSTR(jl_egal__unboxed) ||
callee->getName() == XSTR(jl_lock_value) || callee->getName() == XSTR(jl_unlock_value) ||
callee->getName() == XSTR(jl_lock_field) || callee->getName() == XSTR(jl_unlock_field) ||
callee == write_barrier_func || callee == gc_loaded_func ||
callee == write_barrier_func || callee == gc_loaded_func || callee == pop_handler_noexcept_func ||
callee->getName() == "memcmp") {
continue;
}
Expand Down
3 changes: 2 additions & 1 deletion src/llvm-lower-handlers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ static bool lowerExcHandlers(Function &F) {
return false; // No EH frames in this module
ensure_enter_function(M, TT);
Function *leave_func = M.getFunction(XSTR(jl_pop_handler));
Function *leave_noexcept_func = M.getFunction(XSTR(jl_pop_handler_noexcept));
Function *jlenter_func = M.getFunction(XSTR(jl_enter_handler));
Function *setjmp_func = M.getFunction(jl_setjmp_name);

Expand Down Expand Up @@ -150,7 +151,7 @@ static bool lowerExcHandlers(Function &F) {
continue;
if (Callee == except_enter_func)
EnterDepth[CI] = Depth++;
else if (Callee == leave_func) {
else if (Callee == leave_func || Callee == leave_noexcept_func) {
LeaveDepth[CI] = Depth;
Depth -= cast<ConstantInt>(CI->getArgOperand(0))->getLimitedValue();
}
Expand Down
3 changes: 2 additions & 1 deletion src/llvm-pass-helpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ JuliaPassContext::JuliaPassContext()
pgcstack_getter(nullptr), adoptthread_func(nullptr), gc_flush_func(nullptr),
gc_preserve_begin_func(nullptr), gc_preserve_end_func(nullptr),
pointer_from_objref_func(nullptr), gc_loaded_func(nullptr), alloc_obj_func(nullptr),
typeof_func(nullptr), write_barrier_func(nullptr),
typeof_func(nullptr), write_barrier_func(nullptr), pop_handler_noexcept_func(nullptr),
call_func(nullptr), call2_func(nullptr), call3_func(nullptr), module(nullptr)
{
}
Expand All @@ -53,6 +53,7 @@ void JuliaPassContext::initFunctions(Module &M)
typeof_func = M.getFunction("julia.typeof");
write_barrier_func = M.getFunction("julia.write_barrier");
alloc_obj_func = M.getFunction("julia.gc_alloc_obj");
pop_handler_noexcept_func = M.getFunction(XSTR(jl_pop_handler_noexcept));
call_func = M.getFunction("julia.call");
call2_func = M.getFunction("julia.call2");
call3_func = M.getFunction("julia.call3");
Expand Down
1 change: 1 addition & 0 deletions src/llvm-pass-helpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ struct JuliaPassContext {
llvm::Function *alloc_obj_func;
llvm::Function *typeof_func;
llvm::Function *write_barrier_func;
llvm::Function *pop_handler_noexcept_func;
llvm::Function *call_func;
llvm::Function *call2_func;
llvm::Function *call3_func;
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
21 changes: 21 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 All @@ -308,6 +318,17 @@ JL_DLLEXPORT void jl_pop_handler(int n)
jl_eh_restore_state(eh);
}

JL_DLLEXPORT void jl_pop_handler_noexcept(int n)
{
jl_task_t *ct = jl_current_task;
if (__unlikely(n <= 0))
return;
jl_handler_t *eh = ct->eh;
while (--n > 0)
eh = eh->prev;
jl_eh_restore_state_noexcept(eh);
}

JL_DLLEXPORT size_t jl_excstack_state(void) JL_NOTSAFEPOINT
{
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