[FunctionAttrs] Fix nounwind inference for landingpads

[nikic]

Summary

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.

[efriedma]

While we're here, can you also add handling for catchpad/cleanuppad? (A conservative implementation for them is fine for now.)

[nikic]

In D147694#4249290, @efriedma wrote:

While we're here, can you also add handling for catchpad/cleanuppad? (A conservative implementation for them is fine for now.)

I think those are effectively already handled via catchswitch here: https://github.com/llvm/llvm-project/blob/aee4399f58f4826180138fb294b6b0359c892709/llvm/lib/IR/Instruction.cpp#L741-L742

[efriedma]

Doesn't that have the same problem as the issue you're trying to fix? Specifically, if there's a catchswitch pointing to a cleanuppad that ends in "unreachable", mayThrow() returns false for both the catchswitch and the cleanuppad.

[nikic]

In D147694#4249509, @efriedma wrote:

Doesn't that have the same problem as the issue you're trying to fix? Specifically, if there's a catchswitch pointing to a cleanuppad that ends in "unreachable", mayThrow() returns false for both the catchswitch and the cleanuppad.

Yeah, you're right. In that case the catchswich is not "unwind to caller" so it won't be considered mayThrow() and we do need separate handling for the cleanuppad.

[nikic]

Also handle cleanuppad.

[jdoerfert]

I think we should fold this logic into mayThrow, e.g., via flags picking a behavior. Other places are broken as well and should default to this semantics.

[nikic]

Move logic to mayThrow() behind a flag. I tried doing this without the flag initially, but this does affect some optimizations.

I've opted to make the invoke throwing, rather than the landingpad itself. I think this more closely matches reality, in that it's not the landingpad that's going to throw, it's the invoke that will fail to catch the exception and continue unwinding.

Note that the logic for non-cleanup landingpads applies always, even without the flag. That part should be independent of phase one unwinding.

[efriedma]

llvm/lib/IR/Instruction.cpp

775 ↗

(On Diff #512689)

The "pad" in the destination on Windows can be either a cleanuppad or a catchswitch; you might need to handle both cases? (Either that, or change the way catchswitch is handled.)

[nikic]

I'm handling the cleanuppad itself as throwing now, so we don't need to consider all the different places it can occur (invoke, catchswitch, cleanupret). As this is for the phase-one case only it doesn't really matter whether the cleanuppad itself unwinds or whatever is calling it.

[efriedma]

Looks fine, unless @jdoerfert has some comment on the API.

llvm/lib/Transforms/IPO/FunctionAttrs.cpp

static bool InstrBreaksNonConvergent(Instruction &I,
                                      const SCCNodeSet &SCCNodes) {
   const CallBase *CB = dyn_cast<CallBase>(&I);
   // Breaks non-convergent assumption if CS is a convergent call to a function
   // not in the SCC.
   return CB && CB->isConvergent() &&
          !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)) {
     if (Function *Callee = CI->getCalledFunction()) {
       // I is a may-throw call to a function inside our SCC. This doesn't
       // invalidate our current working assumption that the SCC is no-throw; we
       // just have to scan that other function.
       if (SCCNodes.contains(Callee))

llvm/test/Transforms/FunctionAttrs/nounwind.ll

;
   ret i32 %catch_thing_call
 }
 
 declare void @do_throw()
 declare void @abort() nounwind
 @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()
 ; CHECK-NEXT:    to label [[UNREACHABLE:%.*]] unwind label [[LPAD:%.*]]
 ; CHECK:       lpad:
 ; CHECK-NEXT:    [[LP:%.*]] = landingpad { ptr, i32 }
 ; CHECK-NEXT:    catch ptr @catch_ty
 ; CHECK-NEXT:    call void @abort()

 
 unreachable:
   unreachable
 }
 
 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:
 ; CHECK-NEXT:    [[LP:%.*]] = landingpad { ptr, i32 }
 ; CHECK-NEXT:    catch ptr null
 ; CHECK-NEXT:    call void @abort()
 ; CHECK-NEXT:    unreachable
 ; CHECK:       unreachable:
 ; CHECK-NEXT:    unreachable
 ;
   invoke void @do_throw()
   to label %unreachable unwind label %lpad
 
 lpad:
   %lp = landingpad { ptr, i32 }
   catch ptr null
   call void @abort()
   unreachable
 
 unreachable:
   unreachable
 }
 
 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()
 ; CHECK-NEXT:    to label [[UNREACHABLE:%.*]] unwind label [[LPAD:%.*]]
 ; CHECK:       lpad:
 ; CHECK-NEXT:    [[LP:%.*]] = landingpad { ptr, i32 }
 ; CHECK-NEXT:    filter [1 x ptr] [ptr @catch_ty]
 ; CHECK-NEXT:    call void @abort()

 
 unreachable:
   unreachable
 }
 
 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:
 ; CHECK-NEXT:    [[LP:%.*]] = landingpad { ptr, i32 }
 ; CHECK-NEXT:    filter [0 x ptr] zeroinitializer
 ; CHECK-NEXT:    call void @abort()
 ; CHECK-NEXT:    unreachable
 ; CHECK:       unreachable:
 ; CHECK-NEXT:    unreachable
 ;
   invoke void @do_throw()
   to label %unreachable unwind label %lpad
 
 lpad:
   %lp = landingpad { ptr, i32 }
   filter [0 x ptr] zeroinitializer
   call void @abort()
   unreachable
 
 unreachable:
   unreachable
 }
 
 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()
 ; CHECK-NEXT:    to label [[UNREACHABLE:%.*]] unwind label [[LPAD:%.*]]
 ; CHECK:       lpad:
 ; CHECK-NEXT:    [[LP:%.*]] = landingpad { ptr, i32 }
 ; CHECK-NEXT:    cleanup
 ; CHECK-NEXT:    call void @abort()