Skip to content

Commit

Permalink
[FunctionAttrs] Fix nounwind inference for landingpads
Browse files Browse the repository at this point in the history
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
  • Loading branch information
nikic committed Apr 14, 2023
1 parent 677b0d3 commit 9fe78db
Show file tree
Hide file tree
Showing 6 changed files with 110 additions and 72 deletions.
6 changes: 5 additions & 1 deletion llvm/include/llvm/IR/Instruction.h
Original file line number Diff line number Diff line change
Expand Up @@ -636,7 +636,11 @@ class Instruction : public User,
bool isVolatile() const LLVM_READONLY;

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

/// Return true if this instruction behaves like a memory fence: it can load
/// or store to memory location without being given a memory location.
Expand Down
36 changes: 35 additions & 1 deletion llvm/lib/IR/Instruction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -733,7 +733,29 @@ bool Instruction::isVolatile() const {
}
}

bool Instruction::mayThrow() const {
static bool canUnwindPastLandingPad(const LandingPadInst *LP,
bool IncludePhaseOneUnwind) {
// Because phase one unwinding skips cleanup landingpads, we effectively
// unwind past this frame, and callers need to have valid unwind info.
if (LP->isCleanup())
return IncludePhaseOneUnwind;

for (unsigned I = 0; I < LP->getNumClauses(); ++I) {
Constant *Clause = LP->getClause(I);
// catch ptr null catches all exceptions.
if (LP->isCatch(I) && isa<ConstantPointerNull>(Clause))
return false;
// filter [0 x ptr] catches all exceptions.
if (LP->isFilter(I) && Clause->getType()->getArrayNumElements() == 0)
return false;
}

// May catch only some subset of exceptions, in which case other exceptions
// will continue unwinding.
return true;
}

bool Instruction::mayThrow(bool IncludePhaseOneUnwind) const {
switch (getOpcode()) {
case Instruction::Call:
return !cast<CallInst>(this)->doesNotThrow();
Expand All @@ -743,6 +765,18 @@ bool Instruction::mayThrow() const {
return cast<CatchSwitchInst>(this)->unwindsToCaller();
case Instruction::Resume:
return true;
case Instruction::Invoke: {
// Landingpads themselves don't unwind -- however, an invoke of a skipped
// landingpad may continue unwinding.
BasicBlock *UnwindDest = cast<InvokeInst>(this)->getUnwindDest();
Instruction *Pad = UnwindDest->getFirstNonPHI();
if (auto *LP = dyn_cast<LandingPadInst>(Pad))
return canUnwindPastLandingPad(LP, IncludePhaseOneUnwind);
return false;
}
case Instruction::CleanupPad:
// Treat the same as cleanup landingpad.
return IncludePhaseOneUnwind;
default:
return false;
}
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Transforms/IPO/AttributorAttributes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1981,7 +1981,7 @@ struct AANoUnwindImpl : AANoUnwind {
(unsigned)Instruction::CatchSwitch, (unsigned)Instruction::Resume};

auto CheckForNoUnwind = [&](Instruction &I) {
if (!I.mayThrow())
if (!I.mayThrow(/* IncludePhaseOneUnwind */ true))
return true;

if (const auto *CB = dyn_cast<CallBase>(&I)) {
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Transforms/IPO/FunctionAttrs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1378,7 +1378,7 @@ static bool InstrBreaksNonConvergent(Instruction &I,

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

0 comments on commit 9fe78db

Please sign in to comment.