Skip to content

Commit bbd2ad4

Browse files
gottesmmgottesmm
committed
[move-only] Wire up proper debug info for addresses.
I included here SIL tests and in a separate PR against lldb included lldb tests that validate as we step that the values are validated/invalidated as appropriate. rdar://106767457
1 parent ee451f0 commit bbd2ad4

File tree

3 files changed

+284
-1
lines changed

3 files changed

+284
-1
lines changed

‎lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2030,6 +2030,9 @@ void MoveOnlyAddressCheckerPImpl::insertDestroysOnBoundary(
20302030
// If we're in no_consume_or_assign mode, we don't insert destroys, as we've
20312031
// already checked that there are no consumes. There can only be borrow uses,
20322032
// which means no destruction is needed at all.
2033+
//
2034+
// NOTE: This also implies that we do not need to insert invalidating
2035+
// debug_value undef since our value will not be invalidated.
20332036
if (markedValue->getCheckKind() ==
20342037
MarkMustCheckInst::CheckKind::NoConsumeOrAssign) {
20352038
LLVM_DEBUG(llvm::dbgs()
@@ -2040,6 +2043,30 @@ void MoveOnlyAddressCheckerPImpl::insertDestroysOnBoundary(
20402043

20412044
LLVM_DEBUG(llvm::dbgs() << " Visiting users!\n");
20422045

2046+
auto debugVar = DebugVarCarryingInst::getFromValue(
2047+
stripAccessMarkers(markedValue->getOperand()));
2048+
2049+
// Local helper that insert a debug_value undef to invalidate a noncopyable
2050+
// value that has been moved. Importantly, for LLVM to recognize that we are
2051+
// referring to the same debug variable as the original definition, we have to
2052+
// use the same debug scope and location as the original debug var.
2053+
auto insertUndefDebugValue = [&debugVar](SILInstruction *insertPt) {
2054+
if (!debugVar) {
2055+
return;
2056+
}
2057+
auto varInfo = debugVar.getVarInfo();
2058+
if (!varInfo) {
2059+
return;
2060+
}
2061+
SILBuilderWithScope debugInfoBuilder(insertPt);
2062+
debugInfoBuilder.setCurrentDebugScope(debugVar->getDebugScope());
2063+
debugInfoBuilder.createDebugValue(
2064+
debugVar->getLoc(),
2065+
SILUndef::get(debugVar.getOperandForDebugValueClone()->getType(),
2066+
insertPt->getModule()),
2067+
*varInfo, false, true);
2068+
};
2069+
20432070
for (auto &pair : boundary.getLastUsers()) {
20442071
auto *inst = pair.first;
20452072
auto &bv = pair.second;
@@ -2048,11 +2075,23 @@ void MoveOnlyAddressCheckerPImpl::insertDestroysOnBoundary(
20482075

20492076
auto interestingUse = liveness.isInterestingUser(inst);
20502077
switch (interestingUse.first) {
2051-
case IsInterestingUser::LifetimeEndingUse:
2078+
case IsInterestingUser::LifetimeEndingUse: {
20522079
LLVM_DEBUG(llvm::dbgs()
20532080
<< " Lifetime ending use! Recording final consume!\n");
2081+
// If we have a consuming use, when we stop at the consuming use we want
2082+
// the value to still be around. We only want the value to be invalidated
2083+
// once the consume operation has occured. Thus we always place the
2084+
// debug_value undef strictly after the consuming operation.
2085+
if (auto *ti = dyn_cast<TermInst>(inst)) {
2086+
for (auto *succBlock : ti->getSuccessorBlocks()) {
2087+
insertUndefDebugValue(&succBlock->front());
2088+
}
2089+
} else {
2090+
insertUndefDebugValue(inst->getNextInstruction());
2091+
}
20542092
consumes.recordFinalConsume(inst, *interestingUse.second);
20552093
continue;
2094+
}
20562095
case IsInterestingUser::NonLifetimeEndingUse:
20572096
case IsInterestingUser::NonUser:
20582097
LLVM_DEBUG(llvm::dbgs() << " NoneUser or NonLifetimeEndingUse! "
@@ -2066,13 +2105,19 @@ void MoveOnlyAddressCheckerPImpl::insertDestroysOnBoundary(
20662105
auto *insertPt = &*succBlock->begin();
20672106
insertDestroyBeforeInstruction(addressUseState, insertPt,
20682107
liveness.getRootValue(), bv, consumes);
2108+
// We insert the debug_value undef /after/ the last use since we want
2109+
// the value to be around when we stop at the last use instruction.
2110+
insertUndefDebugValue(insertPt);
20692111
}
20702112
continue;
20712113
}
20722114

20732115
auto *insertPt = inst->getNextInstruction();
20742116
insertDestroyBeforeInstruction(addressUseState, insertPt,
20752117
liveness.getRootValue(), bv, consumes);
2118+
// We insert the debug_value undef /after/ the last use since we want
2119+
// the value to be around when we stop at the last use instruction.
2120+
insertUndefDebugValue(insertPt);
20762121
continue;
20772122
}
20782123
}
@@ -2082,6 +2127,7 @@ void MoveOnlyAddressCheckerPImpl::insertDestroysOnBoundary(
20822127
insertDestroyBeforeInstruction(addressUseState, insertPt,
20832128
liveness.getRootValue(), pair.second,
20842129
consumes);
2130+
insertUndefDebugValue(insertPt);
20852131
LLVM_DEBUG(llvm::dbgs() << " Inserting destroy on edge bb"
20862132
<< pair.first->getDebugID() << "\n");
20872133
}
@@ -2095,13 +2141,15 @@ void MoveOnlyAddressCheckerPImpl::insertDestroysOnBoundary(
20952141
insertDestroyBeforeInstruction(addressUseState, insertPt,
20962142
liveness.getRootValue(), defPair.second,
20972143
consumes);
2144+
insertUndefDebugValue(insertPt);
20982145
} else {
20992146
auto *inst = cast<SILInstruction>(defPair.first);
21002147
auto *insertPt = inst->getNextInstruction();
21012148
assert(insertPt && "def instruction was a terminator");
21022149
insertDestroyBeforeInstruction(addressUseState, insertPt,
21032150
liveness.getRootValue(), defPair.second,
21042151
consumes);
2152+
insertUndefDebugValue(insertPt);
21052153
}
21062154
}
21072155

‎test/SILOptimizer/moveonly_addresschecker.sil

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ public struct NonTrivialStruct {
2828

2929
sil @useNonTrivialStruct : $@convention(thin) (@guaranteed NonTrivialStruct) -> ()
3030
sil @getNonTrivialStruct : $@convention(thin) () -> @owned NonTrivialStruct
31+
sil @consumeNonTrivialStructAddr : $@convention(thin) (@in NonTrivialStruct) -> ()
3132

3233
@_moveOnly
3334
public struct NonTrivialStructPair {
@@ -519,3 +520,18 @@ bb0:
519520
%8 = tuple ()
520521
return %8 : $()
521522
}
523+
524+
// Make sure we do not crash on this.
525+
sil [ossa] @test_in_use : $@convention(thin) () -> () {
526+
%9 = function_ref @getNonTrivialStruct : $@convention(thin) () -> @owned NonTrivialStruct
527+
%10 = apply %9() : $@convention(thin) () -> @owned NonTrivialStruct
528+
%0 = alloc_stack [moveable_value_debuginfo] $NonTrivialStruct
529+
%2 = mark_must_check [assignable_but_not_consumable] %0 : $*NonTrivialStruct
530+
store %10 to [init] %2 : $*NonTrivialStruct
531+
%f2 = function_ref @consumeNonTrivialStructAddr : $@convention(thin) (@in NonTrivialStruct) -> ()
532+
apply %f2(%2) : $@convention(thin) (@in NonTrivialStruct) -> ()
533+
destroy_addr %2 : $*NonTrivialStruct
534+
dealloc_stack %0 : $*NonTrivialStruct
535+
%9999 = tuple ()
536+
return %9999 : $()
537+
}

‎test/SILOptimizer/moveonly_addresschecker_debuginfo.sil

Lines changed: 219 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,219 @@
1+
// RUN: %target-sil-opt -module-name moveonly_addresschecker -emit-verbose-sil -sil-move-only-checker -enable-sil-verify-all %s | %FileCheck %s
2+
3+
sil_stage raw
4+
5+
public class CopyableKlass {}
6+
7+
@_moveOnly
8+
public struct NonTrivialStruct {
9+
var k: CopyableKlass
10+
}
11+
12+
sil @get_nontrivial_struct : $@convention(thin) () -> @owned NonTrivialStruct
13+
sil @use_nontrivial_struct : $@convention(thin) (@guaranteed NonTrivialStruct) -> ()
14+
sil @consume_nontrivial_struct : $@convention(thin) (@owned NonTrivialStruct) -> ()
15+
sil @consume_nontrivial_struct_addr : $@convention(thin) (@in NonTrivialStruct) -> ()
16+
17+
// CHECK-LABEL: sil [ossa] @non_lifetime_ending_use_test_inst : $@convention(thin) () -> () {
18+
// CHECK: [[STACK:%.*]] = alloc_stack $NonTrivialStruct, let, name "v" // {{.*}}; line:[[DEBUG_LOC:.*]]
19+
// CHECK: store {{%.*}} to [init] [[STACK]]
20+
// CHECK: [[BORROW:%.*]] = load_borrow [[STACK]]
21+
// CHECK: apply {{%.*}}([[BORROW]])
22+
// CHECK-NEXT: end_borrow [[BORROW]]
23+
// CHECK-NEXT: debug_value undef : $*NonTrivialStruct, let, name "v" // {{.*}}; line:[[DEBUG_LOC]]
24+
// CHECK: } // end sil function 'non_lifetime_ending_use_test_inst'
25+
sil [ossa] @non_lifetime_ending_use_test_inst : $@convention(thin) () -> () {
26+
%f = function_ref @get_nontrivial_struct : $@convention(thin) () -> @owned NonTrivialStruct
27+
%0 = apply %f() : $@convention(thin) () -> @owned NonTrivialStruct
28+
%1 = alloc_stack $NonTrivialStruct, let, name "v"
29+
%2 = mark_must_check [consumable_and_assignable] %1 : $*NonTrivialStruct
30+
store %0 to [init] %2 : $*NonTrivialStruct
31+
br bb1
32+
33+
bb1:
34+
%3 = load [copy] %2 : $*NonTrivialStruct
35+
%f2 = function_ref @use_nontrivial_struct : $@convention(thin) (@guaranteed NonTrivialStruct) -> ()
36+
apply %f2(%3) : $@convention(thin) (@guaranteed NonTrivialStruct) -> ()
37+
destroy_value %3 : $NonTrivialStruct
38+
destroy_addr %2 : $*NonTrivialStruct
39+
dealloc_stack %1 : $*NonTrivialStruct
40+
%9999 = tuple ()
41+
return %9999 : $()
42+
}
43+
44+
// CHECK-LABEL: sil [ossa] @non_lifetime_ending_use_test_boundary_edge : $@convention(thin) () -> () {
45+
// CHECK: [[STACK:%.*]] = alloc_stack $NonTrivialStruct, let, name "v" // {{.*}}; line:[[DEBUG_LOC:.*]]
46+
// CHECK: store {{%.*}} to [init] [[STACK]]
47+
// CHECK: cond_br undef, bb1, bb2
48+
49+
// CHECK: bb1:
50+
// CHECK: [[BORROW:%.*]] = load_borrow [[STACK]]
51+
// CHECK: apply {{%.*}}([[BORROW]])
52+
// CHECK-NEXT: end_borrow [[BORROW]]
53+
// CHECK-NEXT: destroy_addr [[STACK]]
54+
// CHECK-NEXT: debug_value undef : $*NonTrivialStruct, let, name "v" // {{.*}}; line:[[DEBUG_LOC]]
55+
// CHECK: br bb3
56+
//
57+
// CHECK: bb2:
58+
// CHECK-NEXT: destroy_addr [[STACK]]
59+
// CHECK-NEXT: debug_value undef : $*NonTrivialStruct, let, name "v" // {{.*}}; line:[[DEBUG_LOC]]
60+
// CHECK: } // end sil function 'non_lifetime_ending_use_test_boundary_edge'
61+
sil [ossa] @non_lifetime_ending_use_test_boundary_edge : $@convention(thin) () -> () {
62+
%f = function_ref @get_nontrivial_struct : $@convention(thin) () -> @owned NonTrivialStruct
63+
%0 = apply %f() : $@convention(thin) () -> @owned NonTrivialStruct
64+
%1 = alloc_stack $NonTrivialStruct, let, name "v"
65+
%2 = mark_must_check [consumable_and_assignable] %1 : $*NonTrivialStruct
66+
store %0 to [init] %2 : $*NonTrivialStruct
67+
cond_br undef, bb1, bb2
68+
69+
bb1:
70+
%3 = load [copy] %2 : $*NonTrivialStruct
71+
%f2 = function_ref @use_nontrivial_struct : $@convention(thin) (@guaranteed NonTrivialStruct) -> ()
72+
apply %f2(%3) : $@convention(thin) (@guaranteed NonTrivialStruct) -> ()
73+
destroy_value %3 : $NonTrivialStruct
74+
br bb3
75+
76+
bb2:
77+
br bb3
78+
79+
bb3:
80+
destroy_addr %2 : $*NonTrivialStruct
81+
dealloc_stack %1 : $*NonTrivialStruct
82+
%9999 = tuple ()
83+
return %9999 : $()
84+
}
85+
86+
// CHECK-LABEL: sil [ossa] @lifetime_ending_use : $@convention(thin) () -> () {
87+
// CHECK: [[STACK:%.*]] = alloc_stack $NonTrivialStruct, let, name "v" // {{.*}}; line:[[DEBUG_LOC:.*]]
88+
// CHECK: store {{%.*}} to [init] [[STACK]]
89+
// CHECK: cond_br undef, bb1, bb2
90+
91+
// CHECK: bb1:
92+
//
93+
// TODO: We should be smarter here and redefine the debug_value to be on the
94+
// take value and then when that value is destroyed, insert the actual
95+
// debug_value undef.
96+
//
97+
// CHECK: [[TAKE:%.*]] = load [take] [[STACK]]
98+
// CHECK-NEXT: debug_value undef : $*NonTrivialStruct, let, name "v" // {{.*}}; line:[[DEBUG_LOC]]
99+
// CHECK-NEXT: // function_ref
100+
// CHECK-NEXT: function_ref
101+
// CHECK-NEXT: apply {{%.*}}([[TAKE]])
102+
// CHECK: br bb3
103+
//
104+
// CHECK: bb2:
105+
// CHECK-NEXT: destroy_addr [[STACK]]
106+
// CHECK-NEXT: debug_value undef : $*NonTrivialStruct, let, name "v" // {{.*}}; line:[[DEBUG_LOC]]
107+
// CHECK: } // end sil function 'lifetime_ending_use'
108+
sil [ossa] @lifetime_ending_use : $@convention(thin) () -> () {
109+
%f = function_ref @get_nontrivial_struct : $@convention(thin) () -> @owned NonTrivialStruct
110+
%0 = apply %f() : $@convention(thin) () -> @owned NonTrivialStruct
111+
%1 = alloc_stack $NonTrivialStruct, let, name "v"
112+
%2 = mark_must_check [consumable_and_assignable] %1 : $*NonTrivialStruct
113+
store %0 to [init] %2 : $*NonTrivialStruct
114+
cond_br undef, bb1, bb2
115+
116+
bb1:
117+
%3 = load [copy] %2 : $*NonTrivialStruct
118+
%f2 = function_ref @consume_nontrivial_struct : $@convention(thin) (@owned NonTrivialStruct) -> ()
119+
apply %f2(%3) : $@convention(thin) (@owned NonTrivialStruct) -> ()
120+
br bb3
121+
122+
bb2:
123+
br bb3
124+
125+
bb3:
126+
destroy_addr %2 : $*NonTrivialStruct
127+
dealloc_stack %1 : $*NonTrivialStruct
128+
%9999 = tuple ()
129+
return %9999 : $()
130+
}
131+
132+
// CHECK-LABEL: sil [ossa] @lifetime_ending_use_addr : $@convention(thin) () -> () {
133+
// CHECK: [[STACK:%.*]] = alloc_stack $NonTrivialStruct, let, name "v" // {{.*}}; line:[[DEBUG_LOC:.*]]
134+
// CHECK: store {{%.*}} to [init] [[STACK]]
135+
// CHECK: cond_br undef, bb1, bb2
136+
137+
// CHECK: bb1:
138+
// CHECK: apply {{%.*}}([[STACK]])
139+
// CHECK-NEXT: debug_value undef : $*NonTrivialStruct, let, name "v" // {{.*}}; line:[[DEBUG_LOC]]
140+
// CHECK: br bb3
141+
//
142+
// CHECK: bb2:
143+
// CHECK-NEXT: destroy_addr [[STACK]]
144+
// CHECK-NEXT: debug_value undef : $*NonTrivialStruct, let, name "v" // {{.*}}; line:[[DEBUG_LOC]]
145+
// CHECK: } // end sil function 'lifetime_ending_use_addr'
146+
sil [ossa] @lifetime_ending_use_addr : $@convention(thin) () -> () {
147+
%f = function_ref @get_nontrivial_struct : $@convention(thin) () -> @owned NonTrivialStruct
148+
%0 = apply %f() : $@convention(thin) () -> @owned NonTrivialStruct
149+
%1 = alloc_stack $NonTrivialStruct, let, name "v"
150+
%2 = mark_must_check [consumable_and_assignable] %1 : $*NonTrivialStruct
151+
store %0 to [init] %2 : $*NonTrivialStruct
152+
cond_br undef, bb1, bb2
153+
154+
bb1:
155+
%f2 = function_ref @consume_nontrivial_struct_addr : $@convention(thin) (@in NonTrivialStruct) -> ()
156+
apply %f2(%2) : $@convention(thin) (@in NonTrivialStruct) -> ()
157+
br bb3
158+
159+
bb2:
160+
br bb3
161+
162+
bb3:
163+
destroy_addr %2 : $*NonTrivialStruct
164+
dealloc_stack %1 : $*NonTrivialStruct
165+
%9999 = tuple ()
166+
return %9999 : $()
167+
}
168+
169+
// CHECK-LABEL: sil [ossa] @dead_def_test_inst : $@convention(thin) () -> () {
170+
// CHECK: [[STACK:%.*]] = alloc_stack $NonTrivialStruct, let, name "v" // {{.*}}; line:[[DEBUG_LOC:.*]]
171+
// CHECK: store {{%.*}} to [init] [[STACK]]
172+
// CHECK: cond_br undef, bb1, bb3
173+
//
174+
// CHECK: bb1:
175+
// CHECK: apply {{%.*}}([[STACK]])
176+
// CHECK-NEXT: debug_value undef : $*NonTrivialStruct, let, name "v" // {{.*}}; line:[[DEBUG_LOC]]
177+
// CHECK-NEXT: br bb2
178+
//
179+
// CHECK: bb2:
180+
// CHECK: [[NEW_VAL:%.*]] = apply {{%.*}}() :
181+
// CHECK-NEXT: store [[NEW_VAL]] to [init] [[STACK]]
182+
// CHECK-NEXT: destroy_addr [[STACK]]
183+
// CHECK-NEXT: debug_value undef : $*NonTrivialStruct, let, name "v" // {{.*}}; line:[[DEBUG_LOC]]
184+
// CHECK-NEXT: br bb4
185+
//
186+
// CHECK: bb3:
187+
// CHECK-NEXT: destroy_addr [[STACK]]
188+
// CHECK-NEXT: debug_value undef : $*NonTrivialStruct, let, name "v" // {{.*}}; line:[[DEBUG_LOC]]
189+
// CHECK: br bb4
190+
// CHECK: } // end sil function 'dead_def_test_inst'
191+
sil [ossa] @dead_def_test_inst : $@convention(thin) () -> () {
192+
%f = function_ref @get_nontrivial_struct : $@convention(thin) () -> @owned NonTrivialStruct
193+
%0 = apply %f() : $@convention(thin) () -> @owned NonTrivialStruct
194+
%1 = alloc_stack $NonTrivialStruct, let, name "v"
195+
%2 = mark_must_check [consumable_and_assignable] %1 : $*NonTrivialStruct
196+
store %0 to [init] %2 : $*NonTrivialStruct
197+
cond_br undef, bb1, bb2
198+
199+
bb1:
200+
// TODO: Fix bug where we do not propagate this use to bb0 and thus think that
201+
// store %0 is a dead def.
202+
%f2 = function_ref @consume_nontrivial_struct_addr : $@convention(thin) (@in NonTrivialStruct) -> ()
203+
apply %f2(%2) : $@convention(thin) (@in NonTrivialStruct) -> ()
204+
br bb1a
205+
206+
bb1a:
207+
%0a = apply %f() : $@convention(thin) () -> @owned NonTrivialStruct
208+
store %0a to [init] %2 : $*NonTrivialStruct
209+
br bb3
210+
211+
bb2:
212+
br bb3
213+
214+
bb3:
215+
destroy_addr %2 : $*NonTrivialStruct
216+
dealloc_stack %1 : $*NonTrivialStruct
217+
%9999 = tuple ()
218+
return %9999 : $()
219+
}

0 commit comments

Comments
 (0)