Skip to content

Commit fd13abb

Browse files
SirraideSirraide
committed
Save and restore the current cleanup destination
1 parent 2ace560 commit fd13abb

File tree

3 files changed

+227
-68
lines changed

3 files changed

+227
-68
lines changed

‎clang/lib/CodeGen/CGStmt.cpp‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2060,15 +2060,23 @@ struct EmitDeferredStatement final : EHScopeStack::Cleanup {
20602060
// cleanup. The result is we just branch 'somewhere random'.
20612061
//
20622062
// The rest of the cleanup code simply isn't prepared to handle this case
2063-
// because there are no other cleanups that can push more cleanups, and
2064-
// thus, emitting any other cleanup cannot clobber the cleanup slot.
2063+
// because most other cleanups can't push more cleanups, and thus, emitting
2064+
// other cleanups generally cannot clobber the cleanup slot.
20652065
//
2066-
// To prevent this from happening, simply force the allocation of a new
2067-
// cleanup slot for the body of the defer statement by temporarily clearing
2068-
// out any existing slot.
2069-
SaveAndRestore PreserveCleanupSlot{CGF.NormalCleanupDest,
2070-
RawAddress::invalid()};
2066+
// To prevent this from happening, save the current cleanup slot value and
2067+
// restore it after emitting the '_Defer' statement.
2068+
llvm::Value *SavedCleanupDest = nullptr;
2069+
if (CGF.NormalCleanupDest.isValid())
2070+
SavedCleanupDest =
2071+
CGF.Builder.CreateLoad(CGF.NormalCleanupDest, "cleanup.dest.saved");
2072+
20712073
CGF.EmitStmt(Stmt.getBody());
2074+
2075+
if (SavedCleanupDest && CGF.HaveInsertPoint())
2076+
CGF.Builder.CreateStore(SavedCleanupDest, CGF.NormalCleanupDest);
2077+
2078+
// Cleanups must end with an insert point.
2079+
CGF.EnsureInsertPoint();
20722080
}
20732081
};
20742082
} // namespace

‎clang/test/CodeGen/defer-ts-nested-cleanups.c‎

Lines changed: 43 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,6 @@
22

33
// Test that cleanups emitted in a '_Defer' don't clobber the cleanup slot; we
44
// test this using lifetime intrinsics, which are emitted starting at -O1.
5-
//
6-
// Note that the IR below contains fewer cleanup slots than one might intuitively
7-
// expect because some of them are optimised out (we just emit a direct branch
8-
// instead if the cleanup slot would only be written to once); the important part
9-
// is that we don't clobber a cleanup slot while executing its cleanup.
105

116
void g();
127

@@ -29,6 +24,7 @@ void g();
2924
// CHECK-NEXT: store i32 0, ptr %cleanup.dest.slot, align 4
3025
// CHECK-NEXT: br label %cleanup
3126
// CHECK: cleanup:
27+
// CHECK-NEXT: %cleanup.dest.saved = load i32, ptr %cleanup.dest.slot, align 4
3228
// CHECK-NEXT: call void @llvm.lifetime.start.p0(ptr %j)
3329
// CHECK-NEXT: store i32 0, ptr %j, align 4
3430
// CHECK-NEXT: br label %for.cond1
@@ -37,6 +33,7 @@ void g();
3733
// CHECK-NEXT: %cmp2 = icmp ne i32 %1, 1
3834
// CHECK-NEXT: br i1 %cmp2, label %for.body, label %for.cond.cleanup
3935
// CHECK: for.cond.cleanup:
36+
// CHECK-NEXT: store i32 5, ptr %cleanup.dest.slot, align 4
4037
// CHECK-NEXT: call void @llvm.lifetime.end.p0(ptr %j)
4138
// CHECK-NEXT: br label %for.end
4239
// CHECK: for.body:
@@ -48,21 +45,22 @@ void g();
4845
// CHECK-NEXT: store i32 %inc, ptr %j, align 4
4946
// CHECK-NEXT: br label %for.cond1
5047
// CHECK: for.end:
48+
// CHECK-NEXT: store i32 %cleanup.dest.saved, ptr %cleanup.dest.slot, align 4
5149
// CHECK-NEXT: %cleanup.dest = load i32, ptr %cleanup.dest.slot, align 4
52-
// CHECK-NEXT: switch i32 %cleanup.dest, label %cleanup7 [
50+
// CHECK-NEXT: switch i32 %cleanup.dest, label %cleanup6 [
5351
// CHECK-NEXT: i32 0, label %cleanup.cont
5452
// CHECK-NEXT: ]
5553
// CHECK: cleanup.cont:
56-
// CHECK-NEXT: br label %for.inc5
57-
// CHECK: for.inc5:
54+
// CHECK-NEXT: br label %for.inc4
55+
// CHECK: for.inc4:
5856
// CHECK-NEXT: %3 = load i32, ptr %i, align 4
59-
// CHECK-NEXT: %inc6 = add nsw i32 %3, 1
60-
// CHECK-NEXT: store i32 %inc6, ptr %i, align 4
57+
// CHECK-NEXT: %inc5 = add nsw i32 %3, 1
58+
// CHECK-NEXT: store i32 %inc5, ptr %i, align 4
6159
// CHECK-NEXT: br label %for.cond
62-
// CHECK: cleanup7:
60+
// CHECK: cleanup6:
6361
// CHECK-NEXT: call void @llvm.lifetime.end.p0(ptr %i)
64-
// CHECK-NEXT: br label %for.end8
65-
// CHECK: for.end8:
62+
// CHECK-NEXT: br label %for.end7
63+
// CHECK: for.end7:
6664
// CHECK-NEXT: ret void
6765
void f1() {
6866
for (int i = 0;; i++) {
@@ -80,7 +78,6 @@ void f1() {
8078
// CHECK-NEXT: %i = alloca i32, align 4
8179
// CHECK-NEXT: %cleanup.dest.slot = alloca i32, align 4
8280
// CHECK-NEXT: %j = alloca i32, align 4
83-
// CHECK-NEXT: %cleanup.dest.slot4 = alloca i32, align 4
8481
// CHECK-NEXT: %k = alloca i32, align 4
8582
// CHECK-NEXT: call void @llvm.lifetime.start.p0(ptr %i)
8683
// CHECK-NEXT: store i32 0, ptr %i, align 4
@@ -96,20 +93,22 @@ void f1() {
9693
// CHECK-NEXT: store i32 0, ptr %cleanup.dest.slot, align 4
9794
// CHECK-NEXT: br label %cleanup
9895
// CHECK: cleanup:
96+
// CHECK-NEXT: %cleanup.dest.saved = load i32, ptr %cleanup.dest.slot, align 4
9997
// CHECK-NEXT: call void @llvm.lifetime.start.p0(ptr %j)
10098
// CHECK-NEXT: store i32 0, ptr %j, align 4
10199
// CHECK-NEXT: br label %for.cond1
102100
// CHECK: for.cond1:
103101
// CHECK-NEXT: %1 = load i32, ptr %j, align 4
104102
// CHECK-NEXT: %cmp2 = icmp eq i32 %1, 1
105-
// CHECK-NEXT: br i1 %cmp2, label %if.then3, label %if.end5
103+
// CHECK-NEXT: br i1 %cmp2, label %if.then3, label %if.end4
106104
// CHECK: if.then3:
107-
// CHECK-NEXT: store i32 5, ptr %cleanup.dest.slot4, align 4
108-
// CHECK-NEXT: br label %cleanup6
109-
// CHECK: if.end5:
110-
// CHECK-NEXT: store i32 0, ptr %cleanup.dest.slot4, align 4
111-
// CHECK-NEXT: br label %cleanup6
112-
// CHECK: cleanup6:
105+
// CHECK-NEXT: store i32 5, ptr %cleanup.dest.slot, align 4
106+
// CHECK-NEXT: br label %cleanup5
107+
// CHECK: if.end4:
108+
// CHECK-NEXT: store i32 0, ptr %cleanup.dest.slot, align 4
109+
// CHECK-NEXT: br label %cleanup5
110+
// CHECK: cleanup5:
111+
// CHECK-NEXT: %cleanup.dest.saved6 = load i32, ptr %cleanup.dest.slot, align 4
113112
// CHECK-NEXT: call void @llvm.lifetime.start.p0(ptr %k)
114113
// CHECK-NEXT: store i32 0, ptr %k, align 4
115114
// CHECK-NEXT: br label %for.cond7
@@ -118,6 +117,7 @@ void f1() {
118117
// CHECK-NEXT: %cmp8 = icmp ne i32 %2, 1
119118
// CHECK-NEXT: br i1 %cmp8, label %for.body, label %for.cond.cleanup
120119
// CHECK: for.cond.cleanup:
120+
// CHECK-NEXT: store i32 8, ptr %cleanup.dest.slot, align 4
121121
// CHECK-NEXT: call void @llvm.lifetime.end.p0(ptr %k)
122122
// CHECK-NEXT: br label %for.end
123123
// CHECK: for.body:
@@ -129,36 +129,38 @@ void f1() {
129129
// CHECK-NEXT: store i32 %inc, ptr %k, align 4
130130
// CHECK-NEXT: br label %for.cond7
131131
// CHECK: for.end:
132-
// CHECK-NEXT: %cleanup.dest = load i32, ptr %cleanup.dest.slot4, align 4
133-
// CHECK-NEXT: switch i32 %cleanup.dest, label %cleanup13 [
132+
// CHECK-NEXT: store i32 %cleanup.dest.saved6, ptr %cleanup.dest.slot, align 4
133+
// CHECK-NEXT: %cleanup.dest = load i32, ptr %cleanup.dest.slot, align 4
134+
// CHECK-NEXT: switch i32 %cleanup.dest, label %cleanup12 [
134135
// CHECK-NEXT: i32 0, label %cleanup.cont
135136
// CHECK-NEXT: ]
136137
// CHECK: cleanup.cont:
137-
// CHECK-NEXT: br label %for.inc11
138-
// CHECK: for.inc11:
138+
// CHECK-NEXT: br label %for.inc10
139+
// CHECK: for.inc10:
139140
// CHECK-NEXT: %4 = load i32, ptr %j, align 4
140-
// CHECK-NEXT: %inc12 = add nsw i32 %4, 1
141-
// CHECK-NEXT: store i32 %inc12, ptr %j, align 4
141+
// CHECK-NEXT: %inc11 = add nsw i32 %4, 1
142+
// CHECK-NEXT: store i32 %inc11, ptr %j, align 4
142143
// CHECK-NEXT: br label %for.cond1
143-
// CHECK: cleanup13:
144+
// CHECK: cleanup12:
144145
// CHECK-NEXT: call void @llvm.lifetime.end.p0(ptr %j)
145-
// CHECK-NEXT: br label %for.end14
146-
// CHECK: for.end14:
147-
// CHECK-NEXT: %cleanup.dest15 = load i32, ptr %cleanup.dest.slot, align 4
148-
// CHECK-NEXT: switch i32 %cleanup.dest15, label %cleanup19 [
149-
// CHECK-NEXT: i32 0, label %cleanup.cont16
146+
// CHECK-NEXT: br label %for.end13
147+
// CHECK: for.end13:
148+
// CHECK-NEXT: store i32 %cleanup.dest.saved, ptr %cleanup.dest.slot, align 4
149+
// CHECK-NEXT: %cleanup.dest14 = load i32, ptr %cleanup.dest.slot, align 4
150+
// CHECK-NEXT: switch i32 %cleanup.dest14, label %cleanup18 [
151+
// CHECK-NEXT: i32 0, label %cleanup.cont15
150152
// CHECK-NEXT: ]
151-
// CHECK: cleanup.cont16:
152-
// CHECK-NEXT: br label %for.inc17
153-
// CHECK: for.inc17:
153+
// CHECK: cleanup.cont15:
154+
// CHECK-NEXT: br label %for.inc16
155+
// CHECK: for.inc16:
154156
// CHECK-NEXT: %5 = load i32, ptr %i, align 4
155-
// CHECK-NEXT: %inc18 = add nsw i32 %5, 1
156-
// CHECK-NEXT: store i32 %inc18, ptr %i, align 4
157+
// CHECK-NEXT: %inc17 = add nsw i32 %5, 1
158+
// CHECK-NEXT: store i32 %inc17, ptr %i, align 4
157159
// CHECK-NEXT: br label %for.cond
158-
// CHECK: cleanup19:
160+
// CHECK: cleanup18:
159161
// CHECK-NEXT: call void @llvm.lifetime.end.p0(ptr %i)
160-
// CHECK-NEXT: br label %for.end20
161-
// CHECK: for.end20:
162+
// CHECK-NEXT: br label %for.end19
163+
// CHECK: for.end19:
162164
// CHECK-NEXT: ret void
163165
void f2() {
164166
for (int i = 0;; i++) {

0 commit comments

Comments
 (0)