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;
}
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;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
