Skip to content

Commit 5372ce8

Browse files
gottesmmkavon
authored andcommitted
[move-only] Fix a thinko in FieldSensitivePrunedLiveBlocks::updateForUse.
I think this was a mistake from when I changed implementations to use pure scalar bit processing rather than processing all at once. Instead of just updating the internal found resulting uses array, we were appending to it. Some notes: 1. This actually did not break anything semantically in the move checker since we do not use this array in the caller in anyway. We just use it internally in the routine to first lookup the current state which we then process in the routine. That being said, this API is written such that a user /could/ do that and we want to allow for users to be able to do that so that we match what PrunedLiveness does. 2. This could cause memory corruption due to iterator invalidation if by appending we caused the SmallVector to reallocate as we iterated over the array. So to fix this I did the following: a. I changed the push_back to be an assignment. b. I removed llvm::enumerate just out of paranoia if the assignment could potentially cause iterator invalidation. The given test exercises this code path and with the old behavior would crash with asan or guard malloc. rdar://109673338 (cherry picked from commit 5bef851)
1 parent 66b6e39 commit 5372ce8

File tree

2 files changed

+168
-4
lines changed

2 files changed

+168
-4
lines changed

lib/SIL/Utils/FieldSensitivePrunedLiveness.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -507,19 +507,19 @@ void FieldSensitivePrunedLiveBlocks::updateForUse(
507507

508508
auto *bb = user->getParent();
509509
getBlockLiveness(bb, startBitNo, endBitNo, resultingLivenessInfo);
510+
assert(resultingLivenessInfo.size() == (endBitNo - startBitNo));
510511

511-
for (auto pair : llvm::enumerate(resultingLivenessInfo)) {
512-
unsigned index = pair.index();
512+
for (unsigned index : indices(resultingLivenessInfo)) {
513513
unsigned specificBitNo = startBitNo + index;
514-
switch (pair.value()) {
514+
switch (resultingLivenessInfo[index]) {
515515
case LiveOut:
516516
case LiveWithin:
517517
continue;
518518
case Dead: {
519519
// This use block has not yet been marked live. Mark it and its
520520
// predecessor blocks live.
521521
computeScalarUseBlockLiveness(bb, specificBitNo);
522-
resultingLivenessInfo.push_back(getBlockLiveness(bb, specificBitNo));
522+
resultingLivenessInfo[index] = getBlockLiveness(bb, specificBitNo);
523523
continue;
524524
}
525525
}
@@ -537,6 +537,7 @@ FieldSensitivePrunedLiveBlocks::getStringRef(IsLive isLive) const {
537537
case LiveOut:
538538
return "LiveOut";
539539
}
540+
llvm_unreachable("Covered switch?!");
540541
}
541542

542543
void FieldSensitivePrunedLiveBlocks::print(llvm::raw_ostream &OS) const {
Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
// RUN: %target-swift-emit-sil -sil-verify-all -verify %s
2+
3+
// Previously there was a latent albeit harmless bug in
4+
// FieldSensitivePrunedLiveness around an array inside of it re-allocating. This
5+
// test exercises that behavior by creating a type that is so large that it
6+
// cannot be stored within anything but a very large small vector (> 128
7+
// elements).
8+
9+
public struct Large : ~Copyable {
10+
var a0 = 0
11+
var a1 = 0
12+
var a2 = 0
13+
var a3 = 0
14+
var a4 = 0
15+
var a5 = 0
16+
var a6 = 0
17+
var a7 = 0
18+
var a8 = 0
19+
var a9 = 0
20+
var a10 = 0
21+
var a11 = 0
22+
var a12 = 0
23+
var a13 = 0
24+
var a14 = 0
25+
var a15 = 0
26+
var a16 = 0
27+
var a17 = 0
28+
var a18 = 0
29+
var a19 = 0
30+
var a20 = 0
31+
var a21 = 0
32+
var a22 = 0
33+
var a23 = 0
34+
var a24 = 0
35+
var a25 = 0
36+
var a26 = 0
37+
var a27 = 0
38+
var a28 = 0
39+
var a29 = 0
40+
var a30 = 0
41+
var a31 = 0
42+
var a32 = 0
43+
var a33 = 0
44+
var a34 = 0
45+
var a35 = 0
46+
var a36 = 0
47+
var a37 = 0
48+
var a38 = 0
49+
var a39 = 0
50+
var a40 = 0
51+
var a41 = 0
52+
var a42 = 0
53+
var a43 = 0
54+
var a44 = 0
55+
var a45 = 0
56+
var a46 = 0
57+
var a47 = 0
58+
var a48 = 0
59+
var a49 = 0
60+
var a50 = 0
61+
var a51 = 0
62+
var a52 = 0
63+
var a53 = 0
64+
var a54 = 0
65+
var a55 = 0
66+
var a56 = 0
67+
var a57 = 0
68+
var a58 = 0
69+
var a59 = 0
70+
var a60 = 0
71+
var a61 = 0
72+
var a62 = 0
73+
var a63 = 0
74+
var a64 = 0
75+
var a65 = 0
76+
var a66 = 0
77+
var a67 = 0
78+
var a68 = 0
79+
var a69 = 0
80+
var a70 = 0
81+
var a71 = 0
82+
var a72 = 0
83+
var a73 = 0
84+
var a74 = 0
85+
var a75 = 0
86+
var a76 = 0
87+
var a77 = 0
88+
var a78 = 0
89+
var a79 = 0
90+
var a80 = 0
91+
var a81 = 0
92+
var a82 = 0
93+
var a83 = 0
94+
var a84 = 0
95+
var a85 = 0
96+
var a86 = 0
97+
var a87 = 0
98+
var a88 = 0
99+
var a89 = 0
100+
var a90 = 0
101+
var a91 = 0
102+
var a92 = 0
103+
var a93 = 0
104+
var a94 = 0
105+
var a95 = 0
106+
var a96 = 0
107+
var a97 = 0
108+
var a98 = 0
109+
var a99 = 0
110+
var a100 = 0
111+
var a101 = 0
112+
var a102 = 0
113+
var a103 = 0
114+
var a104 = 0
115+
var a105 = 0
116+
var a106 = 0
117+
var a107 = 0
118+
var a108 = 0
119+
var a109 = 0
120+
var a110 = 0
121+
var a111 = 0
122+
var a112 = 0
123+
var a113 = 0
124+
var a114 = 0
125+
var a115 = 0
126+
var a116 = 0
127+
var a117 = 0
128+
var a118 = 0
129+
var a119 = 0
130+
var a120 = 0
131+
var a121 = 0
132+
var a122 = 0
133+
var a123 = 0
134+
var a124 = 0
135+
var a125 = 0
136+
var a126 = 0
137+
var a127 = 0
138+
var a128 = 0
139+
var a129 = 0
140+
var a130 = 0
141+
}
142+
143+
func borrowVal(_ x: borrowing Large) {}
144+
145+
var bool: Bool { false }
146+
147+
func test() {
148+
let a = Large()
149+
150+
if bool {
151+
152+
} else {
153+
154+
}
155+
156+
if bool {
157+
158+
} else {
159+
160+
}
161+
162+
borrowVal(a)
163+
}

0 commit comments

Comments
 (0)