Skip to content

Commit 81bc77b

Browse files
committed
[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
1 parent d6cfed4 commit 81bc77b

File tree

3 files changed

+81
-19
lines changed

3 files changed

+81
-19
lines changed

clang/lib/CodeGen/CGDecl.cpp

Lines changed: 39 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2094,21 +2094,47 @@ void CodeGenFunction::pushStackRestore(CleanupKind Kind, Address SPMem) {
20942094
EHStack.pushCleanup<CallStackRestore>(Kind, SPMem);
20952095
}
20962096

2097-
void CodeGenFunction::pushLifetimeExtendedDestroy(
2098-
CleanupKind cleanupKind, Address addr, QualType type,
2099-
Destroyer *destroyer, bool useEHCleanupForArray) {
2100-
// Push an EH-only cleanup for the object now.
2101-
// FIXME: When popping normal cleanups, we need to keep this EH cleanup
2102-
// around in case a temporary's destructor throws an exception.
2103-
if (cleanupKind & EHCleanup)
2104-
EHStack.pushCleanup<DestroyObject>(
2105-
static_cast<CleanupKind>(cleanupKind & ~NormalCleanup), addr, type,
2097+
void CodeGenFunction::pushLifetimeExtendedDestroy(CleanupKind cleanupKind,
2098+
Address addr, QualType type,
2099+
Destroyer *destroyer,
2100+
bool useEHCleanupForArray) {
2101+
// If we're not in a conditional branch, we don't need to bother generating a
2102+
// conditional cleanup.
2103+
if (!isInConditionalBranch()) {
2104+
// Push an EH-only cleanup for the object now.
2105+
// FIXME: When popping normal cleanups, we need to keep this EH cleanup
2106+
// around in case a temporary's destructor throws an exception.
2107+
if (cleanupKind & EHCleanup)
2108+
EHStack.pushCleanup<DestroyObject>(
2109+
static_cast<CleanupKind>(cleanupKind & ~NormalCleanup), addr, type,
2110+
destroyer, useEHCleanupForArray);
2111+
2112+
return pushCleanupAfterFullExprWithActiveFlag<DestroyObject>(
2113+
cleanupKind, Address::invalid(), addr, type, destroyer, useEHCleanupForArray);
2114+
}
2115+
2116+
// Otherwise, we should only destroy the object if it's been initialized.
2117+
// Re-use the active flag and saved address across both the EH and end of
2118+
// scope cleanups.
2119+
2120+
using SavedType = typename DominatingValue<Address>::saved_type;
2121+
using ConditionalCleanupType =
2122+
EHScopeStack::ConditionalCleanup<DestroyObject, Address, QualType,
2123+
Destroyer *, bool>;
2124+
2125+
Address ActiveFlag = createCleanupActiveFlag();
2126+
SavedType SavedAddr = saveValueInCond(addr);
2127+
2128+
if (cleanupKind & EHCleanup) {
2129+
EHStack.pushCleanup<ConditionalCleanupType>(
2130+
static_cast<CleanupKind>(cleanupKind & ~NormalCleanup), SavedAddr, type,
21062131
destroyer, useEHCleanupForArray);
2132+
initFullExprCleanupWithFlag(ActiveFlag);
2133+
}
21072134

2108-
// Remember that we need to push a full cleanup for the object at the
2109-
// end of the full-expression.
2110-
pushCleanupAfterFullExpr<DestroyObject>(
2111-
cleanupKind, addr, type, destroyer, useEHCleanupForArray);
2135+
pushCleanupAfterFullExprWithActiveFlag<ConditionalCleanupType>(
2136+
cleanupKind, ActiveFlag, SavedAddr, type, destroyer,
2137+
useEHCleanupForArray);
21122138
}
21132139

21142140
/// emitDestroy - Immediately perform the destruction of the given

clang/lib/CodeGen/CodeGenFunction.h

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -672,12 +672,13 @@ class CodeGenFunction : public CodeGenTypeCache {
672672
initFullExprCleanup();
673673
}
674674

675-
/// Queue a cleanup to be pushed after finishing the current
676-
/// full-expression.
675+
/// Queue a cleanup to be pushed after finishing the current full-expression,
676+
/// potentially with an active flag.
677677
template <class T, class... As>
678678
void pushCleanupAfterFullExpr(CleanupKind Kind, As... A) {
679679
if (!isInConditionalBranch())
680-
return pushCleanupAfterFullExprImpl<T>(Kind, Address::invalid(), A...);
680+
return pushCleanupAfterFullExprWithActiveFlag<T>(Kind, Address::invalid(),
681+
A...);
681682

682683
Address ActiveFlag = createCleanupActiveFlag();
683684
assert(!DominatingValue<Address>::needsSaving(ActiveFlag) &&
@@ -687,12 +688,12 @@ class CodeGenFunction : public CodeGenTypeCache {
687688
SavedTuple Saved{saveValueInCond(A)...};
688689

689690
typedef EHScopeStack::ConditionalCleanup<T, As...> CleanupType;
690-
pushCleanupAfterFullExprImpl<CleanupType>(Kind, ActiveFlag, Saved);
691+
pushCleanupAfterFullExprWithActiveFlag<CleanupType>(Kind, ActiveFlag, Saved);
691692
}
692693

693694
template <class T, class... As>
694-
void pushCleanupAfterFullExprImpl(CleanupKind Kind, Address ActiveFlag,
695-
As... A) {
695+
void pushCleanupAfterFullExprWithActiveFlag(CleanupKind Kind,
696+
Address ActiveFlag, As... A) {
696697
LifetimeExtendedCleanupHeader Header = {sizeof(T), Kind,
697698
ActiveFlag.isValid()};
698699

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// 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
2+
3+
void test1(_Bool c) {
4+
void test1_fn(void (^blk)());
5+
__weak id weakId = 0;
6+
test1_fn(c ? ^{ (void)weakId; } : 0);
7+
8+
// CHECK: [[CLEANUP_COND:%.*]] = alloca i1
9+
// CHECK-NEXT: [[CLEANUP_SAVE:%.*]] = alloca i8**
10+
11+
// CHECK: store i1 true, i1* [[CLEANUP_COND]]
12+
// CHECK-NEXT: store i8** {{.*}}, i8*** [[CLEANUP_SAVE]]
13+
14+
// CHECK: invoke void @test1_fn(
15+
// CHECK-NEXT: to label %[[INVOKE_CONT:.*]] unwind label %[[LANDING_PAD_LAB:.*]]
16+
17+
// CHECK: [[INVOKE_CONT]]:
18+
// CHECK-NEXT: [[LOAD:%.*]] = load i1, i1* [[CLEANUP_COND]]
19+
// CHECK-NEXT: br i1 [[LOAD]], label %[[END_OF_SCOPE_LAB:.*]], label
20+
21+
// CHECK: [[END_OF_SCOPE_LAB]]:
22+
// CHECK-NEXT: [[LOAD:%.*]] = load i8**, i8*** [[CLEANUP_SAVE]]
23+
// CHECK-NEXT: call void @llvm.objc.destroyWeak(i8** [[LOAD]])
24+
// CHECK-NEXT: br label
25+
26+
// CHECK: [[LANDING_PAD_LAB]]:
27+
// /* some EH stuff */
28+
// CHECK: [[LOAD:%.*]] = load i1, i1* [[CLEANUP_COND]]
29+
// CHECK-NEXT: br i1 [[LOAD]], label %[[EH_CLEANUP_LAB:.*]], label
30+
31+
// CHECK: [[EH_CLEANUP_LAB]]:
32+
// CHECK-NEXT: [[LOAD:%.*]] = load i8**, i8*** [[CLEANUP_SAVE]]
33+
// CHECK-NEXT: call void @llvm.objc.destroyWeak(i8** [[LOAD]])
34+
// CHECK-NEXT: br label
35+
}

0 commit comments

Comments
 (0)