Skip to content

Commit 9fe78db

Browse files
committed
[FunctionAttrs] Fix nounwind inference for landingpads
Currently, FunctionAttrs treats landingpads as non-throwing, and will infer nounwind for functions with landingpads (assuming they can't unwind in some other way, e.g. via resum). There are two problems with this: * Non-cleanup landingpads with catch/filter clauses do not necessarily catch all exceptions. Unless there are catch ptr null or filter [0 x ptr] zeroinitializer clauses, we should assume that we may unwind past this landingpad. This seems like an outright bug. * Cleanup landingpads are skipped during phase one unwinding, so we effectively need to support unwinding past them. Marking these nounwind is technically correct, but not compatible with how unwinding works in reality. Fixes #61945. Differential Revision: https://reviews.llvm.org/D147694
1 parent 677b0d3 commit 9fe78db

File tree

6 files changed

+110
-72
lines changed

6 files changed

+110
-72
lines changed

llvm/include/llvm/IR/Instruction.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -636,7 +636,11 @@ class Instruction : public User,
636636
bool isVolatile() const LLVM_READONLY;
637637

638638
/// Return true if this instruction may throw an exception.
639-
bool mayThrow() const LLVM_READONLY;
639+
///
640+
/// If IncludePhaseOneUnwind is set, this will also include cases where
641+
/// phase one unwinding may unwind past this frame due to skipping of
642+
/// cleanup landingpads.
643+
bool mayThrow(bool IncludePhaseOneUnwind = false) const LLVM_READONLY;
640644

641645
/// Return true if this instruction behaves like a memory fence: it can load
642646
/// or store to memory location without being given a memory location.

llvm/lib/IR/Instruction.cpp

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -733,7 +733,29 @@ bool Instruction::isVolatile() const {
733733
}
734734
}
735735

736-
bool Instruction::mayThrow() const {
736+
static bool canUnwindPastLandingPad(const LandingPadInst *LP,
737+
bool IncludePhaseOneUnwind) {
738+
// Because phase one unwinding skips cleanup landingpads, we effectively
739+
// unwind past this frame, and callers need to have valid unwind info.
740+
if (LP->isCleanup())
741+
return IncludePhaseOneUnwind;
742+
743+
for (unsigned I = 0; I < LP->getNumClauses(); ++I) {
744+
Constant *Clause = LP->getClause(I);
745+
// catch ptr null catches all exceptions.
746+
if (LP->isCatch(I) && isa<ConstantPointerNull>(Clause))
747+
return false;
748+
// filter [0 x ptr] catches all exceptions.
749+
if (LP->isFilter(I) && Clause->getType()->getArrayNumElements() == 0)
750+
return false;
751+
}
752+
753+
// May catch only some subset of exceptions, in which case other exceptions
754+
// will continue unwinding.
755+
return true;
756+
}
757+
758+
bool Instruction::mayThrow(bool IncludePhaseOneUnwind) const {
737759
switch (getOpcode()) {
738760
case Instruction::Call:
739761
return !cast<CallInst>(this)->doesNotThrow();
@@ -743,6 +765,18 @@ bool Instruction::mayThrow() const {
743765
return cast<CatchSwitchInst>(this)->unwindsToCaller();
744766
case Instruction::Resume:
745767
return true;
768+
case Instruction::Invoke: {
769+
// Landingpads themselves don't unwind -- however, an invoke of a skipped
770+
// landingpad may continue unwinding.
771+
BasicBlock *UnwindDest = cast<InvokeInst>(this)->getUnwindDest();
772+
Instruction *Pad = UnwindDest->getFirstNonPHI();
773+
if (auto *LP = dyn_cast<LandingPadInst>(Pad))
774+
return canUnwindPastLandingPad(LP, IncludePhaseOneUnwind);
775+
return false;
776+
}
777+
case Instruction::CleanupPad:
778+
// Treat the same as cleanup landingpad.
779+
return IncludePhaseOneUnwind;
746780
default:
747781
return false;
748782
}

llvm/lib/Transforms/IPO/AttributorAttributes.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1981,7 +1981,7 @@ struct AANoUnwindImpl : AANoUnwind {
19811981
(unsigned)Instruction::CatchSwitch, (unsigned)Instruction::Resume};
19821982

19831983
auto CheckForNoUnwind = [&](Instruction &I) {
1984-
if (!I.mayThrow())
1984+
if (!I.mayThrow(/* IncludePhaseOneUnwind */ true))
19851985
return true;
19861986

19871987
if (const auto *CB = dyn_cast<CallBase>(&I)) {

llvm/lib/Transforms/IPO/FunctionAttrs.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1378,7 +1378,7 @@ static bool InstrBreaksNonConvergent(Instruction &I,
13781378

13791379
/// Helper for NoUnwind inference predicate InstrBreaksAttribute.
13801380
static bool InstrBreaksNonThrowing(Instruction &I, const SCCNodeSet &SCCNodes) {
1381-
if (!I.mayThrow())
1381+
if (!I.mayThrow(/* IncludePhaseOneUnwind */ true))
13821382
return false;
13831383
if (const auto *CI = dyn_cast<CallInst>(&I)) {
13841384
if (Function *Callee = CI->getCalledFunction()) {

0 commit comments

Comments
 (0)