[PATCH] D147694: [FunctionAttrs] Fix nounwind inference for landingpads

[Nikita Popov via Phabricator via llvm-commits]

nikic created this revision.
nikic added a reviewer: efriedma.
Herald added subscribers: ormris, StephenFan, hiraditya.
Herald added a project: All.
nikic requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Currently, FunctionAttrs treats landingpads as non-throwing, and will infer nounwind for functions with landingpads (and without something like resumes that continue unwinding). 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 https://github.com/llvm/llvm-project/issues/61945.


https://reviews.llvm.org/D147694

Files:
  llvm/lib/Transforms/IPO/FunctionAttrs.cpp
  llvm/test/Transforms/FunctionAttrs/nounwind.ll


Index: llvm/test/Transforms/FunctionAttrs/nounwind.ll
===================================================================
--- llvm/test/Transforms/FunctionAttrs/nounwind.ll
+++ llvm/test/Transforms/FunctionAttrs/nounwind.ll
@@ -130,7 +130,7 @@
 @catch_ty = external global ptr
 
 define void @catch_specific_landingpad() personality ptr @__gxx_personality_v0 {
-; CHECK: Function Attrs: noreturn nounwind
+; CHECK: Function Attrs: noreturn
 ; CHECK-LABEL: define {{[^@]+}}@catch_specific_landingpad
 ; CHECK-SAME: () #[[ATTR3:[0-9]+]] personality ptr @__gxx_personality_v0 {
 ; CHECK-NEXT:    invoke void @do_throw()
@@ -159,7 +159,7 @@
 define void @catch_all_landingpad() personality ptr @__gxx_personality_v0 {
 ; CHECK: Function Attrs: noreturn nounwind
 ; CHECK-LABEL: define {{[^@]+}}@catch_all_landingpad
-; CHECK-SAME: () #[[ATTR3]] personality ptr @__gxx_personality_v0 {
+; CHECK-SAME: () #[[ATTR4:[0-9]+]] personality ptr @__gxx_personality_v0 {
 ; CHECK-NEXT:    invoke void @do_throw()
 ; CHECK-NEXT:    to label [[UNREACHABLE:%.*]] unwind label [[LPAD:%.*]]
 ; CHECK:       lpad:
@@ -184,7 +184,7 @@
 }
 
 define void @filter_specific_landingpad() personality ptr @__gxx_personality_v0 {
-; CHECK: Function Attrs: noreturn nounwind
+; CHECK: Function Attrs: noreturn
 ; CHECK-LABEL: define {{[^@]+}}@filter_specific_landingpad
 ; CHECK-SAME: () #[[ATTR3]] personality ptr @__gxx_personality_v0 {
 ; CHECK-NEXT:    invoke void @do_throw()
@@ -213,7 +213,7 @@
 define void @filter_none_landingpad() personality ptr @__gxx_personality_v0 {
 ; CHECK: Function Attrs: noreturn nounwind
 ; CHECK-LABEL: define {{[^@]+}}@filter_none_landingpad
-; CHECK-SAME: () #[[ATTR3]] personality ptr @__gxx_personality_v0 {
+; CHECK-SAME: () #[[ATTR4]] personality ptr @__gxx_personality_v0 {
 ; CHECK-NEXT:    invoke void @do_throw()
 ; CHECK-NEXT:    to label [[UNREACHABLE:%.*]] unwind label [[LPAD:%.*]]
 ; CHECK:       lpad:
@@ -238,7 +238,7 @@
 }
 
 define void @cleanup_landingpad() personality ptr @__gxx_personality_v0 {
-; CHECK: Function Attrs: noreturn nounwind
+; CHECK: Function Attrs: noreturn
 ; CHECK-LABEL: define {{[^@]+}}@cleanup_landingpad
 ; CHECK-SAME: () #[[ATTR3]] personality ptr @__gxx_personality_v0 {
 ; CHECK-NEXT:    invoke void @do_throw()
Index: llvm/lib/Transforms/IPO/FunctionAttrs.cpp
===================================================================
--- llvm/lib/Transforms/IPO/FunctionAttrs.cpp
+++ llvm/lib/Transforms/IPO/FunctionAttrs.cpp
@@ -1376,8 +1376,32 @@
          !SCCNodes.contains(CB->getCalledFunction());
 }
 
+static bool canUnwindPastLandingPad(const LandingPadInst *LP) {
+  // 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 true;
+
+  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;
+}
+
 /// Helper for NoUnwind inference predicate InstrBreaksAttribute.
 static bool InstrBreaksNonThrowing(Instruction &I, const SCCNodeSet &SCCNodes) {
+  if (auto *LP = dyn_cast<LandingPadInst>(&I))
+    return canUnwindPastLandingPad(LP);
+
   if (!I.mayThrow())
     return false;
   if (const auto *CI = dyn_cast<CallInst>(&I)) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D147694.511343.patch
Type: text/x-patch
Size: 3660 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230406/349a6b49/attachment.bin>