[PATCH] D124762: [WinEHPrepare] Avoid truncation of EH funclets with GNUstep ObjC runtime

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

sgraenitz created this revision.
sgraenitz added reviewers: rnk, theraven, DHowett-MSFT.
Herald added a subscriber: hiraditya.
Herald added a project: All.
sgraenitz requested review of this revision.
Herald added projects: clang, LLVM.
Herald added a subscriber: cfe-commits.

Unwinding ObjC code with automatic reference counting 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: https://github.com/gnustep/libobjc2/issues/222

This patch forces the emission of a "funclet" bundle operand for calls to ObjC ARC intrinsics during codegen and lets PreISelIntrinsicLowering carry it over onto lowered replacement calls. This way all ObjC ARC calls survive WinEHPrepare through the FuncletBundleOperand check. The latter had to be adjusted in order to allow funclet pads to get optimized away.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124762

Files:
  clang/lib/CodeGen/CGCall.cpp
  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
@@ -963,7 +963,7 @@
         if (auto BU = CB->getOperandBundle(LLVMContext::OB_funclet))
           FuncletBundleOperand = BU->Inputs.front();
 
-        if (FuncletBundleOperand == FuncletPad)
+        if (!FuncletPad || FuncletBundleOperand == FuncletPad)
           continue;
 
         // Skip call sites which are nounwind intrinsics or inline asm.
Index: llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
===================================================================
--- llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
+++ llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
@@ -107,7 +107,9 @@
 
     IRBuilder<> Builder(CI->getParent(), CI->getIterator());
     SmallVector<Value *, 8> Args(CI->args());
-    CallInst *NewCI = Builder.CreateCall(FCache, Args);
+    SmallVector<llvm::OperandBundleDef, 1> BundleList;
+    CI->getOperandBundlesAsDefs(BundleList);
+    CallInst *NewCI = Builder.CreateCall(FCache, Args, BundleList);
     NewCI->setName(CI->getName());
 
     // Try to set the most appropriate TailCallKind based on both the current
Index: clang/lib/CodeGen/CGCall.cpp
===================================================================
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -25,11 +25,13 @@
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/Basic/CodeGenOptions.h"
+#include "clang/Basic/ObjCRuntime.h"
 #include "clang/Basic/TargetBuiltins.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/CodeGen/CGFunctionInfo.h"
 #include "clang/CodeGen/SwiftCallingConv.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Analysis/ObjCARCInstKind.h"
 #include "llvm/Analysis/ValueTracking.h"
 #include "llvm/IR/Assumptions.h"
 #include "llvm/IR/Attributes.h"
@@ -4470,11 +4472,30 @@
     return BundleList;
 
   // Skip intrinsics which cannot throw.
+  bool InsertFuncletOp = true;
   auto *CalleeFn = dyn_cast<llvm::Function>(Callee->stripPointerCasts());
   if (CalleeFn && CalleeFn->isIntrinsic() && CalleeFn->doesNotThrow())
-    return BundleList;
+    InsertFuncletOp = false;
+
+  // ObjC ARC intrinics are lowered in PreISelIntrinsicLowering. Thus,
+  // WinEHPrepare will see them as regular calls. We need to set the funclet
+  // operand explicitly in this case to avoid accidental truncation of EH
+  // funclets on Windows.
+  using namespace llvm::objcarc;
+  if (GetFunctionClass(CalleeFn) != ARCInstKind::None) {
+    assert(CalleeFn->isIntrinsic() && CalleeFn->doesNotThrow() &&
+           "Right now these are nounwind intrinsics");
+    if (CGM.getTarget().getTriple().isOSWindows()) {
+      assert(CGM.getLangOpts().ObjCRuntime.getKind() == ObjCRuntime::GNUstep &&
+             "Only reproduced with GNUstep so far, but likely applies to other "
+             "ObjC runtimes on Windows");
+      InsertFuncletOp = true;
+    }
+  }
+
+  if (InsertFuncletOp)
+    BundleList.emplace_back("funclet", CurrentFuncletPad);
 
-  BundleList.emplace_back("funclet", CurrentFuncletPad);
   return BundleList;
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D124762.426381.patch
Type: text/x-patch
Size: 3196 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220502/7fb45738/attachment.bin>