Skip to content

Commit

Permalink
Enable analyzegc checks for try catch and fix found issues (#53527)
Browse files Browse the repository at this point in the history 
This PR also makes a successful `JL_TRY` not do so much work + fixes
clang not finding the sdk when running those tests in macos.

Fixes https://github.com/JuliaLang/julia/issues/
Co-authored-by: Cody Tapscott <84105208+topolarity@users.noreply.github.com>
Co-authored-by: Jameson Nash <vtjnash@gmail.com>

(cherry picked from commit 67cdb9b)
  • Loading branch information
@gbaraldi
gbaraldi authored and KristofferC committed Mar 15, 2024
1 parent 2e45f8f commit 2623abb
Show file tree
Hide file tree
Showing 31 changed files with 241 additions and 166 deletions.
2 changes: 1 addition & 1 deletion Make.inc
View file
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
12 changes: 8 additions & 4 deletions base/stream.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -453,9 +453,11 @@ function closewrite(s::LibuvStream)
sigatomic_begin()
uv_req_set_data(req, ct)
iolock_end()
status = try
local status
try
sigatomic_end()
wait()::Cint
status = wait()::Cint
sigatomic_begin()
finally
# try-finally unwinds the sigatomic level, so need to repeat sigatomic_end
sigatomic_end()
Expand Down Expand Up @@ -1061,12 +1063,14 @@ function uv_write(s::LibuvStream, p::Ptr{UInt8}, n::UInt)
sigatomic_begin()
uv_req_set_data(uvw, ct)
iolock_end()
status = try
local status
try
sigatomic_end()
# wait for the last chunk to complete (or error)
# assume that any errors would be sticky,
# (so we don't need to monitor the error status of the intermediate writes)
wait()::Cint
status = wait()::Cint
sigatomic_begin()
finally
# try-finally unwinds the sigatomic level, so need to repeat sigatomic_end
sigatomic_end()
Expand Down
2 changes: 1 addition & 1 deletion doc/src/devdocs/ast.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ These symbols appear in the `head` field of [`Expr`](@ref)s in lowered form.

* `the_exception`

Yields the caught exception inside a `catch` block, as returned by `jl_current_exception()`.
Yields the caught exception inside a `catch` block, as returned by `jl_current_exception(ct)`.

* `enter`

Expand Down
4 changes: 2 additions & 2 deletions src/ast.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,7 @@ static jl_value_t *scm_to_julia(fl_context_t *fl_ctx, value_t e, jl_module_t *mo
}
JL_CATCH {
// if expression cannot be converted, replace with error expr
//jl_(jl_current_exception());
//jl_(jl_current_exception(ct));
//jlbacktrace();
jl_expr_t *ex = jl_exprn(jl_error_sym, 1);
v = (jl_value_t*)ex;
Expand Down Expand Up @@ -1151,7 +1151,7 @@ static jl_value_t *jl_invoke_julia_macro(jl_array_t *args, jl_module_t *inmodule
margs[0] = jl_cstr_to_string("<macrocall>");
margs[1] = jl_fieldref(lno, 0); // extract and allocate line number
jl_rethrow_other(jl_new_struct(jl_loaderror_type, margs[0], margs[1],
jl_current_exception()));
jl_current_exception(ct)));
}
}
ct->world_age = last_age;
Expand Down
2 changes: 1 addition & 1 deletion src/ccall.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1613,7 +1613,7 @@ static jl_cgval_t emit_ccall(jl_codectx_t &ctx, jl_value_t **args, size_t nargs)
assert(lrt == ctx.types().T_prjlvalue);
assert(!isVa && !llvmcall && nccallargs == 0);
JL_GC_POP();
auto ct = track_pjlvalue(ctx, emit_bitcast(ctx, get_current_task(ctx), ctx.types().T_pjlvalue));
auto ct = track_pjlvalue(ctx, get_current_task(ctx));
return mark_or_box_ccall_result(ctx, ct, retboxed, rt, unionall, static_rt);
}
else if (is_libjulia_func(jl_set_next_task)) {
Expand Down
94 changes: 67 additions & 27 deletions src/codegen.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1048,30 +1048,64 @@ static const auto jlunlockfield_func = new JuliaFunction<>{
};
static const auto jlenter_func = new JuliaFunction<>{
XSTR(jl_enter_handler),
[](LLVMContext &C) { return FunctionType::get(getVoidTy(C),
{getInt8PtrTy(C)}, false); },
[](LLVMContext &C) {
auto T_pjlvalue = JuliaType::get_pjlvalue_ty(C);
return FunctionType::get(getVoidTy(C),
{T_pjlvalue, getInt8PtrTy(C)}, false); },
nullptr,
};
static const auto jl_current_exception_func = new JuliaFunction<>{
XSTR(jl_current_exception),
[](LLVMContext &C) { return FunctionType::get(JuliaType::get_prjlvalue_ty(C), false); },
[](LLVMContext &C) { return FunctionType::get(JuliaType::get_prjlvalue_ty(C), {JuliaType::get_pjlvalue_ty(C)}, false); },
nullptr,
};
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 T_pjlvalue = JuliaType::get_pjlvalue_ty(C);
return FunctionType::get(getVoidTy(C),
{T_pjlvalue, 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 jlleave_noexcept_func = new JuliaFunction<>{
XSTR(jl_pop_handler_noexcept),
[](LLVMContext &C) {
auto T_pjlvalue = JuliaType::get_pjlvalue_ty(C);
return FunctionType::get(getVoidTy(C),
{T_pjlvalue, 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),
[](LLVMContext &C, Type *T_size) { return FunctionType::get(getVoidTy(C),
{T_size}, false); },
[](LLVMContext &C, Type *T_size) {
auto T_pjlvalue = JuliaType::get_pjlvalue_ty(C);
return FunctionType::get(getVoidTy(C),
{T_pjlvalue, T_size}, false); },
nullptr,
};
static const auto jl_excstack_state_func = new JuliaFunction<TypeFnContextAndSizeT>{
XSTR(jl_excstack_state),
[](LLVMContext &C, Type *T_size) { return FunctionType::get(T_size, false); },
[](LLVMContext &C, Type *T_size) {
auto T_pjlvalue = JuliaType::get_pjlvalue_ty(C);
return FunctionType::get(T_size, {T_pjlvalue}, false); },
nullptr,
};
static const auto jlegalx_func = new JuliaFunction<TypeFnContextAndSizeT>{
Expand All @@ -1098,9 +1132,9 @@ static const auto jl_alloc_obj_func = new JuliaFunction<TypeFnContextAndSizeT>{
[](LLVMContext &C, Type *T_size) {
auto T_jlvalue = JuliaType::get_jlvalue_ty(C);
auto T_prjlvalue = PointerType::get(T_jlvalue, AddressSpace::Tracked);
auto T_ppjlvalue = PointerType::get(PointerType::get(T_jlvalue, 0), 0);
auto T_pjlvalue = PointerType::get(T_jlvalue, 0);
return FunctionType::get(T_prjlvalue,
{T_ppjlvalue, T_size, T_prjlvalue}, false);
{T_pjlvalue, T_size, T_prjlvalue}, false);
},
[](LLVMContext &C) {
auto FnAttrs = AttrBuilder(C);
Expand Down Expand Up @@ -1442,7 +1476,9 @@ static const auto gc_preserve_end_func = new JuliaFunction<> {
};
static const auto except_enter_func = new JuliaFunction<>{
"julia.except_enter",
[](LLVMContext &C) { return FunctionType::get(getInt32Ty(C), false); },
[](LLVMContext &C) {
auto T_pjlvalue = JuliaType::get_pjlvalue_ty(C);
return FunctionType::get(getInt32Ty(C), {T_pjlvalue}, false); },
[](LLVMContext &C) { return AttributeList::get(C,
Attributes(C, {Attribute::ReturnsTwice}),
AttributeSet(),
Expand Down Expand Up @@ -5957,8 +5993,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),
ConstantInt::get(getInt32Ty(ctx.builder.getContext()), hand_n_leave));
ctx.builder.CreateCall(prepare_call(jlleave_noexcept_func), {get_current_task(ctx), 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);
scope_ai.decorateInst(
Expand All @@ -5968,7 +6003,7 @@ static void emit_stmtpos(jl_codectx_t &ctx, jl_value_t *expr, int ssaval_result)
else if (head == jl_pop_exception_sym) {
jl_cgval_t excstack_state = emit_expr(ctx, jl_exprarg(expr, 0));
assert(excstack_state.V && excstack_state.V->getType() == ctx.types().T_size);
ctx.builder.CreateCall(prepare_call(jl_restore_excstack_func), excstack_state.V);
ctx.builder.CreateCall(prepare_call(jl_restore_excstack_func), {get_current_task(ctx), excstack_state.V});
return;
}
else {
Expand Down Expand Up @@ -6199,7 +6234,7 @@ static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr, ssize_t ssaidx_
bnd = jl_get_binding_for_method_def(mod, (jl_sym_t*)mn);
}
JL_CATCH {
jl_value_t *e = jl_current_exception();
jl_value_t *e = jl_current_exception(jl_current_task);
// errors. boo. :(
JL_GC_PUSH1(&e);
e = jl_as_global_root(e, 1);
Expand Down Expand Up @@ -6375,7 +6410,7 @@ static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr, ssize_t ssaidx_
else if (head == jl_exc_sym) {
assert(nargs == 0);
return mark_julia_type(ctx,
ctx.builder.CreateCall(prepare_call(jl_current_exception_func)),
ctx.builder.CreateCall(prepare_call(jl_current_exception_func), {get_current_task(ctx)}),
true, jl_any_type);
}
else if (head == jl_copyast_sym) {
Expand Down Expand Up @@ -6479,9 +6514,14 @@ static void allocate_gc_frame(jl_codectx_t &ctx, BasicBlock *b0, bool or_new=fal
ctx.pgcstack->setName("pgcstack");
}

static Value *get_current_task(jl_codectx_t &ctx, Type *T)
{
return emit_bitcast(ctx, get_current_task_from_pgcstack(ctx.builder, ctx.types().T_size, ctx.pgcstack), T);
}

static Value *get_current_task(jl_codectx_t &ctx)
{
return get_current_task_from_pgcstack(ctx.builder, ctx.types().T_size, ctx.pgcstack);
return get_current_task(ctx, ctx.types().T_pjlvalue);
}

// Get PTLS through current task.
Expand All @@ -6493,20 +6533,20 @@ static Value *get_current_ptls(jl_codectx_t &ctx)
// Get the address of the world age of the current task
static Value *get_last_age_field(jl_codectx_t &ctx)
{
Value *ct = get_current_task(ctx);
Value *ct = get_current_task(ctx, ctx.types().T_size->getPointerTo());
return ctx.builder.CreateInBoundsGEP(
ctx.types().T_size,
ctx.builder.CreateBitCast(ct, ctx.types().T_size->getPointerTo()),
ct,
ConstantInt::get(ctx.types().T_size, offsetof(jl_task_t, world_age) / ctx.types().sizeof_ptr),
"world_age");
}

static Value *get_scope_field(jl_codectx_t &ctx)
{
Value *ct = get_current_task(ctx);
Value *ct = get_current_task(ctx, ctx.types().T_prjlvalue->getPointerTo());
return ctx.builder.CreateInBoundsGEP(
ctx.types().T_prjlvalue,
ctx.builder.CreateBitCast(ct, ctx.types().T_prjlvalue->getPointerTo()),
ct,
ConstantInt::get(ctx.types().T_size, offsetof(jl_task_t, scope) / ctx.types().sizeof_ptr),
"current_scope");
}
Expand Down Expand Up @@ -9119,12 +9159,12 @@ static jl_llvm_functions_t
if (lname) {
// Save exception stack depth at enter for use in pop_exception
Value *excstack_state =
ctx.builder.CreateCall(prepare_call(jl_excstack_state_func));
ctx.builder.CreateCall(prepare_call(jl_excstack_state_func), {get_current_task(ctx)});
assert(!ctx.ssavalue_assigned[cursor]);
ctx.SAvalues[cursor] = jl_cgval_t(excstack_state, (jl_value_t*)jl_ulong_type, NULL);
ctx.ssavalue_assigned[cursor] = true;
// Actually enter the exception frame
CallInst *sj = ctx.builder.CreateCall(prepare_call(except_enter_func));
CallInst *sj = ctx.builder.CreateCall(prepare_call(except_enter_func), {get_current_task(ctx)});
// We need to mark this on the call site as well. See issue #6757
sj->setCanReturnTwice();
Value *isz = ctx.builder.CreateICmpEQ(sj, ConstantInt::get(getInt32Ty(ctx.builder.getContext()), 0));
Expand All @@ -9137,8 +9177,7 @@ static jl_llvm_functions_t
ctx.builder.CreateCondBr(isz, tryblk, catchpop);
ctx.builder.SetInsertPoint(catchpop);
{
ctx.builder.CreateCall(prepare_call(jlleave_func),
ConstantInt::get(getInt32Ty(ctx.builder.getContext()), 1));
ctx.builder.CreateCall(prepare_call(jlleave_func), {get_current_task(ctx), ConstantInt::get(getInt32Ty(ctx.builder.getContext()), 1)});
if (old_scope) {
scope_ai.decorateInst(
ctx.builder.CreateAlignedStore(old_scope, scope_ptr, ctx.types().alignof_ptr));
Expand Down Expand Up @@ -9504,7 +9543,7 @@ jl_llvm_functions_t jl_emit_code(
decls.functionObject = "";
decls.specFunctionObject = "";
jl_printf((JL_STREAM*)STDERR_FILENO, "Internal error: encountered unexpected error during compilation of %s:\n", mname.c_str());
jl_static_show((JL_STREAM*)STDERR_FILENO, jl_current_exception());
jl_static_show((JL_STREAM*)STDERR_FILENO, jl_current_exception(jl_current_task));
jl_printf((JL_STREAM*)STDERR_FILENO, "\n");
jlbacktrace(); // written to STDERR_FILENO
#ifndef JL_NDEBUG
Expand Down Expand Up @@ -9816,6 +9855,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
2 changes: 1 addition & 1 deletion src/gc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ static void run_finalizer(jl_task_t *ct, void *o, void *ff)
}
JL_CATCH {
jl_printf((JL_STREAM*)STDERR_FILENO, "error in running finalizer: ");
jl_static_show((JL_STREAM*)STDERR_FILENO, jl_current_exception());
jl_static_show((JL_STREAM*)STDERR_FILENO, jl_current_exception(ct));
jl_printf((JL_STREAM*)STDERR_FILENO, "\n");
jlbacktrace(); // written to STDERR_FILENO
}
Expand Down
4 changes: 2 additions & 2 deletions src/gf.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ void jl_call_tracer(tracer_cb callback, jl_value_t *tracee)
JL_CATCH {
ct->ptls->in_pure_callback = last_in;
jl_printf((JL_STREAM*)STDERR_FILENO, "WARNING: tracer callback function threw an error:\n");
jl_static_show((JL_STREAM*)STDERR_FILENO, jl_current_exception());
jl_static_show((JL_STREAM*)STDERR_FILENO, jl_current_exception(ct));
jl_printf((JL_STREAM*)STDERR_FILENO, "\n");
jlbacktrace(); // written to STDERR_FILENO
}
Expand Down Expand Up @@ -390,7 +390,7 @@ jl_code_info_t *jl_type_infer(jl_method_instance_t *mi, size_t world, int force)
src = (jl_code_info_t*)jl_apply(fargs, 3);
}
JL_CATCH {
jl_value_t *e = jl_current_exception();
jl_value_t *e = jl_current_exception(ct);
jl_printf((JL_STREAM*)STDERR_FILENO, "Internal error: during type inference of\n");
jl_static_show_func_sig((JL_STREAM*)STDERR_FILENO, (jl_value_t*)mi->specTypes);
jl_printf((JL_STREAM*)STDERR_FILENO, "\nEncountered ");
Expand Down
6 changes: 3 additions & 3 deletions src/init.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ JL_DLLEXPORT void jl_atexit_hook(int exitcode) JL_NOTSAFEPOINT_ENTER
}
JL_CATCH {
jl_printf((JL_STREAM*)STDERR_FILENO, "\natexit hook threw an error: ");
jl_static_show((JL_STREAM*)STDERR_FILENO, jl_current_exception());
jl_static_show((JL_STREAM*)STDERR_FILENO, jl_current_exception(ct));
jl_printf((JL_STREAM*)STDERR_FILENO, "\n");
jlbacktrace(); // written to STDERR_FILENO
}
Expand Down Expand Up @@ -317,7 +317,7 @@ JL_DLLEXPORT void jl_atexit_hook(int exitcode) JL_NOTSAFEPOINT_ENTER
assert(item);
uv_unref(item->h);
jl_printf((JL_STREAM*)STDERR_FILENO, "error during exit cleanup: close: ");
jl_static_show((JL_STREAM*)STDERR_FILENO, jl_current_exception());
jl_static_show((JL_STREAM*)STDERR_FILENO, jl_current_exception(ct));
jl_printf((JL_STREAM*)STDERR_FILENO, "\n");
jlbacktrace(); // written to STDERR_FILENO
item = next_shutdown_queue_item(item);
Expand Down Expand Up @@ -372,7 +372,7 @@ JL_DLLEXPORT void jl_postoutput_hook(void)
}
JL_CATCH {
jl_printf((JL_STREAM*)STDERR_FILENO, "\npostoutput hook threw an error: ");
jl_static_show((JL_STREAM*)STDERR_FILENO, jl_current_exception());
jl_static_show((JL_STREAM*)STDERR_FILENO, jl_current_exception(ct));
jl_printf((JL_STREAM*)STDERR_FILENO, "\n");
jlbacktrace(); // written to STDERR_FILENO
}
Expand Down
12 changes: 7 additions & 5 deletions src/interpreter.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ static jl_value_t *eval_value(jl_value_t *e, interpreter_state *s)
return jl_copy_ast(eval_value(args[0], s));
}
else if (head == jl_exc_sym) {
return jl_current_exception();
return jl_current_exception(jl_current_task);
}
else if (head == jl_boundscheck_sym) {
return jl_true;
Expand Down Expand Up @@ -490,7 +490,7 @@ static jl_value_t *eval_body(jl_array_t *stmts, interpreter_state *s, size_t ip,
s->locals[jl_source_nslots(s->src) + id] = val;
}
else if (jl_is_enternode(stmt)) {
jl_enter_handler(&__eh);
jl_enter_handler(ct, &__eh);
// This is a bit tricky, but supports the implementation of PhiC nodes.
// They are conceptually slots, but the slot to store to doesn't get explicitly
// mentioned in the store (aka the "UpsilonNode") (this makes them integrate more
Expand Down Expand Up @@ -521,7 +521,7 @@ static jl_value_t *eval_body(jl_array_t *stmts, interpreter_state *s, size_t ip,
}
// store current top of exception stack for restore in pop_exception.
}
s->locals[jl_source_nslots(s->src) + ip] = jl_box_ulong(jl_excstack_state());
s->locals[jl_source_nslots(s->src) + ip] = jl_box_ulong(jl_excstack_state(ct));
if (jl_enternode_scope(stmt)) {
jl_value_t *old_scope = ct->scope;
JL_GC_PUSH1(&old_scope);
Expand All @@ -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(ct, &__eh);
ip = s->continue_at;
s->continue_at = 0;
continue;
}
else { // a real exception
jl_eh_restore_state(ct, &__eh);
ip = catch_ip;
assert(jl_enternode_catch_dest(stmt) != 0);
continue;
Expand Down Expand Up @@ -609,7 +611,7 @@ static jl_value_t *eval_body(jl_array_t *stmts, interpreter_state *s, size_t ip,
}
else if (head == jl_pop_exception_sym) {
size_t prev_state = jl_unbox_ulong(eval_value(jl_exprarg(stmt, 0), s));
jl_restore_excstack(prev_state);
jl_restore_excstack(ct, prev_state);
}
else if (toplevel) {
if (head == jl_method_sym && jl_expr_nargs(stmt) > 1) {
Expand Down
Loading

0 comments on commit 2623abb

Please sign in to comment.