From 1c0b48695a48127512a0fb5f705c853afdcb6853 Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Sun, 21 Feb 2021 18:41:22 -0500 Subject: [PATCH] codegen: guard phi node loads of invalid inputs (#39747) At runtime, it is prohibited to observe these values, but we need to make sure they are not reading through undefined pointers (and potentially trying to GC-root the memory there) Refs ChainRules in #39641 (cherry picked from commit fdd26337fde20b3470f8f3eeb9026b1b05394b36) --- src/codegen.cpp | 43 ++++++++++++++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 11 deletions(-) diff --git a/src/codegen.cpp b/src/codegen.cpp index 02076a82e99092..817aceddfe4039 100644 --- a/src/codegen.cpp +++ b/src/codegen.cpp @@ -2336,8 +2336,13 @@ static jl_cgval_t emit_getfield(jl_codectx_t &ctx, const jl_cgval_t &strct, jl_s } template -static Value *emit_guarded_test(jl_codectx_t &ctx, Value *ifnot, bool defval, Func &&func) +static Value *emit_guarded_test(jl_codectx_t &ctx, Value *ifnot, Constant *defval, Func &&func) { + if (auto Cond = dyn_cast(ifnot)) { + if (Cond->isZero()) + return defval; + return func(); + } BasicBlock *currBB = ctx.builder.GetInsertBlock(); BasicBlock *passBB = BasicBlock::Create(jl_LLVMContext, "guard_pass", ctx.f); BasicBlock *exitBB = BasicBlock::Create(jl_LLVMContext, "guard_exit", ctx.f); @@ -2347,12 +2352,20 @@ static Value *emit_guarded_test(jl_codectx_t &ctx, Value *ifnot, bool defval, Fu passBB = ctx.builder.GetInsertBlock(); ctx.builder.CreateBr(exitBB); ctx.builder.SetInsertPoint(exitBB); - PHINode *phi = ctx.builder.CreatePHI(T_int1, 2); - phi->addIncoming(ConstantInt::get(T_int1, defval), currBB); + if (defval == nullptr) + return nullptr; + PHINode *phi = ctx.builder.CreatePHI(defval->getType(), 2); + phi->addIncoming(defval, currBB); phi->addIncoming(res, passBB); return phi; } +template +static Value *emit_guarded_test(jl_codectx_t &ctx, Value *ifnot, bool defval, Func &&func) +{ + return emit_guarded_test(ctx, ifnot, ConstantInt::get(T_int1, defval), func); +} + template static Value *emit_nullcheck_guard(jl_codectx_t &ctx, Value *nullcheck, Func &&func) { @@ -6842,7 +6855,7 @@ static std::pair, jl_llvm_functions_t> auto undef_value_for_type = [&](Type *T) { auto tracked = CountTrackedPointers(T); - Value *undef; + Constant *undef; if (tracked.count) // make sure gc pointers (including ptr_phi of union-split) are initialized to NULL undef = Constant::getNullValue(T); @@ -6925,23 +6938,31 @@ static std::pair, jl_llvm_functions_t> if (val.typ == (jl_value_t*)jl_bottom_type) { V = undef_value_for_type(VN->getType()); } - else if (VN && VN->getType() == T_prjlvalue) { + else if (VN->getType() == T_prjlvalue) { // Includes the jl_is_uniontype(phiType) && !TindexN case + // TODO: if convert_julia_type says it is wasted effort and to skip it, is it worth using V_rnull (dynamically)? V = boxed(ctx, val); } else { - // XXX: must emit undef here (rather than a bitcast or - // load of val) if the runtime type of val isn't phiType - V = emit_unbox(ctx, VN->getType(), val, phiType); + // must be careful to emit undef here (rather than a bitcast or + // load of val) if the runtime type of val isn't phiType + Value *isvalid = emit_isa(ctx, val, phiType, NULL).first; + V = emit_guarded_test(ctx, isvalid, undef_value_for_type(VN->getType()), [&] { + return emit_unbox(ctx, VN->getType(), val, phiType); + }); } VN->addIncoming(V, ctx.builder.GetInsertBlock()); assert(!TindexN); } else if (dest && val.typ != (jl_value_t*)jl_bottom_type) { - // XXX: must emit undef here (rather than a bitcast or - // load of val) if the runtime type of val isn't phiType + // must be careful to emit undef here (rather than a bitcast or + // load of val) if the runtime type of val isn't phiType assert(lty != T_prjlvalue); - (void)emit_unbox(ctx, lty, val, phiType, maybe_decay_tracked(ctx, dest)); + Value *isvalid = emit_isa(ctx, val, phiType, NULL).first; + emit_guarded_test(ctx, isvalid, nullptr, [&] { + (void)emit_unbox(ctx, lty, val, phiType, maybe_decay_tracked(ctx, dest)); + return nullptr; + }); } } else {