[clang] a9a6e62 - [CodeGen] Make sure the EH cleanup for block captures is conditional when the block literal is in a conditional context
Erik Pilkington via cfe-commits
cfe-commits at lists.llvm.org
Mon Aug 31 07:12:34 PDT 2020
Author: Erik Pilkington
Date: 2020-08-31T10:12:17-04:00
New Revision: a9a6e62ddff21c597b82f0f6d26bca6a1a473a6f
URL: https://github.com/llvm/llvm-project/commit/a9a6e62ddff21c597b82f0f6d26bca6a1a473a6f
DIFF: https://github.com/llvm/llvm-project/commit/a9a6e62ddff21c597b82f0f6d26bca6a1a473a6f.diff
LOG: [CodeGen] Make sure the EH cleanup for block captures is conditional when the block literal is in a conditional context
Previously, clang was crashing on the attached test because the EH cleanup for
the block capture was incorrectly emitted under the assumption that the
expression wasn't conditionally evaluated. This was because before 9a52de00260,
pushLifetimeExtendedDestroy was mainly used with C++ automatic lifetime
extension, where a conditionally evaluated expression wasn't possible. Now that
we're using this path for block captures, we need to handle this case.
rdar://66250047
Differential revision: https://reviews.llvm.org/D86854
Added:
clang/test/CodeGenObjC/arc-blocks-exceptions.m
Modified:
clang/lib/CodeGen/CGDecl.cpp
clang/lib/CodeGen/CodeGenFunction.h
Removed:
################################################################################
diff --git a/clang/lib/CodeGen/CGDecl.cpp b/clang/lib/CodeGen/CGDecl.cpp
index 0ad5050fabbb..7daf91ff73e3 100644
--- a/clang/lib/CodeGen/CGDecl.cpp
+++ b/clang/lib/CodeGen/CGDecl.cpp
@@ -2095,21 +2095,47 @@ void CodeGenFunction::pushStackRestore(CleanupKind Kind, Address SPMem) {
EHStack.pushCleanup<CallStackRestore>(Kind, SPMem);
}
-void CodeGenFunction::pushLifetimeExtendedDestroy(
- CleanupKind cleanupKind, Address addr, QualType type,
- Destroyer *destroyer, bool useEHCleanupForArray) {
- // Push an EH-only cleanup for the object now.
- // FIXME: When popping normal cleanups, we need to keep this EH cleanup
- // around in case a temporary's destructor throws an exception.
- if (cleanupKind & EHCleanup)
- EHStack.pushCleanup<DestroyObject>(
- static_cast<CleanupKind>(cleanupKind & ~NormalCleanup), addr, type,
+void CodeGenFunction::pushLifetimeExtendedDestroy(CleanupKind cleanupKind,
+ Address addr, QualType type,
+ Destroyer *destroyer,
+ bool useEHCleanupForArray) {
+ // If we're not in a conditional branch, we don't need to bother generating a
+ // conditional cleanup.
+ if (!isInConditionalBranch()) {
+ // Push an EH-only cleanup for the object now.
+ // FIXME: When popping normal cleanups, we need to keep this EH cleanup
+ // around in case a temporary's destructor throws an exception.
+ if (cleanupKind & EHCleanup)
+ EHStack.pushCleanup<DestroyObject>(
+ static_cast<CleanupKind>(cleanupKind & ~NormalCleanup), addr, type,
+ destroyer, useEHCleanupForArray);
+
+ return pushCleanupAfterFullExprWithActiveFlag<DestroyObject>(
+ cleanupKind, Address::invalid(), addr, type, destroyer, useEHCleanupForArray);
+ }
+
+ // Otherwise, we should only destroy the object if it's been initialized.
+ // Re-use the active flag and saved address across both the EH and end of
+ // scope cleanups.
+
+ using SavedType = typename DominatingValue<Address>::saved_type;
+ using ConditionalCleanupType =
+ EHScopeStack::ConditionalCleanup<DestroyObject, Address, QualType,
+ Destroyer *, bool>;
+
+ Address ActiveFlag = createCleanupActiveFlag();
+ SavedType SavedAddr = saveValueInCond(addr);
+
+ if (cleanupKind & EHCleanup) {
+ EHStack.pushCleanup<ConditionalCleanupType>(
+ static_cast<CleanupKind>(cleanupKind & ~NormalCleanup), SavedAddr, type,
destroyer, useEHCleanupForArray);
+ initFullExprCleanupWithFlag(ActiveFlag);
+ }
- // Remember that we need to push a full cleanup for the object at the
- // end of the full-expression.
- pushCleanupAfterFullExpr<DestroyObject>(
- cleanupKind, addr, type, destroyer, useEHCleanupForArray);
+ pushCleanupAfterFullExprWithActiveFlag<ConditionalCleanupType>(
+ cleanupKind, ActiveFlag, SavedAddr, type, destroyer,
+ useEHCleanupForArray);
}
/// emitDestroy - Immediately perform the destruction of the given
diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h
index 77671b0a9952..b4f8b11c0cd3 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -672,12 +672,13 @@ class CodeGenFunction : public CodeGenTypeCache {
initFullExprCleanup();
}
- /// Queue a cleanup to be pushed after finishing the current
- /// full-expression.
+ /// Queue a cleanup to be pushed after finishing the current full-expression,
+ /// potentially with an active flag.
template <class T, class... As>
void pushCleanupAfterFullExpr(CleanupKind Kind, As... A) {
if (!isInConditionalBranch())
- return pushCleanupAfterFullExprImpl<T>(Kind, Address::invalid(), A...);
+ return pushCleanupAfterFullExprWithActiveFlag<T>(Kind, Address::invalid(),
+ A...);
Address ActiveFlag = createCleanupActiveFlag();
assert(!DominatingValue<Address>::needsSaving(ActiveFlag) &&
@@ -687,12 +688,12 @@ class CodeGenFunction : public CodeGenTypeCache {
SavedTuple Saved{saveValueInCond(A)...};
typedef EHScopeStack::ConditionalCleanup<T, As...> CleanupType;
- pushCleanupAfterFullExprImpl<CleanupType>(Kind, ActiveFlag, Saved);
+ pushCleanupAfterFullExprWithActiveFlag<CleanupType>(Kind, ActiveFlag, Saved);
}
template <class T, class... As>
- void pushCleanupAfterFullExprImpl(CleanupKind Kind, Address ActiveFlag,
- As... A) {
+ void pushCleanupAfterFullExprWithActiveFlag(CleanupKind Kind,
+ Address ActiveFlag, As... A) {
LifetimeExtendedCleanupHeader Header = {sizeof(T), Kind,
ActiveFlag.isValid()};
diff --git a/clang/test/CodeGenObjC/arc-blocks-exceptions.m b/clang/test/CodeGenObjC/arc-blocks-exceptions.m
new file mode 100644
index 000000000000..ae67d5a32b2c
--- /dev/null
+++ b/clang/test/CodeGenObjC/arc-blocks-exceptions.m
@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm -fblocks -fobjc-arc -fobjc-runtime-has-weak -fexceptions -disable-llvm-passes -o - %s | FileCheck %s
+
+void test1(_Bool c) {
+ void test1_fn(void (^blk)());
+ __weak id weakId = 0;
+ test1_fn(c ? ^{ (void)weakId; } : 0);
+
+ // CHECK: [[CLEANUP_COND:%.*]] = alloca i1
+ // CHECK-NEXT: [[CLEANUP_SAVE:%.*]] = alloca i8**
+
+ // CHECK: store i1 true, i1* [[CLEANUP_COND]]
+ // CHECK-NEXT: store i8** {{.*}}, i8*** [[CLEANUP_SAVE]]
+
+ // CHECK: invoke void @test1_fn(
+ // CHECK-NEXT: to label %[[INVOKE_CONT:.*]] unwind label %[[LANDING_PAD_LAB:.*]]
+
+ // CHECK: [[INVOKE_CONT]]:
+ // CHECK-NEXT: [[LOAD:%.*]] = load i1, i1* [[CLEANUP_COND]]
+ // CHECK-NEXT: br i1 [[LOAD]], label %[[END_OF_SCOPE_LAB:.*]], label
+
+ // CHECK: [[END_OF_SCOPE_LAB]]:
+ // CHECK-NEXT: [[LOAD:%.*]] = load i8**, i8*** [[CLEANUP_SAVE]]
+ // CHECK-NEXT: call void @llvm.objc.destroyWeak(i8** [[LOAD]])
+ // CHECK-NEXT: br label
+
+ // CHECK: [[LANDING_PAD_LAB]]:
+ // /* some EH stuff */
+ // CHECK: [[LOAD:%.*]] = load i1, i1* [[CLEANUP_COND]]
+ // CHECK-NEXT: br i1 [[LOAD]], label %[[EH_CLEANUP_LAB:.*]], label
+
+ // CHECK: [[EH_CLEANUP_LAB]]:
+ // CHECK-NEXT: [[LOAD:%.*]] = load i8**, i8*** [[CLEANUP_SAVE]]
+ // CHECK-NEXT: call void @llvm.objc.destroyWeak(i8** [[LOAD]])
+ // CHECK-NEXT: br label
+}
More information about the cfe-commits
mailing list