Skip to content

Commit

Permalink
Hide returns_twice function from LLVM passes
Browse files Browse the repository at this point in the history
LLVM has really bad support for returns_twice functions and can incorrectly move memory operations
(both at IR and machine code level) due to missing control flow edge.
By outlining the exception body, we can hide these functions from LLVM completely
(they only exist in C code) and prevent all miscompilation.
This also makes it much easier to check the correctness of heap to stack allocation optimization
especially since not all memory operation intrinsics in LLVM has a volatile counterpart.

This will obviously inhibit some valid optimizations too.
These are mainly forwarding of memory operations from the caller to the exception body
(since the other way around is almost always invalid) and can be improved with some simple IPO.

This also makes it unnecessary to mark any memory operations on the stack with `volatile`
this should also improve optimization in certain cases.

Since we are scanning all the instructions in the outlined code anyway this also includes
a simple optimization to delete exception frame that can't trigger.

This implements a tweaked version of https://discourse.julialang.org/t/avoid-llvm-setjmp-bug/1140

Fix #17288
  • Loading branch information
yuyichao committed Nov 12, 2017
1 parent c8f56e9 commit 952c163
Show file tree
Hide file tree
Showing 15 changed files with 1,184 additions and 456 deletions.
5 changes: 3 additions & 2 deletions src/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,9 @@ LLVMLINK :=

ifeq ($(JULIACODEGEN),LLVM)
SRCS += codegen jitlayers disasm debuginfo llvm-simdloop llvm-ptls llvm-muladd \
llvm-late-gc-lowering llvm-lower-handlers llvm-gc-invariant-verifier \
llvm-propagate-addrspaces llvm-multiversioning llvm-alloc-opt cgmemmgr
llvm-late-gc-lowering llvm-gc-invariant-verifier \
llvm-propagate-addrspaces llvm-multiversioning llvm-alloc-opt cgmemmgr \
llvm-eh-outlining llvm-eh-lowering
FLAGS += -I$(shell $(LLVM_CONFIG_HOST) --includedir)
LLVM_LIBS := all
ifeq ($(USE_POLLY),1)
Expand Down
56 changes: 10 additions & 46 deletions src/codegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -291,16 +291,13 @@ static Function *jlapply2va_func;
static Function *jlgetfield_func;
static Function *jlmethod_func;
static Function *jlgenericfunction_func;
static Function *jlenter_func;
static Function *jlleave_func;
static Function *jlegal_func;
static Function *jl_alloc_obj_func;
static Function *jl_typeof_func;
static Function *jl_write_barrier_func;
static Function *jlisa_func;
static Function *jlsubtype_func;
static Function *jlapplytype_func;
static Function *setjmp_func;
static Function *memcmp_derived_func;
static Function *box_int8_func;
static Function *box_uint8_func;
Expand Down Expand Up @@ -340,6 +337,7 @@ static Function *gcroot_flush_func;
static Function *gc_preserve_begin_func;
static Function *gc_preserve_end_func;
static Function *except_enter_func;
static Function *except_leave_func;
static Function *pointer_from_objref_func;

static std::vector<Type *> two_pvalue_llvmt;
Expand Down Expand Up @@ -3693,8 +3691,10 @@ static void emit_stmtpos(jl_codectx_t &ctx, jl_value_t *expr)
}
else if (head == leave_sym) {
assert(jl_is_long(args[0]));
ctx.builder.CreateCall(prepare_call(jlleave_func),
ConstantInt::get(T_int32, jl_unbox_long(args[0])));
int nlevel = jl_unbox_long(args[0]);
for (int i = 0; i < nlevel; i++) {
ctx.builder.CreateCall(prepare_call(except_leave_func));
}
}
else {
if (!jl_is_method(ctx.linfo->def.method)) {
Expand Down Expand Up @@ -5789,21 +5789,7 @@ static std::unique_ptr<Module> emit_function(
Value *isz = ctx.builder.CreateICmpEQ(sj, ConstantInt::get(T_int32, 0));
BasicBlock *tryblk = BasicBlock::Create(jl_LLVMContext, "try", f);
BasicBlock *handlr = handle_label(lname, false);
#ifdef _OS_WINDOWS_
BasicBlock *cond_resetstkoflw_blk = BasicBlock::Create(jl_LLVMContext, "cond_resetstkoflw", f);
BasicBlock *resetstkoflw_blk = BasicBlock::Create(jl_LLVMContext, "resetstkoflw", f);
ctx.builder.CreateCondBr(isz, tryblk, cond_resetstkoflw_blk);
ctx.builder.SetInsertPoint(cond_resetstkoflw_blk);
ctx.builder.CreateCondBr(ctx.builder.CreateICmpEQ(
maybe_decay_untracked(literal_pointer_val(ctx, jl_stackovf_exception)),
ctx.builder.CreateLoad(emit_exc_in_transit(ctx), /*isvolatile*/true)),
resetstkoflw_blk, handlr);
ctx.builder.SetInsertPoint(resetstkoflw_blk);
ctx.builder.CreateCall(prepare_call(resetstkoflw_func), {});
ctx.builder.CreateBr(handlr);
#else
ctx.builder.CreateCondBr(isz, tryblk, handlr);
#endif
ctx.builder.SetInsertPoint(tryblk);
}
else {
Expand Down Expand Up @@ -6227,17 +6213,6 @@ static void init_julia_llvm_env(Module *m)
jlnew_func->addFnAttr("thunk");
add_named_global(jlnew_func, &jl_new_structv);

std::vector<Type*> args2(0);
args2.push_back(T_pint8);
#ifndef _OS_WINDOWS_
args2.push_back(T_int32);
#endif
setjmp_func =
Function::Create(FunctionType::get(T_int32, args2, false),
Function::ExternalLinkage, jl_setjmp_name, m);
setjmp_func->addFnAttr(Attribute::ReturnsTwice);
add_named_global(setjmp_func, &jl_setjmp_f);

std::vector<Type*> args_memcmp(0);
args_memcmp.push_back(T_pint8_derived);
args_memcmp.push_back(T_pint8_derived);
Expand Down Expand Up @@ -6406,14 +6381,6 @@ static void init_julia_llvm_env(Module *m)
"jl_generic_function_def", m);
add_named_global(jlgenericfunction_func, &jl_generic_function_def);

std::vector<Type*> ehargs(0);
ehargs.push_back(T_pint8);
jlenter_func =
Function::Create(FunctionType::get(T_void, ehargs, false),
Function::ExternalLinkage,
"jl_enter_handler", m);
add_named_global(jlenter_func, &jl_enter_handler);

#ifdef _OS_WINDOWS_
resetstkoflw_func = Function::Create(FunctionType::get(T_int32, false),
Function::ExternalLinkage, "_resetstkoflw", m);
Expand Down Expand Up @@ -6448,14 +6415,6 @@ static void init_julia_llvm_env(Module *m)
#endif
#endif

std::vector<Type*> lhargs(0);
lhargs.push_back(T_int32);
jlleave_func =
Function::Create(FunctionType::get(T_void, lhargs, false),
Function::ExternalLinkage,
"jl_pop_handler", m);
add_named_global(jlleave_func, &jl_pop_handler);

std::vector<Type *> args_2vals_callee_rooted(0);
args_2vals_callee_rooted.push_back(PointerType::get(T_jlvalue, AddressSpace::CalleeRooted));
args_2vals_callee_rooted.push_back(PointerType::get(T_jlvalue, AddressSpace::CalleeRooted));
Expand Down Expand Up @@ -6594,6 +6553,11 @@ static void init_julia_llvm_env(Module *m)
except_enter_func->addFnAttr(Attribute::ReturnsTwice);
add_named_global(except_enter_func, (void*)NULL, /*dllimport*/false);

except_leave_func = Function::Create(FunctionType::get(T_void, false),
Function::ExternalLinkage,
"julia.except_leave");
add_named_global(except_leave_func, (void*)NULL, /*dllimport*/false);

jlgetworld_global =
new GlobalVariable(*m, T_size,
false, GlobalVariable::ExternalLinkage,
Expand Down
1 change: 0 additions & 1 deletion src/interpreter.c
Original file line number Diff line number Diff line change
Expand Up @@ -563,7 +563,6 @@ SECT_INTERP static jl_value_t *eval_body(jl_array_t *stmts, interpreter_state *s
else if (head == leave_sym) {
int hand_n_leave = jl_unbox_long(jl_exprarg(stmt,0));
assert(hand_n_leave > 0);
// equivalent to jl_pop_handler(hand_n_leave) :
jl_ptls_t ptls = jl_get_ptls_states();
jl_handler_t *eh = ptls->current_task->eh;
while (--hand_n_leave > 0)
Expand Down
11 changes: 7 additions & 4 deletions src/jitlayers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ void addOptimizationPasses(legacy::PassManagerBase *PM, int opt_level, bool dump
PM->add(createCFGSimplificationPass()); // Clean up disgusting code
#if JL_LLVM_VERSION < 50000
PM->add(createBarrierNoopPass());
PM->add(createLowerExcHandlersPass());
PM->add(createEHOutliningPass());
PM->add(createGCInvariantVerifierPass(false));
PM->add(createLateLowerGCFramePass());
PM->add(createLowerPTLSPass(dump_native));
Expand All @@ -128,13 +128,14 @@ void addOptimizationPasses(legacy::PassManagerBase *PM, int opt_level, bool dump
#endif
#if JL_LLVM_VERSION >= 50000
PM->add(createBarrierNoopPass());
PM->add(createLowerExcHandlersPass());
PM->add(createEHOutliningPass());
PM->add(createGCInvariantVerifierPass(false));
PM->add(createLateLowerGCFramePass());
PM->add(createLowerPTLSPass(dump_native));
#endif
if (dump_native)
PM->add(createMultiVersioningPass());
PM->add(createEHLoweringPass());
return;
}
PM->add(createPropagateJuliaAddrspaces());
Expand All @@ -146,12 +147,12 @@ void addOptimizationPasses(legacy::PassManagerBase *PM, int opt_level, bool dump
PM->add(createCFGSimplificationPass()); // Clean up disgusting code
PM->add(createDeadInstEliminationPass());
PM->add(createPromoteMemoryToRegisterPass()); // Kill useless allocas
PM->add(createEHOutliningPass());

// Due to bugs and missing features LLVM < 5.0, does not properly propagate
// our invariants. We need to do GC rooting here. This reduces the
// effectiveness of the optimization, but should retain correctness.
#if JL_LLVM_VERSION < 50000
PM->add(createLowerExcHandlersPass());
PM->add(createAllocOptPass());
PM->add(createLateLowerGCFramePass());
// Remove dead use of ptls
Expand Down Expand Up @@ -252,14 +253,16 @@ void addOptimizationPasses(legacy::PassManagerBase *PM, int opt_level, bool dump
// pass pipeline being re-exectuted. Prevent this by inserting a barrier.
#if JL_LLVM_VERSION >= 50000
PM->add(createBarrierNoopPass());
PM->add(createLowerExcHandlersPass());
PM->add(createGCInvariantVerifierPass(false));
PM->add(createLateLowerGCFramePass());
// Remove dead use of ptls
PM->add(createDeadCodeEliminationPass());
PM->add(createLowerPTLSPass(dump_native));
PM->add(createEHLoweringPass());
// Clean up write barrier and ptls lowering
PM->add(createCFGSimplificationPass());
#else
PM->add(createEHLoweringPass());
#endif
PM->add(createCombineMulAddPass());
}
Expand Down
3 changes: 2 additions & 1 deletion src/jitlayers.h
Original file line number Diff line number Diff line change
Expand Up @@ -178,10 +178,11 @@ class JuliaOJIT {
extern JuliaOJIT *jl_ExecutionEngine;
JL_DLLEXPORT extern LLVMContext jl_LLVMContext;

Pass *createEHOutliningPass();
Pass *createEHLoweringPass();
Pass *createLowerPTLSPass(bool imaging_mode);
Pass *createCombineMulAddPass();
Pass *createLateLowerGCFramePass();
Pass *createLowerExcHandlersPass();
Pass *createGCInvariantVerifierPass(bool Strong);
Pass *createPropagateJuliaAddrspaces();
Pass *createMultiVersioningPass();
Expand Down
18 changes: 6 additions & 12 deletions src/julia.h
Original file line number Diff line number Diff line change
Expand Up @@ -1556,9 +1556,8 @@ JL_DLLEXPORT void JL_NORETURN jl_no_exc_handler(jl_value_t *e);

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

STATIC_INLINE void jl_eh_restore_state(jl_handler_t *eh)
STATIC_INLINE void jl_eh_restore_state_th(jl_ptls_t ptls, jl_handler_t *eh)
{
jl_ptls_t ptls = jl_get_ptls_states();
jl_task_t *current_task = ptls->current_task;
// `eh` may not be `ptls->current_task->eh`. See `jl_pop_handler`
// This function should **NOT** have any safepoint before the ones at the
Expand Down Expand Up @@ -1587,8 +1586,12 @@ STATIC_INLINE void jl_eh_restore_state(jl_handler_t *eh)
}
}

STATIC_INLINE void jl_eh_restore_state(jl_handler_t *eh)
{
jl_eh_restore_state_th(jl_get_ptls_states(), eh);
}

JL_DLLEXPORT void jl_enter_handler(jl_handler_t *eh);
JL_DLLEXPORT void jl_pop_handler(int n);

#if defined(_OS_WINDOWS_)
#if defined(_COMPILER_MINGW_)
Expand All @@ -1598,19 +1601,10 @@ __declspec(noreturn) __attribute__ ((__nothrow__)) void (jl_longjmp)(jmp_buf _Bu
int (jl_setjmp)(jmp_buf _Buf);
void (jl_longjmp)(jmp_buf _Buf, int _Value);
#endif
#define jl_setjmp_f jl_setjmp
#define jl_setjmp_name "jl_setjmp"
#define jl_setjmp(a,b) jl_setjmp(a)
#define jl_longjmp(a,b) jl_longjmp(a,b)
#else
// determine actual entry point name
#if defined(sigsetjmp)
#define jl_setjmp_f __sigsetjmp
#define jl_setjmp_name "__sigsetjmp"
#else
#define jl_setjmp_f sigsetjmp
#define jl_setjmp_name "sigsetjmp"
#endif
#define jl_setjmp(a,b) sigsetjmp(a,b)
#define jl_longjmp(a,b) siglongjmp(a,b)
#endif
Expand Down
Loading

0 comments on commit 952c163

Please sign in to comment.