[CodeGen][ObjC] Make block copy/dispose helper functions exception-safe.
When an exception is thrown in a block copy helper function, captured
objects that have previously been copied should be destructed or
released. Similarly, captured objects that are yet to be released should
be released when an exception is thrown in a dispose helper function.
rdar://problem/42410255
Differential Revision: https://reviews.llvm.org/D49718
llvm-svn: 338041
diff --git a/clang/lib/CodeGen/CGBlocks.cpp b/clang/lib/CodeGen/CGBlocks.cpp
index 352a02d..617856a 100644
--- a/clang/lib/CodeGen/CGBlocks.cpp
+++ b/clang/lib/CodeGen/CGBlocks.cpp
@@ -1585,6 +1585,64 @@
}
}
+namespace {
+/// Release a __block variable.
+struct CallBlockRelease final : EHScopeStack::Cleanup {
+ Address Addr;
+ BlockFieldFlags FieldFlags;
+ bool LoadBlockVarAddr;
+
+ CallBlockRelease(Address Addr, BlockFieldFlags Flags, bool LoadValue)
+ : Addr(Addr), FieldFlags(Flags), LoadBlockVarAddr(LoadValue) {}
+
+ void Emit(CodeGenFunction &CGF, Flags flags) override {
+ llvm::Value *BlockVarAddr;
+ if (LoadBlockVarAddr) {
+ BlockVarAddr = CGF.Builder.CreateLoad(Addr);
+ BlockVarAddr = CGF.Builder.CreateBitCast(BlockVarAddr, CGF.VoidPtrTy);
+ } else {
+ BlockVarAddr = Addr.getPointer();
+ }
+
+ CGF.BuildBlockRelease(BlockVarAddr, FieldFlags);
+ }
+};
+} // end anonymous namespace
+
+static void pushCaptureCleanup(BlockCaptureEntityKind CaptureKind,
+ Address Field, QualType CaptureType,
+ BlockFieldFlags Flags, bool EHOnly,
+ CodeGenFunction &CGF) {
+ switch (CaptureKind) {
+ case BlockCaptureEntityKind::CXXRecord:
+ case BlockCaptureEntityKind::ARCWeak:
+ case BlockCaptureEntityKind::NonTrivialCStruct:
+ case BlockCaptureEntityKind::ARCStrong: {
+ if (CaptureType.isDestructedType() &&
+ (!EHOnly || CGF.needsEHCleanup(CaptureType.isDestructedType()))) {
+ CodeGenFunction::Destroyer *Destroyer =
+ CaptureKind == BlockCaptureEntityKind::ARCStrong
+ ? CodeGenFunction::destroyARCStrongImprecise
+ : CGF.getDestroyer(CaptureType.isDestructedType());
+ CleanupKind Kind =
+ EHOnly ? EHCleanup
+ : CGF.getCleanupKind(CaptureType.isDestructedType());
+ CGF.pushDestroy(Kind, Field, CaptureType, Destroyer, Kind & EHCleanup);
+ }
+ break;
+ }
+ case BlockCaptureEntityKind::BlockObject: {
+ if (!EHOnly || CGF.getLangOpts().Exceptions) {
+ CleanupKind Kind = EHOnly ? EHCleanup : NormalAndEHCleanup;
+ CGF.enterByrefCleanup(Kind, Field, Flags, /*LoadBlockVarAddr*/ true);
+ }
+ break;
+ }
+ case BlockCaptureEntityKind::None:
+ llvm_unreachable("unexpected BlockCaptureEntityKind");
+ }
+}
+
/// Generate the copy-helper function for a block closure object:
/// static void block_copy_helper(block_t *dst, block_t *src);
/// The runtime will have previously initialized 'dst' by doing a
@@ -1648,6 +1706,7 @@
for (const auto &CopiedCapture : CopiedCaptures) {
const BlockDecl::Capture &CI = CopiedCapture.CI;
const CGBlockInfo::Capture &capture = CopiedCapture.Capture;
+ QualType captureType = CI.getVariable()->getType();
BlockFieldFlags flags = CopiedCapture.Flags;
unsigned index = capture.getIndex();
@@ -1685,9 +1744,11 @@
} else {
EmitARCRetainNonBlock(srcValue);
- // We don't need this anymore, so kill it. It's not quite
- // worth the annoyance to avoid creating it in the first place.
- cast<llvm::Instruction>(dstField.getPointer())->eraseFromParent();
+ // Unless EH cleanup is required, we don't need this anymore, so kill
+ // it. It's not quite worth the annoyance to avoid creating it in the
+ // first place.
+ if (!needsEHCleanup(captureType.isDestructedType()))
+ cast<llvm::Instruction>(dstField.getPointer())->eraseFromParent();
}
} else {
assert(CopiedCapture.Kind == BlockCaptureEntityKind::BlockObject);
@@ -1715,6 +1776,11 @@
}
}
}
+
+ // Ensure that we destroy the copied object if an exception is thrown later
+ // in the helper function.
+ pushCaptureCleanup(CopiedCapture.Kind, dstField, captureType, flags, /*EHOnly*/ true,
+ *this);
}
FinishFunction();
@@ -1830,36 +1896,8 @@
Address srcField =
Builder.CreateStructGEP(src, capture.getIndex(), capture.getOffset());
- // If the captured record has a destructor then call it.
- if (DestroyedCapture.Kind == BlockCaptureEntityKind::CXXRecord) {
- const auto *Dtor =
- CI.getVariable()->getType()->getAsCXXRecordDecl()->getDestructor();
- PushDestructorCleanup(Dtor, srcField);
-
- // If this is a __weak capture, emit the release directly.
- } else if (DestroyedCapture.Kind == BlockCaptureEntityKind::ARCWeak) {
- EmitARCDestroyWeak(srcField);
-
- // Destroy strong objects with a call if requested.
- } else if (DestroyedCapture.Kind == BlockCaptureEntityKind::ARCStrong) {
- EmitARCDestroyStrong(srcField, ARCImpreciseLifetime);
-
- // If this is a C struct that requires non-trivial destruction, emit a call
- // to its destructor.
- } else if (DestroyedCapture.Kind ==
- BlockCaptureEntityKind::NonTrivialCStruct) {
- QualType varType = CI.getVariable()->getType();
- pushDestroy(varType.isDestructedType(), srcField, varType);
-
- // Otherwise we call _Block_object_dispose. It wouldn't be too
- // hard to just emit this as a cleanup if we wanted to make sure
- // that things were done in reverse.
- } else {
- assert(DestroyedCapture.Kind == BlockCaptureEntityKind::BlockObject);
- llvm::Value *value = Builder.CreateLoad(srcField);
- value = Builder.CreateBitCast(value, VoidPtrTy);
- BuildBlockRelease(value, flags);
- }
+ pushCaptureCleanup(DestroyedCapture.Kind, srcField,
+ CI.getVariable()->getType(), flags, /*EHOnly*/ false, *this);
}
cleanups.ForceCleanup();
@@ -2538,30 +2576,10 @@
EmitNounwindRuntimeCall(F, args); // FIXME: throwing destructors?
}
-namespace {
- /// Release a __block variable.
- struct CallBlockRelease final : EHScopeStack::Cleanup {
- llvm::Value *Addr;
- CallBlockRelease(llvm::Value *Addr) : Addr(Addr) {}
-
- void Emit(CodeGenFunction &CGF, Flags flags) override {
- // Should we be passing FIELD_IS_WEAK here?
- CGF.BuildBlockRelease(Addr, BLOCK_FIELD_IS_BYREF);
- }
- };
-} // end anonymous namespace
-
-/// Enter a cleanup to destroy a __block variable. Note that this
-/// cleanup should be a no-op if the variable hasn't left the stack
-/// yet; if a cleanup is required for the variable itself, that needs
-/// to be done externally.
-void CodeGenFunction::enterByrefCleanup(const AutoVarEmission &emission) {
- // We don't enter this cleanup if we're in pure-GC mode.
- if (CGM.getLangOpts().getGC() == LangOptions::GCOnly)
- return;
-
- EHStack.pushCleanup<CallBlockRelease>(NormalAndEHCleanup,
- emission.Addr.getPointer());
+void CodeGenFunction::enterByrefCleanup(CleanupKind Kind, Address Addr,
+ BlockFieldFlags Flags,
+ bool LoadBlockVarAddr) {
+ EHStack.pushCleanup<CallBlockRelease>(Kind, Addr, Flags, LoadBlockVarAddr);
}
/// Adjust the declaration of something from the blocks API.
diff --git a/clang/lib/CodeGen/CGDecl.cpp b/clang/lib/CodeGen/CGDecl.cpp
index d1df5b3..57b2fba 100644
--- a/clang/lib/CodeGen/CGDecl.cpp
+++ b/clang/lib/CodeGen/CGDecl.cpp
@@ -1710,9 +1710,15 @@
}
// If this is a block variable, call _Block_object_destroy
- // (on the unforwarded address).
- if (emission.IsByRef)
- enterByrefCleanup(emission);
+ // (on the unforwarded address). Don't enter this cleanup if we're in pure-GC
+ // mode.
+ if (emission.IsByRef && CGM.getLangOpts().getGC() != LangOptions::GCOnly) {
+ BlockFieldFlags Flags = BLOCK_FIELD_IS_BYREF;
+ if (emission.Variable->getType().isObjCGCWeak())
+ Flags |= BLOCK_FIELD_IS_WEAK;
+ enterByrefCleanup(NormalAndEHCleanup, emission.Addr, Flags,
+ /*LoadBlockVarAddr*/ false);
+ }
}
CodeGenFunction::Destroyer *
diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h
index 752d670..79870ed 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -1758,7 +1758,25 @@
class AutoVarEmission;
void emitByrefStructureInit(const AutoVarEmission &emission);
- void enterByrefCleanup(const AutoVarEmission &emission);
+
+ /// Enter a cleanup to destroy a __block variable. Note that this
+ /// cleanup should be a no-op if the variable hasn't left the stack
+ /// yet; if a cleanup is required for the variable itself, that needs
+ /// to be done externally.
+ ///
+ /// \param Kind Cleanup kind.
+ ///
+ /// \param Addr When \p LoadBlockVarAddr is false, the address of the __block
+ /// structure that will be passed to _Block_object_dispose. When
+ /// \p LoadBlockVarAddr is true, the address of the field of the block
+ /// structure that holds the address of the __block structure.
+ ///
+ /// \param Flags The flag that will be passed to _Block_object_dispose.
+ ///
+ /// \param LoadBlockVarAddr Indicates whether we need to emit a load from
+ /// \p Addr to get the address of the __block structure.
+ void enterByrefCleanup(CleanupKind Kind, Address Addr, BlockFieldFlags Flags,
+ bool LoadBlockVarAddr);
void setBlockContextParameter(const ImplicitParamDecl *D, unsigned argNum,
llvm::Value *ptr);