Skip to content

Commit c4ebb32

Browse files
aslasl
authored
Properly transfer location info from debug_value into alloc_stack: we need to drop op_deref expression (#42245)
In order to transfer debug info from `debug_value` to `alloc_stack` we need to drop `op_ref` debug expression, as the latter instruction represents the location and not the value itself. Also, fix the debug info verifier as variable type was deduced from `alloc_stack` / `alloc_box` improperly: pointer type of instruction itself was used instead of underlying object type. The issue is only exposed when both optimizations (SIL mem2reg at least) and debug info are enabled Resolves SR-15849
1 parent f045f14 commit c4ebb32

File tree

3 files changed

+154
-5
lines changed

3 files changed

+154
-5
lines changed

‎lib/SIL/Verifier/SILVerifier.cpp

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1315,9 +1315,9 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
13151315
case SILInstructionKind::DebugValueInst:
13161316
DebugVarTy = inst->getOperand(0)->getType();
13171317
if (DebugVarTy.isAddress()) {
1318-
auto Expr = varInfo->DIExpr.operands();
1319-
if (!Expr.empty() &&
1320-
Expr.begin()->getOperator() == SILDIExprOperator::Dereference)
1318+
// FIXME: op_deref could be applied to address types only.
1319+
// FIXME: Add this check
1320+
if (varInfo->DIExpr.startsWithDeref())
13211321
DebugVarTy = DebugVarTy.getObjectType();
13221322
}
13231323
break;
@@ -1343,9 +1343,16 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
13431343
require(DebugVars[argNum].first == varInfo->Name,
13441344
"Scope contains conflicting debug variables for one function "
13451345
"argument");
1346-
// Check for type
1347-
require(DebugVars[argNum].second == DebugVarTy,
1346+
// The source variable might change its location (e.g. due to
1347+
// optimizations). Check for most common transformations (e.g. loading
1348+
// to SSA value and vice versa) as well
1349+
require(DebugVars[argNum].second == DebugVarTy ||
1350+
(DebugVars[argNum].second.isAddress() &&
1351+
DebugVars[argNum].second.getObjectType() == DebugVarTy) ||
1352+
(DebugVarTy.isAddress() &&
1353+
DebugVars[argNum].second == DebugVarTy.getObjectType()),
13481354
"conflicting debug variable type!");
1355+
DebugVars[argNum].second = DebugVarTy;
13491356
} else {
13501357
// Reserve enough space.
13511358
while (DebugVars.size() <= argNum) {

‎lib/SILOptimizer/Differentiation/Common.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,11 @@ findDebugLocationAndVariable(SILValue originalValue) {
257257
for (auto *use : originalValue->getUses()) {
258258
if (auto *dvi = dyn_cast<DebugValueInst>(use->getUser()))
259259
return dvi->getVarInfo().map([&](SILDebugVariable var) {
260+
// We need to drop `op_deref` here as we're transferring debug info
261+
// location from debug_value instruction (which describes how to get value)
262+
// into alloc_stack (which describes the location)
263+
if (var.DIExpr.startsWithDeref())
264+
var.DIExpr.eraseElement(var.DIExpr.element_begin());
260265
return std::make_pair(dvi->getDebugLocation(), var);
261266
});
262267
}

‎test/AutoDiff/compiler_crashers_fixed/sr15849-invalid-debug-info.swift

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
// SR-15849: Mutating functions with control flow can cause assertion failure
2+
// for conflicting debug variable type.
3+
4+
// RUN: %target-swift-frontend -emit-ir -O -g %s | %FileCheck %s
5+
// CHECK-LABEL: define internal swiftcc float @"$s4main8TestTypeV24doDifferentiableTimeStep04timeG0ySf_tFTJpSSpSrTA"
6+
// CHECK: [[SELF:%.*]] = alloca %T4main8TestTypeV06ManualB7TangentV, align 4
7+
// CHECK: call void @llvm.dbg.declare(metadata %T4main8TestTypeV06ManualB7TangentV* [[SELF]]
8+
9+
import _Differentiation
10+
11+
public protocol TestInterface {
12+
mutating func doDifferentiableTimeStep(timeStep: Float)
13+
14+
var zeroTangentVectorInitializer: () -> TestInterfaceTangentVector { get }
15+
mutating func move(by offset: TestInterfaceTangentVector)
16+
}
17+
18+
public protocol TestInterfaceTangentVector {
19+
static var zero: Self { get }
20+
static func add(lhs: TestInterfaceTangentVector, rhs: TestInterfaceTangentVector) -> TestInterfaceTangentVector
21+
static func subtract(lhs: TestInterfaceTangentVector, rhs: TestInterfaceTangentVector) -> TestInterfaceTangentVector
22+
static func equals(lhs: TestInterfaceTangentVector, rhs: TestInterfaceTangentVector) -> Bool
23+
}
24+
25+
public extension TestInterface {
26+
var zeroTangentVector: TestInterfaceTangentVector { zeroTangentVectorInitializer() }
27+
}
28+
29+
private typealias InitFunction = @convention(c) () -> UnsafeMutableRawPointer
30+
31+
public protocol HasZeroTangentVectorDuplicate: Differentiable {
32+
var duplicateZeroTangentVectorInitializer: () -> TangentVector { get }
33+
}
34+
35+
public extension HasZeroTangentVectorDuplicate {
36+
var zeroTangentVector: TangentVector { duplicateZeroTangentVectorInitializer() }
37+
}
38+
39+
public extension HasZeroTangentVectorDuplicate {
40+
var duplicateZeroTangentVectorInitializer: () -> TangentVector {
41+
{ Self.TangentVector.zero }
42+
}
43+
}
44+
45+
struct TestType: TestInterface {
46+
struct TestState: Differentiable {
47+
public struct TangentVector: Differentiable, AdditiveArithmetic {
48+
public typealias TangentVector = TestState.TangentVector
49+
public var property0: Float.TangentVector
50+
public var time: Float.TangentVector
51+
public var property1: Float.TangentVector
52+
}
53+
54+
public mutating func move(by offset: TangentVector) {
55+
self.property0.move(by: offset.property0)
56+
self.time.move(by: offset.time)
57+
self.property1.move(by: offset.property1)
58+
}
59+
60+
@noDerivative
61+
var needUpdate: Bool
62+
@noDerivative
63+
var initialConditionsAreStale: Bool
64+
var property0: Float
65+
var time: Float
66+
var property1: Float
67+
68+
init() {
69+
self.needUpdate = true
70+
self.initialConditionsAreStale = true
71+
self.property0 = 0.01
72+
self.time = 0.01
73+
self.property1 = 0.01
74+
}
75+
}
76+
77+
var state = TestState()
78+
79+
@differentiable(reverse)
80+
mutating func doDifferentiableTimeStep(timeStep: Float) {
81+
if state.needUpdate {
82+
differentiableDoFlow()
83+
}
84+
if state.initialConditionsAreStale {
85+
doInit()
86+
}
87+
}
88+
89+
@differentiable(reverse)
90+
mutating func differentiableDoFlow() {
91+
state.property1 = 1.2
92+
state.property0 = 2.3
93+
state.needUpdate = false
94+
}
95+
mutating func doInit() {
96+
state.initialConditionsAreStale = false
97+
}
98+
99+
}
100+
101+
extension TestType: Differentiable {
102+
struct ManualTestTangent: Differentiable & AdditiveArithmetic {
103+
var state: TestState.TangentVector
104+
}
105+
typealias TangentVector = ManualTestTangent
106+
107+
mutating func move(by offset: ManualTestTangent) {
108+
self.state.move(by: offset.state)
109+
}
110+
}
111+
extension TestType: HasZeroTangentVectorDuplicate {}
112+
113+
114+
extension TestType {
115+
mutating func move(by offset: TestInterfaceTangentVector) {
116+
self.move(by: offset as! Self.TangentVector)
117+
}
118+
119+
var zeroTangentVectorInitializer: () -> TestInterfaceTangentVector {
120+
let initializer: () -> TangentVector = self.duplicateZeroTangentVectorInitializer
121+
return initializer
122+
}
123+
}
124+
125+
extension TestType.TangentVector: TestInterfaceTangentVector {
126+
static func add(lhs: TestInterfaceTangentVector, rhs: TestInterfaceTangentVector) -> TestInterfaceTangentVector {
127+
return (lhs as! Self) + (rhs as! Self)
128+
}
129+
130+
static func subtract(lhs: TestInterfaceTangentVector, rhs: TestInterfaceTangentVector) -> TestInterfaceTangentVector {
131+
return (lhs as! Self) - (rhs as! Self)
132+
}
133+
134+
static func equals(lhs: TestInterfaceTangentVector, rhs: TestInterfaceTangentVector) -> Bool {
135+
return (lhs as! Self) == (rhs as! Self)
136+
}
137+
}

0 commit comments

Comments
 (0)