From f613ee3cfb9c045f95ac1a2eb5b9b5027749bfff Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Wed, 29 May 2024 17:49:49 -0400 Subject: [PATCH 1/2] only install error frame if fully allocated and valid Avoids a race condition where a signal (e.g. StackOverflowError) happens while trying to initialize the rest of the frame, resulting in trying to longjmp to garbage. --- src/codegen.cpp | 15 ++++++++++--- src/interpreter.c | 2 ++ src/julia.h | 3 ++- src/llvm-lower-handlers.cpp | 23 ++++++++++++++++++-- src/rtutils.c | 1 - test/llvmpasses/lower-handlers-addrspaces.ll | 5 +++-- test/llvmpasses/lower-handlers.ll | 7 ++++-- 7 files changed, 45 insertions(+), 11 deletions(-) diff --git a/src/codegen.cpp b/src/codegen.cpp index dc9bccb7b4fa0..b471e302507e7 100644 --- a/src/codegen.cpp +++ b/src/codegen.cpp @@ -1487,7 +1487,8 @@ static const auto except_enter_func = new JuliaFunction<>{ "julia.except_enter", [](LLVMContext &C) { auto T_pjlvalue = JuliaType::get_pjlvalue_ty(C); - return FunctionType::get(getInt32Ty(C), {T_pjlvalue}, false); }, + auto RT = StructType::get(getInt32Ty(C), getInt8PtrTy(C)); + return FunctionType::get(RT, {T_pjlvalue}, false); }, [](LLVMContext &C) { return AttributeList::get(C, Attributes(C, {Attribute::ReturnsTwice}), AttributeSet(), @@ -9377,10 +9378,12 @@ static jl_llvm_functions_t 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), {get_current_task(ctx)}); + auto ct = get_current_task(ctx); + CallInst *sj = ctx.builder.CreateCall(prepare_call(except_enter_func), {ct}); // 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)); + Value *isz = ctx.builder.CreateICmpEQ(ctx.builder.CreateExtractValue(sj, 0), ConstantInt::get(getInt32Ty(ctx.builder.getContext()), 0)); + Value *ehbuf = ctx.builder.CreateExtractValue(sj, 1); BasicBlock *tryblk = BasicBlock::Create(ctx.builder.getContext(), "try", f); BasicBlock *catchpop = BasicBlock::Create(ctx.builder.getContext(), "catch_pop", f); BasicBlock *handlr = NULL; @@ -9398,6 +9401,12 @@ static jl_llvm_functions_t ctx.builder.CreateBr(handlr); } ctx.builder.SetInsertPoint(tryblk); + auto ehptr = ctx.builder.CreateInBoundsGEP( + ctx.types().T_ptr, + emit_bitcast(ctx, ct, ctx.types().T_ppint8), + ConstantInt::get(ctx.types().T_size, offsetof(jl_task_t, eh) / ctx.types().sizeof_ptr), + "eh"); + ctx.builder.CreateAlignedStore(ehbuf, ehptr, ctx.types().alignof_ptr); } } else { diff --git a/src/interpreter.c b/src/interpreter.c index d5d440ae289c8..7b67be3063e7d 100644 --- a/src/interpreter.c +++ b/src/interpreter.c @@ -529,6 +529,7 @@ static jl_value_t *eval_body(jl_array_t *stmts, interpreter_state *s, size_t ip, jl_value_t *new_scope = eval_value(jl_enternode_scope(stmt), s); ct->scope = new_scope; if (!jl_setjmp(__eh.eh_ctx, 1)) { + ct->eh = &__eh; eval_body(stmts, s, next_ip, toplevel); jl_unreachable(); } @@ -537,6 +538,7 @@ static jl_value_t *eval_body(jl_array_t *stmts, interpreter_state *s, size_t ip, } else { if (!jl_setjmp(__eh.eh_ctx, 1)) { + ct->eh = &__eh; eval_body(stmts, s, next_ip, toplevel); jl_unreachable(); } diff --git a/src/julia.h b/src/julia.h index 0d46f15776610..4e6f1fde882d9 100644 --- a/src/julia.h +++ b/src/julia.h @@ -2374,6 +2374,7 @@ extern int had_exception; __eh_ct = jl_current_task; \ size_t __excstack_state = jl_excstack_state(__eh_ct); \ jl_enter_handler(__eh_ct, &__eh); \ + __eh_ct->eh = &__eh; \ if (1) /* TRY BLOCK; */ #define JL_CATCH \ @@ -2390,7 +2391,7 @@ extern int had_exception; size_t __excstack_state = jl_excstack_state(__eh_ct); \ jl_enter_handler(__eh_ct, &__eh); \ if (!jl_setjmp(__eh.eh_ctx, 0)) \ - for (i__try=1; i__try; i__try=0, /* TRY BLOCK; */ jl_eh_restore_state_noexcept(__eh_ct, &__eh)) + for (i__try=1, __eh_ct->eh = &__eh; i__try; i__try=0, /* TRY BLOCK; */ jl_eh_restore_state_noexcept(__eh_ct, &__eh)) #define JL_CATCH \ else \ diff --git a/src/llvm-lower-handlers.cpp b/src/llvm-lower-handlers.cpp index 0c4edd5fd00c0..35ce322ad3a71 100644 --- a/src/llvm-lower-handlers.cpp +++ b/src/llvm-lower-handlers.cpp @@ -41,13 +41,14 @@ using namespace llvm; /* Lowers Julia Exception Handlers and colors EH frames. * * Our task is to lower: - * call void @julia.except_enter(ct) + * call {i32, ptr} @julia.except_enter(ct) * <...> * call void jl_pop_handler(1) * * to * * call void @jl_enter_handler(ct, jl_handler *%buff) + * call i32 @jl_setjmp(jmpbuf[] %buff, 0) * <...> * call void jl_pop_handler(1) * @@ -209,7 +210,25 @@ static bool lowerExcHandlers(Function &F) { new_enter->setMetadata(LLVMContext::MD_dbg, dbg); sj->setMetadata(LLVMContext::MD_dbg, dbg); } - enter->replaceAllUsesWith(sj); + SmallVector ToErase; + for (auto *U : enter->users()) { + if (auto *EEI = dyn_cast(U)) { + if (EEI->getNumIndices() == 1) { + if (EEI->getIndices()[0] == 0) + EEI->replaceAllUsesWith(sj); + else + EEI->replaceAllUsesWith(buff); + ToErase.push_back(EEI); + } + } + } + for (auto *EEI : ToErase) + EEI->eraseFromParent(); + if (!enter->use_empty()) { + Value *agg = InsertValueInst::Create(UndefValue::get(enter->getType()), sj, ArrayRef(0), "", enter); + agg = InsertValueInst::Create(agg, buff, ArrayRef(1), "", enter); + enter->replaceAllUsesWith(agg); + } enter->eraseFromParent(); } // Insert lifetime end intrinsics after every leave. diff --git a/src/rtutils.c b/src/rtutils.c index f18a1ac11291a..7df3230755e63 100644 --- a/src/rtutils.c +++ b/src/rtutils.c @@ -248,7 +248,6 @@ JL_DLLEXPORT void jl_enter_handler(jl_task_t *ct, jl_handler_t *eh) eh->locks_len = ct->ptls->locks.len; eh->defer_signal = ct->ptls->defer_signal; eh->world_age = ct->world_age; - ct->eh = eh; #ifdef ENABLE_TIMINGS eh->timing_stack = ct->ptls->timing_stack; #endif diff --git a/test/llvmpasses/lower-handlers-addrspaces.ll b/test/llvmpasses/lower-handlers-addrspaces.ll index 9aa5e6651da05..ce3bdc6401b91 100644 --- a/test/llvmpasses/lower-handlers-addrspaces.ll +++ b/test/llvmpasses/lower-handlers-addrspaces.ll @@ -6,7 +6,7 @@ target triple = "amdgcn-amd-amdhsa" target datalayout = "e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32-p6:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S32-A5-G1-ni:7-ni:10:11:12:13" attributes #1 = { returns_twice } -declare i32 @julia.except_enter({}*) #1 +declare {i32, i8*} @julia.except_enter({}*) #1 declare void @ijl_pop_handler({}*, i32) declare i8**** @julia.ptls_states() declare i8**** @julia.get_pgcstack() @@ -17,7 +17,8 @@ top: ; CHECK: call void @llvm.lifetime.start ; CHECK: call void @ijl_enter_handler ; CHECK: setjmp - %r = call i32 @julia.except_enter({}* null) + %rb = call {i32, i8*} @julia.except_enter({}* null) + %r = extractvalue {i32, i8*} %rb, 0 %cmp = icmp eq i32 %r, 0 br i1 %cmp, label %try, label %catch try: diff --git a/test/llvmpasses/lower-handlers.ll b/test/llvmpasses/lower-handlers.ll index 5d58e9d40e98f..7f0648a1a8bf5 100644 --- a/test/llvmpasses/lower-handlers.ll +++ b/test/llvmpasses/lower-handlers.ll @@ -3,7 +3,7 @@ ; RUN: opt --load-pass-plugin=libjulia-codegen%shlibext -passes='function(LowerExcHandlers)' -S %s | FileCheck %s attributes #1 = { returns_twice } -declare i32 @julia.except_enter({}*) #1 +declare {i32, i8*} @julia.except_enter({}*) #1 declare void @ijl_pop_handler({}*, i32) declare i8**** @julia.ptls_states() declare i8**** @julia.get_pgcstack() @@ -14,10 +14,13 @@ top: ; CHECK: call void @llvm.lifetime.start ; CHECK: call void @ijl_enter_handler ; CHECK: setjmp - %r = call i32 @julia.except_enter({}* null) + %rb = call {i32, i8*} @julia.except_enter({}* null) + %r = extractvalue {i32, i8*} %rb, 0 + %b = extractvalue {i32, i8*} %rb, 1 %cmp = icmp eq i32 %r, 0 br i1 %cmp, label %try, label %catch try: + %lcssa = phi {i32, i8*} [ %rb, %top ] br label %after catch: br label %after From 21b1e15b8f0433239005edf5f3758d78b89c6595 Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Wed, 29 May 2024 14:01:49 -0400 Subject: [PATCH 2/2] add unconditional warning before handling StackOverflow Since this is not modeled by the exception logic, and it can interrupt arbitrary program state or corrupt locks (leading to hangs and other issues), as well as just frequently segfaulting afterwards, give a printed message as soon as we notice things are going badly before attempting to recover. --- src/signal-handling.c | 5 +++++ src/signals-mach.c | 3 ++- src/signals-unix.c | 1 + src/signals-win.c | 1 + 4 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/signal-handling.c b/src/signal-handling.c index 2ddbf2ad1cc8e..3b8d98fbee588 100644 --- a/src/signal-handling.c +++ b/src/signal-handling.c @@ -309,6 +309,11 @@ static void jl_check_profile_autostop(void) } } +static void stack_overflow_warning(void) +{ + jl_safe_printf("Warning: detected a stack overflow; program state may be corrupted, so further execution might be unreliable.\n"); +} + #if defined(_WIN32) #include "signals-win.c" #else diff --git a/src/signals-mach.c b/src/signals-mach.c index 2191dc3268721..7bb110f1639da 100644 --- a/src/signals-mach.c +++ b/src/signals-mach.c @@ -353,7 +353,7 @@ kern_return_t catch_mach_exception_raise( // XXX: jl_throw_in_thread or segv_handler will eventually check this, but // we would like to avoid some of this work if we could detect this earlier // if (jl_has_safe_restore(ptls2)) { - // jl_throw_in_thread(ptls2, thread, jl_stackovf_exception); + // jl_throw_in_thread(ptls2, thread, NULL); // return KERN_SUCCESS; // } if (jl_atomic_load_acquire(&ptls2->gc_state) == JL_GC_STATE_WAITING) @@ -385,6 +385,7 @@ kern_return_t catch_mach_exception_raise( return KERN_FAILURE; jl_value_t *excpt; if (is_addr_on_stack(jl_atomic_load_relaxed(&ptls2->current_task), (void*)fault_addr)) { + stack_overflow_warning(); excpt = jl_stackovf_exception; } else if (is_write_fault(exc_state)) // false for alignment errors diff --git a/src/signals-unix.c b/src/signals-unix.c index 91c47421669f2..eedc5f0aae94a 100644 --- a/src/signals-unix.c +++ b/src/signals-unix.c @@ -404,6 +404,7 @@ JL_NO_ASAN static void segv_handler(int sig, siginfo_t *info, void *context) if (ct->eh == NULL) sigdie_handler(sig, info, context); if ((sig != SIGBUS || info->si_code == BUS_ADRERR) && is_addr_on_stack(ct, info->si_addr)) { // stack overflow and not a BUS_ADRALN (alignment error) + stack_overflow_warning(); jl_throw_in_ctx(ct, jl_stackovf_exception, sig, context); } else if (jl_is_on_sigstack(ct->ptls, info->si_addr, context)) { diff --git a/src/signals-win.c b/src/signals-win.c index f763b71e1cf32..aca20a45dd2ce 100644 --- a/src/signals-win.c +++ b/src/signals-win.c @@ -244,6 +244,7 @@ LONG WINAPI jl_exception_handler(struct _EXCEPTION_POINTERS *ExceptionInfo) case EXCEPTION_STACK_OVERFLOW: if (ct->eh != NULL) { ptls->needs_resetstkoflw = 1; + stack_overflow_warning(); jl_throw_in_ctx(ct, jl_stackovf_exception, ExceptionInfo->ContextRecord); return EXCEPTION_CONTINUE_EXECUTION; }