Skip to content

[WinEH] Fix try scopes leaking to caller on inline #167176

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+386 −75
Open

[WinEH] Fix try scopes leaking to caller on inline #167176

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
144 changes: 77 additions & 67 deletions clang/lib/CodeGen/CGCall.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3982,63 +3982,41 @@ llvm::Value *CodeGenFunction::EmitCMSEClearRecord(llvm::Value *Src,
return R;
}

void CodeGenFunction::EmitFunctionEpilog(
const CGFunctionInfo &FI, bool EmitRetDbgLoc, SourceLocation EndLoc,
uint64_t RetKeyInstructionsSourceAtom) {
if (FI.isNoReturn()) {
// Noreturn functions don't return.
EmitUnreachable(EndLoc);
return;
}

if (CurCodeDecl && CurCodeDecl->hasAttr<NakedAttr>()) {
// Naked functions don't have epilogues.
Builder.CreateUnreachable();
return;
}

// Functions with no result always return void.
if (!ReturnValue.isValid()) {
auto *I = Builder.CreateRetVoid();
if (RetKeyInstructionsSourceAtom)
addInstToSpecificSourceAtom(I, nullptr, RetKeyInstructionsSourceAtom);
else
addInstToNewSourceAtom(I, nullptr);
return;
}

llvm::DebugLoc RetDbgLoc;
llvm::Value *RV = nullptr;
QualType RetTy = FI.getReturnType();
static void processFunctionReturnInfo(CodeGenFunction &CGF,
const CGFunctionInfo &FI,
bool EmitRetDbgLoc, SourceLocation EndLoc,
QualType RetTy, llvm::Value *&RV,
llvm::DebugLoc &RetDbgLoc) {
const ABIArgInfo &RetAI = FI.getReturnInfo();

switch (RetAI.getKind()) {
case ABIArgInfo::InAlloca:
// Aggregates get evaluated directly into the destination. Sometimes we
// need to return the sret value in a register, though.
assert(hasAggregateEvaluationKind(RetTy));
assert(CodeGenFunction::hasAggregateEvaluationKind(RetTy));
if (RetAI.getInAllocaSRet()) {
llvm::Function::arg_iterator EI = CurFn->arg_end();
llvm::Function::arg_iterator EI = CGF.CurFn->arg_end();
--EI;
llvm::Value *ArgStruct = &*EI;
llvm::Value *SRet = Builder.CreateStructGEP(
llvm::Value *SRet = CGF.Builder.CreateStructGEP(
FI.getArgStruct(), ArgStruct, RetAI.getInAllocaFieldIndex());
llvm::Type *Ty =
cast<llvm::GetElementPtrInst>(SRet)->getResultElementType();
RV = Builder.CreateAlignedLoad(Ty, SRet, getPointerAlign(), "sret");
RV = CGF.Builder.CreateAlignedLoad(Ty, SRet, CGF.getPointerAlign(),
"sret");
}
break;

case ABIArgInfo::Indirect: {
auto AI = CurFn->arg_begin();
auto AI = CGF.CurFn->arg_begin();
if (RetAI.isSRetAfterThis())
++AI;
switch (getEvaluationKind(RetTy)) {
switch (CodeGenFunction::getEvaluationKind(RetTy)) {
case TEK_Complex: {
ComplexPairTy RT =
EmitLoadOfComplex(MakeAddrLValue(ReturnValue, RetTy), EndLoc);
EmitStoreOfComplex(RT, MakeNaturalAlignAddrLValue(&*AI, RetTy),
/*isInit*/ true);
CodeGenFunction::ComplexPairTy RT = CGF.EmitLoadOfComplex(
CGF.MakeAddrLValue(CGF.ReturnValue, RetTy), EndLoc);
CGF.EmitStoreOfComplex(RT, CGF.MakeNaturalAlignAddrLValue(&*AI, RetTy),
/*isInit*/ true);
break;
}
case TEK_Aggregate:
Expand All @@ -4048,13 +4026,14 @@ void CodeGenFunction::EmitFunctionEpilog(
LValueBaseInfo BaseInfo;
TBAAAccessInfo TBAAInfo;
CharUnits Alignment =
CGM.getNaturalTypeAlignment(RetTy, &BaseInfo, &TBAAInfo);
Address ArgAddr(&*AI, ConvertType(RetTy), Alignment);
LValue ArgVal =
LValue::MakeAddr(ArgAddr, RetTy, getContext(), BaseInfo, TBAAInfo);
EmitStoreOfScalar(
EmitLoadOfScalar(MakeAddrLValue(ReturnValue, RetTy), EndLoc), ArgVal,
/*isInit*/ true);
CGF.CGM.getNaturalTypeAlignment(RetTy, &BaseInfo, &TBAAInfo);
Address ArgAddr(&*AI, CGF.ConvertType(RetTy), Alignment);
LValue ArgVal = LValue::MakeAddr(ArgAddr, RetTy, CGF.getContext(),
BaseInfo, TBAAInfo);
CGF.EmitStoreOfScalar(
CGF.EmitLoadOfScalar(CGF.MakeAddrLValue(CGF.ReturnValue, RetTy),
EndLoc),
ArgVal, /*isInit*/ true);
break;
}
}
Expand All @@ -4063,57 +4042,57 @@ void CodeGenFunction::EmitFunctionEpilog(

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

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

// Otherwise, we have to do a simple load.
// Otherwise, we have to do a simple load.
} else {
RV = Builder.CreateLoad(ReturnValue);
RV = CGF.Builder.CreateLoad(CGF.ReturnValue);
}
} else {
// If the value is offset in memory, apply the offset now.
Address V = emitAddressAtOffset(*this, ReturnValue, RetAI);
Address V = emitAddressAtOffset(CGF, CGF.ReturnValue, RetAI);

RV = CreateCoercedLoad(V, RetAI.getCoerceToType(), *this);
RV = CreateCoercedLoad(V, RetAI.getCoerceToType(), CGF);
}

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

if (auto *FD = dyn_cast<FunctionDecl>(CurCodeDecl))
if (auto *FD = dyn_cast<FunctionDecl>(CGF.CurCodeDecl))
RT = FD->getReturnType();
else if (auto *MD = dyn_cast<ObjCMethodDecl>(CurCodeDecl))
else if (auto *MD = dyn_cast<ObjCMethodDecl>(CGF.CurCodeDecl))
RT = MD->getReturnType();
else if (isa<BlockDecl>(CurCodeDecl))
RT = BlockInfo->BlockExpression->getFunctionType()->getReturnType();
else if (isa<BlockDecl>(CGF.CurCodeDecl))
RT = CGF.BlockInfo->BlockExpression->getFunctionType()->getReturnType();
else
llvm_unreachable("Unexpected function/method type");

assert(getLangOpts().ObjCAutoRefCount && !FI.isReturnsRetained() &&
assert(CGF.getLangOpts().ObjCAutoRefCount && !FI.isReturnsRetained() &&
RT->isObjCRetainableType());
#endif
RV = emitAutoreleaseOfResult(*this, RV);
RV = emitAutoreleaseOfResult(CGF, RV);
}

break;
Expand All @@ -4128,47 +4107,78 @@ void CodeGenFunction::EmitFunctionEpilog(

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

auto eltAddr = Builder.CreateStructGEP(addr, i);
auto eltAddr = CGF.Builder.CreateStructGEP(addr, i);
llvm::Value *elt = CreateCoercedLoad(
eltAddr,
unpaddedStruct ? unpaddedStruct->getElementType(unpaddedIndex++)
: unpaddedCoercionType,
*this);
CGF);
results.push_back(elt);
}

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

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

RV = llvm::PoisonValue::get(returnType);
for (unsigned i = 0, e = results.size(); i != e; ++i) {
RV = Builder.CreateInsertValue(RV, results[i], i);
RV = CGF.Builder.CreateInsertValue(RV, results[i], i);
}
}
break;
}
case ABIArgInfo::TargetSpecific: {
Address V = emitAddressAtOffset(*this, ReturnValue, RetAI);
RV = CGM.getABIInfo().createCoercedLoad(V, RetAI, *this);
Address V = emitAddressAtOffset(CGF, CGF.ReturnValue, RetAI);
RV = CGF.CGM.getABIInfo().createCoercedLoad(V, RetAI, CGF);
break;
}
case ABIArgInfo::Expand:
case ABIArgInfo::IndirectAliased:
llvm_unreachable("Invalid ABI kind for return argument");
}
}

static bool isReturnReachable(CodeGenFunction::JumpDest &ReturnBlock) {
return !ReturnBlock.isValid() || !ReturnBlock.getBlock()->use_empty();
}

void CodeGenFunction::EmitFunctionEpilog(
const CGFunctionInfo &FI, bool EmitRetDbgLoc, SourceLocation EndLoc,
uint64_t RetKeyInstructionsSourceAtom) {
if (FI.isNoReturn()) {
// Noreturn functions don't return.
EmitUnreachable(EndLoc);
return;
}

if (CurCodeDecl && CurCodeDecl->hasAttr<NakedAttr>()) {
// Naked functions don't have epilogues.
Builder.CreateUnreachable();
return;
}

llvm::Value *RV = nullptr;
llvm::DebugLoc RetDbgLoc;
QualType RetTy = FI.getReturnType();

if (ReturnValue.isValid())
processFunctionReturnInfo(*this, FI, EmitRetDbgLoc, EndLoc, RetTy, RV,
RetDbgLoc);

if (SehTryEndInvokeDest && isReturnReachable(ReturnBlock))
EmitSehTryScopeEnd(SehTryEndInvokeDest);
Copy link
Collaborator

@efriedma-quic efriedma-quic Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks wrong. You need to end the scope in the ReturnStmt, before you branch to the return block. The return block is shared by all return statements; different return statements can be in different scopes.

CodeGenFunction::EmitReturnStmt calls EmitBranchThroughCleanup, which should pop the cleanup scopes, and as part of that should call EmitSehTryScopeEnd. I haven't dug into EmitBranchThroughCleanup to figure out why that isn't working the way it's supposed to.


llvm::Instruction *Ret;
if (RV) {
Expand Down Expand Up @@ -4203,7 +4213,7 @@ void CodeGenFunction::EmitReturnValueCheck(llvm::Value *RV) {

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

ReturnsNonNullAttr *RetNNAttr = nullptr;
Expand Down
10 changes: 6 additions & 4 deletions clang/lib/CodeGen/CGCleanup.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1329,8 +1329,10 @@ void CodeGenFunction::EmitCXXTemporary(const CXXTemporary *Temporary,
// Need to set "funclet" in OperandBundle properly for noThrow
// intrinsic (see CGCall.cpp)
static void EmitSehScope(CodeGenFunction &CGF,
llvm::FunctionCallee &SehCppScope) {
llvm::BasicBlock *InvokeDest = CGF.getInvokeDest();
llvm::FunctionCallee &SehCppScope,
llvm::BasicBlock *InvokeDest = nullptr) {
if (!InvokeDest)
InvokeDest = CGF.getInvokeDest();
assert(CGF.Builder.GetInsertBlock() && InvokeDest);
llvm::BasicBlock *Cont = CGF.createBasicBlock("invoke.cont");
SmallVector<llvm::OperandBundleDef, 1> BundleList =
Expand Down Expand Up @@ -1373,11 +1375,11 @@ void CodeGenFunction::EmitSehTryScopeBegin() {
}

// Invoke a llvm.seh.try.end at the end of a SEH scope for -EHa
void CodeGenFunction::EmitSehTryScopeEnd() {
void CodeGenFunction::EmitSehTryScopeEnd(llvm::BasicBlock *InvokeDest) {
assert(getLangOpts().EHAsynch);
llvm::FunctionType *FTy =
llvm::FunctionType::get(CGM.VoidTy, /*isVarArg=*/false);
llvm::FunctionCallee SehCppScope =
CGM.CreateRuntimeFunction(FTy, "llvm.seh.try.end");
EmitSehScope(*this, SehCppScope);
EmitSehScope(*this, SehCppScope, InvokeDest);
}
45 changes: 43 additions & 2 deletions clang/lib/CodeGen/CGException.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -635,6 +635,37 @@ void CodeGenFunction::EmitCXXTryStmt(const CXXTryStmt &S) {
ExitCXXTryStmt(S);
}

static bool
RequiresSehTryEnd(const CompoundStmt *S,
llvm::SmallPtrSet<const Stmt *, 1> &CheckedSehTryBlockStmts) {
if (!S || CheckedSehTryBlockStmts.contains(S))
return false;

llvm::SmallVector<const Stmt *> WorkList;
WorkList.push_back(S);

while (!WorkList.empty()) {
auto *Next = WorkList.back();
WorkList.pop_back();
if (!Next)
continue;

if (isa<ReturnStmt>(Next))
return true;

if (auto *Try = dyn_cast<CXXTryStmt>(Next))
CheckedSehTryBlockStmts.insert(Try->getTryBlock());

if (auto *Try = dyn_cast<SEHTryStmt>(Next))
CheckedSehTryBlockStmts.insert(Try->getTryBlock());

auto Children = Next->children();
WorkList.insert(WorkList.end(), Children.begin(), Children.end());
}

return false;
}

void CodeGenFunction::EnterCXXTryStmt(const CXXTryStmt &S, bool IsFnTryBlock) {
unsigned NumHandlers = S.getNumHandlers();
EHCatchScope *CatchScope = EHStack.pushCatch(NumHandlers);
Expand Down Expand Up @@ -666,8 +697,14 @@ void CodeGenFunction::EnterCXXTryStmt(const CXXTryStmt &S, bool IsFnTryBlock) {
CatchScope->setHandler(I, CGM.getCXXABI().getCatchAllTypeInfo(), Handler);
// Under async exceptions, catch(...) need to catch HW exception too
// Mark scope with SehTryBegin as a SEH __try scope
if (getLangOpts().EHAsynch)
if (getLangOpts().EHAsynch) {
EmitSehTryScopeBegin();
auto *TryBlock = S.getTryBlock();

if (!SehTryEndInvokeDest &&
RequiresSehTryEnd(TryBlock, CheckedSehTryBlockStmts))
SehTryEndInvokeDest = getInvokeDest();
}
}
}
}
Expand Down Expand Up @@ -1670,14 +1707,18 @@ void CodeGenFunction::EmitSEHTryStmt(const SEHTryStmt &S) {
SEHTryEpilogueStack.push_back(&TryExit);

llvm::BasicBlock *TryBB = nullptr;
auto *TryBlock = S.getTryBlock();
// IsEHa: emit an invoke to _seh_try_begin() runtime for -EHa
if (getLangOpts().EHAsynch) {
EmitRuntimeCallOrInvoke(getSehTryBeginFn(CGM));
if (SEHTryEpilogueStack.size() == 1) // outermost only
TryBB = Builder.GetInsertBlock();
if (!SehTryEndInvokeDest &&
RequiresSehTryEnd(TryBlock, CheckedSehTryBlockStmts))
SehTryEndInvokeDest = getInvokeDest();
}

EmitStmt(S.getTryBlock());
EmitStmt(TryBlock);

// Volatilize all blocks in Try, till current insert point
if (TryBB) {
Expand Down
8 changes: 7 additions & 1 deletion clang/lib/CodeGen/CodeGenFunction.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2129,6 +2129,12 @@ class CodeGenFunction : public CodeGenTypeCache {
/// Terminate funclets keyed by parent funclet pad.
llvm::MapVector<llvm::Value *, llvm::BasicBlock *> TerminateFunclets;

/// Visited try block statements that do not need a scope end.
llvm::SmallPtrSet<const Stmt *, 1> CheckedSehTryBlockStmts;

/// Unwind destination for try scope end.
llvm::BasicBlock *SehTryEndInvokeDest = nullptr;

/// Largest vector width used in ths function. Will be used to create a
/// function attribute.
unsigned LargestVectorWidth = 0;
Expand Down Expand Up @@ -3241,7 +3247,7 @@ class CodeGenFunction : public CodeGenTypeCache {
void EmitSehCppScopeBegin();
void EmitSehCppScopeEnd();
void EmitSehTryScopeBegin();
void EmitSehTryScopeEnd();
void EmitSehTryScopeEnd(llvm::BasicBlock *InvokeDest = nullptr);

bool EmitLifetimeStart(llvm::Value *Addr);
void EmitLifetimeEnd(llvm::Value *Addr);
Expand Down
Loading