[PATCH] D124680: [WinEHPrepare] Fix accidental truncation of EH funclets with GNUstep ObjC runtime

[Stefan Gränitz via Phabricator via llvm-commits]

sgraenitz created this revision.
sgraenitz added reviewers: theraven, ahatanak, majnemer, JosephTremoulet, triplef.
Herald added a subscriber: hiraditya.
Herald added a project: All.
sgraenitz requested review of this revision.
Herald added a project: LLVM.

Unwinding ObjC code involves calls to intrinsics like `llvm.obj.retain` and `llvm.objc.destroyWeak`. Exception handling on Windows is based on funclets and WinEHPrepare only allows calls to `nounwind` intrinsics. This works just fine, except for ObjC `nounwind` intrinsics, because these are lowered into regular function calls in an earlier stage already (i.e. PreISelIntrinsicLoweringPass). Thus, WinEHPrepare accidentally drops them as implausible instructions and silently truncates certain funclets. Eventually, this causes crashes during unwinding on Windows with the GNUstep ObjC runtime [1].

This patch is a first attempt to fix the issue: PreISelIntrinsicLoweringPass passes on `nounwind` attributes to lowered call sites and WinEHPrepare gets a little less conservative by allowing any `nounwind` calls. It fixes the issue in GNUstep libobjc2 [1] and related projects. I will work on a test case during the next weeks and I understand if this is not immediately ready to land upstream. For the moment, I am looking for feedback to avoid unintended side effects.

Thanks for all input!

[1] https://github.com/gnustep/libobjc2/issues/222


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124680

Files:
  llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
  llvm/lib/CodeGen/WinEHPrepare.cpp


Index: llvm/lib/CodeGen/WinEHPrepare.cpp
===================================================================
--- llvm/lib/CodeGen/WinEHPrepare.cpp
+++ llvm/lib/CodeGen/WinEHPrepare.cpp
@@ -966,12 +966,10 @@
         if (FuncletBundleOperand == FuncletPad)
           continue;
 
-        // Skip call sites which are nounwind intrinsics or inline asm.
-        auto *CalledFn =
-            dyn_cast<Function>(CB->getCalledOperand()->stripPointerCasts());
-        if (CalledFn && ((CalledFn->isIntrinsic() && CB->doesNotThrow()) ||
-                         CB->isInlineAsm()))
-          continue;
+        // Skip call sites which are nounwind or inline asm.
+        if (isa<Function>(CB->getCalledOperand()->stripPointerCasts()))
+          if (CB->doesNotThrow() || CB->isInlineAsm())
+            continue;
 
         // This call site was not part of this funclet, remove it.
         if (isa<InvokeInst>(CB)) {
Index: llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
===================================================================
--- llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
+++ llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
@@ -109,6 +109,8 @@
     SmallVector<Value *, 8> Args(CI->args());
     CallInst *NewCI = Builder.CreateCall(FCache, Args);
     NewCI->setName(CI->getName());
+    if (CI->hasFnAttr(Attribute::NoUnwind))
+      NewCI->addFnAttr(Attribute::NoUnwind);
 
     // Try to set the most appropriate TailCallKind based on both the current
     // attributes and the ones that we could get from ObjCARC's special


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D124680.426078.patch
Type: text/x-patch
Size: 1546 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220429/27c867ff/attachment.bin>