Skip to content

Commit 262b751

Browse files
MuellerMPMuellerMP
committed
[WinEH] Fix try scopes leaking to caller on inline
This fixes issue #164169 When inlining functions compiled with -EHa, try scope terminators might need to be inserted before inlined returns. This prevents leaking try scopes over to the caller. Try scopes can be ended before a ret due to the unwinder forwarding exceptions in the seh epilog to the caller.
1 parent 26bb121 commit 262b751

File tree

8 files changed

+386
-75
lines changed

8 files changed

+386
-75
lines changed

‎clang/lib/CodeGen/CGCall.cpp‎

Lines changed: 77 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -3982,63 +3982,41 @@ llvm::Value *CodeGenFunction::EmitCMSEClearRecord(llvm::Value *Src,
39823982
return R;
39833983
}
39843984

3985-
void CodeGenFunction::EmitFunctionEpilog(
3986-
const CGFunctionInfo &FI, bool EmitRetDbgLoc, SourceLocation EndLoc,
3987-
uint64_t RetKeyInstructionsSourceAtom) {
3988-
if (FI.isNoReturn()) {
3989-
// Noreturn functions don't return.
3990-
EmitUnreachable(EndLoc);
3991-
return;
3992-
}
3993-
3994-
if (CurCodeDecl && CurCodeDecl->hasAttr<NakedAttr>()) {
3995-
// Naked functions don't have epilogues.
3996-
Builder.CreateUnreachable();
3997-
return;
3998-
}
3999-
4000-
// Functions with no result always return void.
4001-
if (!ReturnValue.isValid()) {
4002-
auto *I = Builder.CreateRetVoid();
4003-
if (RetKeyInstructionsSourceAtom)
4004-
addInstToSpecificSourceAtom(I, nullptr, RetKeyInstructionsSourceAtom);
4005-
else
4006-
addInstToNewSourceAtom(I, nullptr);
4007-
return;
4008-
}
4009-
4010-
llvm::DebugLoc RetDbgLoc;
4011-
llvm::Value *RV = nullptr;
4012-
QualType RetTy = FI.getReturnType();
3985+
static void processFunctionReturnInfo(CodeGenFunction &CGF,
3986+
const CGFunctionInfo &FI,
3987+
bool EmitRetDbgLoc, SourceLocation EndLoc,
3988+
QualType RetTy, llvm::Value *&RV,
3989+
llvm::DebugLoc &RetDbgLoc) {
40133990
const ABIArgInfo &RetAI = FI.getReturnInfo();
40143991

40153992
switch (RetAI.getKind()) {
40163993
case ABIArgInfo::InAlloca:
40173994
// Aggregates get evaluated directly into the destination. Sometimes we
40183995
// need to return the sret value in a register, though.
4019-
assert(hasAggregateEvaluationKind(RetTy));
3996+
assert(CodeGenFunction::hasAggregateEvaluationKind(RetTy));
40203997
if (RetAI.getInAllocaSRet()) {
4021-
llvm::Function::arg_iterator EI = CurFn->arg_end();
3998+
llvm::Function::arg_iterator EI = CGF.CurFn->arg_end();
40223999
--EI;
40234000
llvm::Value *ArgStruct = &*EI;
4024-
llvm::Value *SRet = Builder.CreateStructGEP(
4001+
llvm::Value *SRet = CGF.Builder.CreateStructGEP(
40254002
FI.getArgStruct(), ArgStruct, RetAI.getInAllocaFieldIndex());
40264003
llvm::Type *Ty =
40274004
cast<llvm::GetElementPtrInst>(SRet)->getResultElementType();
4028-
RV = Builder.CreateAlignedLoad(Ty, SRet, getPointerAlign(), "sret");
4005+
RV = CGF.Builder.CreateAlignedLoad(Ty, SRet, CGF.getPointerAlign(),
4006+
"sret");
40294007
}
40304008
break;
40314009

40324010
case ABIArgInfo::Indirect: {
4033-
auto AI = CurFn->arg_begin();
4011+
auto AI = CGF.CurFn->arg_begin();
40344012
if (RetAI.isSRetAfterThis())
40354013
++AI;
4036-
switch (getEvaluationKind(RetTy)) {
4014+
switch (CodeGenFunction::getEvaluationKind(RetTy)) {
40374015
case TEK_Complex: {
4038-
ComplexPairTy RT =
4039-
EmitLoadOfComplex(MakeAddrLValue(ReturnValue, RetTy), EndLoc);
4040-
EmitStoreOfComplex(RT, MakeNaturalAlignAddrLValue(&*AI, RetTy),
4041-
/*isInit*/ true);
4016+
CodeGenFunction::ComplexPairTy RT = CGF.EmitLoadOfComplex(
4017+
CGF.MakeAddrLValue(CGF.ReturnValue, RetTy), EndLoc);
4018+
CGF.EmitStoreOfComplex(RT, CGF.MakeNaturalAlignAddrLValue(&*AI, RetTy),
4019+
/*isInit*/ true);
40424020
break;
40434021
}
40444022
case TEK_Aggregate:
@@ -4048,13 +4026,14 @@ void CodeGenFunction::EmitFunctionEpilog(
40484026
LValueBaseInfo BaseInfo;
40494027
TBAAAccessInfo TBAAInfo;
40504028
CharUnits Alignment =
4051-
CGM.getNaturalTypeAlignment(RetTy, &BaseInfo, &TBAAInfo);
4052-
Address ArgAddr(&*AI, ConvertType(RetTy), Alignment);
4053-
LValue ArgVal =
4054-
LValue::MakeAddr(ArgAddr, RetTy, getContext(), BaseInfo, TBAAInfo);
4055-
EmitStoreOfScalar(
4056-
EmitLoadOfScalar(MakeAddrLValue(ReturnValue, RetTy), EndLoc), ArgVal,
4057-
/*isInit*/ true);
4029+
CGF.CGM.getNaturalTypeAlignment(RetTy, &BaseInfo, &TBAAInfo);
4030+
Address ArgAddr(&*AI, CGF.ConvertType(RetTy), Alignment);
4031+
LValue ArgVal = LValue::MakeAddr(ArgAddr, RetTy, CGF.getContext(),
4032+
BaseInfo, TBAAInfo);
4033+
CGF.EmitStoreOfScalar(
4034+
CGF.EmitLoadOfScalar(CGF.MakeAddrLValue(CGF.ReturnValue, RetTy),
4035+
EndLoc),
4036+
ArgVal, /*isInit*/ true);
40584037
break;
40594038
}
40604039
}
@@ -4063,57 +4042,57 @@ void CodeGenFunction::EmitFunctionEpilog(
40634042

40644043
case ABIArgInfo::Extend:
40654044
case ABIArgInfo::Direct:
4066-
if (RetAI.getCoerceToType() == ConvertType(RetTy) &&
4045+
if (RetAI.getCoerceToType() == CGF.ConvertType(RetTy) &&
40674046
RetAI.getDirectOffset() == 0) {
40684047
// The internal return value temp always will have pointer-to-return-type
40694048
// type, just do a load.
40704049

40714050
// If there is a dominating store to ReturnValue, we can elide
40724051
// the load, zap the store, and usually zap the alloca.
4073-
if (llvm::StoreInst *SI = findDominatingStoreToReturnValue(*this)) {
4052+
if (llvm::StoreInst *SI = findDominatingStoreToReturnValue(CGF)) {
40744053
// Reuse the debug location from the store unless there is
40754054
// cleanup code to be emitted between the store and return
40764055
// instruction.
4077-
if (EmitRetDbgLoc && !AutoreleaseResult)
4056+
if (EmitRetDbgLoc && !CGF.AutoreleaseResult)
40784057
RetDbgLoc = SI->getDebugLoc();
40794058
// Get the stored value and nuke the now-dead store.
40804059
RV = SI->getValueOperand();
40814060
SI->eraseFromParent();
40824061

4083-
// Otherwise, we have to do a simple load.
4062+
// Otherwise, we have to do a simple load.
40844063
} else {
4085-
RV = Builder.CreateLoad(ReturnValue);
4064+
RV = CGF.Builder.CreateLoad(CGF.ReturnValue);
40864065
}
40874066
} else {
40884067
// If the value is offset in memory, apply the offset now.
4089-
Address V = emitAddressAtOffset(*this, ReturnValue, RetAI);
4068+
Address V = emitAddressAtOffset(CGF, CGF.ReturnValue, RetAI);
40904069

4091-
RV = CreateCoercedLoad(V, RetAI.getCoerceToType(), *this);
4070+
RV = CreateCoercedLoad(V, RetAI.getCoerceToType(), CGF);
40924071
}
40934072

40944073
// In ARC, end functions that return a retainable type with a call
40954074
// to objc_autoreleaseReturnValue.
4096-
if (AutoreleaseResult) {
4075+
if (CGF.AutoreleaseResult) {
40974076
#ifndef NDEBUG
40984077
// Type::isObjCRetainabletype has to be called on a QualType that hasn't
40994078
// been stripped of the typedefs, so we cannot use RetTy here. Get the
41004079
// original return type of FunctionDecl, CurCodeDecl, and BlockDecl from
41014080
// CurCodeDecl or BlockInfo.
41024081
QualType RT;
41034082

4104-
if (auto *FD = dyn_cast<FunctionDecl>(CurCodeDecl))
4083+
if (auto *FD = dyn_cast<FunctionDecl>(CGF.CurCodeDecl))
41054084
RT = FD->getReturnType();
4106-
else if (auto *MD = dyn_cast<ObjCMethodDecl>(CurCodeDecl))
4085+
else if (auto *MD = dyn_cast<ObjCMethodDecl>(CGF.CurCodeDecl))
41074086
RT = MD->getReturnType();
4108-
else if (isa<BlockDecl>(CurCodeDecl))
4109-
RT = BlockInfo->BlockExpression->getFunctionType()->getReturnType();
4087+
else if (isa<BlockDecl>(CGF.CurCodeDecl))
4088+
RT = CGF.BlockInfo->BlockExpression->getFunctionType()->getReturnType();
41104089
else
41114090
llvm_unreachable("Unexpected function/method type");
41124091

4113-
assert(getLangOpts().ObjCAutoRefCount && !FI.isReturnsRetained() &&
4092+
assert(CGF.getLangOpts().ObjCAutoRefCount && !FI.isReturnsRetained() &&
41144093
RT->isObjCRetainableType());
41154094
#endif
4116-
RV = emitAutoreleaseOfResult(*this, RV);
4095+
RV = emitAutoreleaseOfResult(CGF, RV);
41174096
}
41184097

41194098
break;
@@ -4128,47 +4107,78 @@ void CodeGenFunction::EmitFunctionEpilog(
41284107

41294108
// Load all of the coerced elements out into results.
41304109
llvm::SmallVector<llvm::Value *, 4> results;
4131-
Address addr = ReturnValue.withElementType(coercionType);
4110+
Address addr = CGF.ReturnValue.withElementType(coercionType);
41324111
unsigned unpaddedIndex = 0;
41334112
for (unsigned i = 0, e = coercionType->getNumElements(); i != e; ++i) {
41344113
auto coercedEltType = coercionType->getElementType(i);
41354114
if (ABIArgInfo::isPaddingForCoerceAndExpand(coercedEltType))
41364115
continue;
41374116

4138-
auto eltAddr = Builder.CreateStructGEP(addr, i);
4117+
auto eltAddr = CGF.Builder.CreateStructGEP(addr, i);
41394118
llvm::Value *elt = CreateCoercedLoad(
41404119
eltAddr,
41414120
unpaddedStruct ? unpaddedStruct->getElementType(unpaddedIndex++)
41424121
: unpaddedCoercionType,
4143-
*this);
4122+
CGF);
41444123
results.push_back(elt);
41454124
}
41464125

41474126
// If we have one result, it's the single direct result type.
41484127
if (results.size() == 1) {
41494128
RV = results[0];
41504129

4151-
// Otherwise, we need to make a first-class aggregate.
4130+
// Otherwise, we need to make a first-class aggregate.
41524131
} else {
41534132
// Construct a return type that lacks padding elements.
41544133
llvm::Type *returnType = RetAI.getUnpaddedCoerceAndExpandType();
41554134

41564135
RV = llvm::PoisonValue::get(returnType);
41574136
for (unsigned i = 0, e = results.size(); i != e; ++i) {
4158-
RV = Builder.CreateInsertValue(RV, results[i], i);
4137+
RV = CGF.Builder.CreateInsertValue(RV, results[i], i);
41594138
}
41604139
}
41614140
break;
41624141
}
41634142
case ABIArgInfo::TargetSpecific: {
4164-
Address V = emitAddressAtOffset(*this, ReturnValue, RetAI);
4165-
RV = CGM.getABIInfo().createCoercedLoad(V, RetAI, *this);
4143+
Address V = emitAddressAtOffset(CGF, CGF.ReturnValue, RetAI);
4144+
RV = CGF.CGM.getABIInfo().createCoercedLoad(V, RetAI, CGF);
41664145
break;
41674146
}
41684147
case ABIArgInfo::Expand:
41694148
case ABIArgInfo::IndirectAliased:
41704149
llvm_unreachable("Invalid ABI kind for return argument");
41714150
}
4151+
}
4152+
4153+
static bool isReturnReachable(CodeGenFunction::JumpDest &ReturnBlock) {
4154+
return !ReturnBlock.isValid() || !ReturnBlock.getBlock()->use_empty();
4155+
}
4156+
4157+
void CodeGenFunction::EmitFunctionEpilog(
4158+
const CGFunctionInfo &FI, bool EmitRetDbgLoc, SourceLocation EndLoc,
4159+
uint64_t RetKeyInstructionsSourceAtom) {
4160+
if (FI.isNoReturn()) {
4161+
// Noreturn functions don't return.
4162+
EmitUnreachable(EndLoc);
4163+
return;
4164+
}
4165+
4166+
if (CurCodeDecl && CurCodeDecl->hasAttr<NakedAttr>()) {
4167+
// Naked functions don't have epilogues.
4168+
Builder.CreateUnreachable();
4169+
return;
4170+
}
4171+
4172+
llvm::Value *RV = nullptr;
4173+
llvm::DebugLoc RetDbgLoc;
4174+
QualType RetTy = FI.getReturnType();
4175+
4176+
if (ReturnValue.isValid())
4177+
processFunctionReturnInfo(*this, FI, EmitRetDbgLoc, EndLoc, RetTy, RV,
4178+
RetDbgLoc);
4179+
4180+
if (SehTryEndInvokeDest && isReturnReachable(ReturnBlock))
4181+
EmitSehTryScopeEnd(SehTryEndInvokeDest);
41724182

41734183
llvm::Instruction *Ret;
41744184
if (RV) {
@@ -4203,7 +4213,7 @@ void CodeGenFunction::EmitReturnValueCheck(llvm::Value *RV) {
42034213

42044214
// If the return block isn't reachable, neither is this check, so don't emit
42054215
// it.
4206-
if (ReturnBlock.isValid() && ReturnBlock.getBlock()->use_empty())
4216+
if (!isReturnReachable(ReturnBlock))
42074217
return;
42084218

42094219
ReturnsNonNullAttr *RetNNAttr = nullptr;

‎clang/lib/CodeGen/CGCleanup.cpp‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1329,8 +1329,10 @@ void CodeGenFunction::EmitCXXTemporary(const CXXTemporary *Temporary,
13291329
// Need to set "funclet" in OperandBundle properly for noThrow
13301330
// intrinsic (see CGCall.cpp)
13311331
static void EmitSehScope(CodeGenFunction &CGF,
1332-
llvm::FunctionCallee &SehCppScope) {
1333-
llvm::BasicBlock *InvokeDest = CGF.getInvokeDest();
1332+
llvm::FunctionCallee &SehCppScope,
1333+
llvm::BasicBlock *InvokeDest = nullptr) {
1334+
if (!InvokeDest)
1335+
InvokeDest = CGF.getInvokeDest();
13341336
assert(CGF.Builder.GetInsertBlock() && InvokeDest);
13351337
llvm::BasicBlock *Cont = CGF.createBasicBlock("invoke.cont");
13361338
SmallVector<llvm::OperandBundleDef, 1> BundleList =
@@ -1373,11 +1375,11 @@ void CodeGenFunction::EmitSehTryScopeBegin() {
13731375
}
13741376

13751377
// Invoke a llvm.seh.try.end at the end of a SEH scope for -EHa
1376-
void CodeGenFunction::EmitSehTryScopeEnd() {
1378+
void CodeGenFunction::EmitSehTryScopeEnd(llvm::BasicBlock *InvokeDest) {
13771379
assert(getLangOpts().EHAsynch);
13781380
llvm::FunctionType *FTy =
13791381
llvm::FunctionType::get(CGM.VoidTy, /*isVarArg=*/false);
13801382
llvm::FunctionCallee SehCppScope =
13811383
CGM.CreateRuntimeFunction(FTy, "llvm.seh.try.end");
1382-
EmitSehScope(*this, SehCppScope);
1384+
EmitSehScope(*this, SehCppScope, InvokeDest);
13831385
}

‎clang/lib/CodeGen/CGException.cpp‎

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -635,6 +635,37 @@ void CodeGenFunction::EmitCXXTryStmt(const CXXTryStmt &S) {
635635
ExitCXXTryStmt(S);
636636
}
637637

638+
static bool
639+
RequiresSehTryEnd(const CompoundStmt *S,
640+
llvm::SmallPtrSet<const Stmt *, 1> &CheckedSehTryBlockStmts) {
641+
if (!S || CheckedSehTryBlockStmts.contains(S))
642+
return false;
643+
644+
llvm::SmallVector<const Stmt *> WorkList;
645+
WorkList.push_back(S);
646+
647+
while (!WorkList.empty()) {
648+
auto *Next = WorkList.back();
649+
WorkList.pop_back();
650+
if (!Next)
651+
continue;
652+
653+
if (isa<ReturnStmt>(Next))
654+
return true;
655+
656+
if (auto *Try = dyn_cast<CXXTryStmt>(Next))
657+
CheckedSehTryBlockStmts.insert(Try->getTryBlock());
658+
659+
if (auto *Try = dyn_cast<SEHTryStmt>(Next))
660+
CheckedSehTryBlockStmts.insert(Try->getTryBlock());
661+
662+
auto Children = Next->children();
663+
WorkList.insert(WorkList.end(), Children.begin(), Children.end());
664+
}
665+
666+
return false;
667+
}
668+
638669
void CodeGenFunction::EnterCXXTryStmt(const CXXTryStmt &S, bool IsFnTryBlock) {
639670
unsigned NumHandlers = S.getNumHandlers();
640671
EHCatchScope *CatchScope = EHStack.pushCatch(NumHandlers);
@@ -666,8 +697,14 @@ void CodeGenFunction::EnterCXXTryStmt(const CXXTryStmt &S, bool IsFnTryBlock) {
666697
CatchScope->setHandler(I, CGM.getCXXABI().getCatchAllTypeInfo(), Handler);
667698
// Under async exceptions, catch(...) need to catch HW exception too
668699
// Mark scope with SehTryBegin as a SEH __try scope
669-
if (getLangOpts().EHAsynch)
700+
if (getLangOpts().EHAsynch) {
670701
EmitSehTryScopeBegin();
702+
auto *TryBlock = S.getTryBlock();
703+
704+
if (!SehTryEndInvokeDest &&
705+
RequiresSehTryEnd(TryBlock, CheckedSehTryBlockStmts))
706+
SehTryEndInvokeDest = getInvokeDest();
707+
}
671708
}
672709
}
673710
}
@@ -1670,14 +1707,18 @@ void CodeGenFunction::EmitSEHTryStmt(const SEHTryStmt &S) {
16701707
SEHTryEpilogueStack.push_back(&TryExit);
16711708

16721709
llvm::BasicBlock *TryBB = nullptr;
1710+
auto *TryBlock = S.getTryBlock();
16731711
// IsEHa: emit an invoke to _seh_try_begin() runtime for -EHa
16741712
if (getLangOpts().EHAsynch) {
16751713
EmitRuntimeCallOrInvoke(getSehTryBeginFn(CGM));
16761714
if (SEHTryEpilogueStack.size() == 1) // outermost only
16771715
TryBB = Builder.GetInsertBlock();
1716+
if (!SehTryEndInvokeDest &&
1717+
RequiresSehTryEnd(TryBlock, CheckedSehTryBlockStmts))
1718+
SehTryEndInvokeDest = getInvokeDest();
16781719
}
16791720

1680-
EmitStmt(S.getTryBlock());
1721+
EmitStmt(TryBlock);
16811722

16821723
// Volatilize all blocks in Try, till current insert point
16831724
if (TryBB) {

‎clang/lib/CodeGen/CodeGenFunction.h‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2129,6 +2129,12 @@ class CodeGenFunction : public CodeGenTypeCache {
21292129
/// Terminate funclets keyed by parent funclet pad.
21302130
llvm::MapVector<llvm::Value *, llvm::BasicBlock *> TerminateFunclets;
21312131

2132+
/// Visited try block statements that do not need a scope end.
2133+
llvm::SmallPtrSet<const Stmt *, 1> CheckedSehTryBlockStmts;
2134+
2135+
/// Unwind destination for try scope end.
2136+
llvm::BasicBlock *SehTryEndInvokeDest = nullptr;
2137+
21322138
/// Largest vector width used in ths function. Will be used to create a
21332139
/// function attribute.
21342140
unsigned LargestVectorWidth = 0;
@@ -3241,7 +3247,7 @@ class CodeGenFunction : public CodeGenTypeCache {
32413247
void EmitSehCppScopeBegin();
32423248
void EmitSehCppScopeEnd();
32433249
void EmitSehTryScopeBegin();
3244-
void EmitSehTryScopeEnd();
3250+
void EmitSehTryScopeEnd(llvm::BasicBlock *InvokeDest = nullptr);
32453251

32463252
bool EmitLifetimeStart(llvm::Value *Addr);
32473253
void EmitLifetimeEnd(llvm::Value *Addr);

0 commit comments

Comments
 (0)