Skip to content

Commit b4274c3

Browse files
dwblaikietru
authored andcommitted
[DebugInfo] When referencing structured bindings use the reference's location, not the binding's declaration's location (#153637)
For structured bindings that use custom `get` specializations, the resulting LLVM IR ascribes the load of the result of `get` to the binding's declaration, rather than the place where the binding is referenced - this caused awkward sequencing in the debug info where, when stepping through the code you'd step back to the binding declaration every time there was a reference to the binding. To fix that - when we cross into IRGening a binding - suppress the debug info location of that subexpression. I don't represent this as a great bit of API design - certainly open to ideas, but putting it out here as a place to start. It's /possible/ this is an incomplete fix, even - if the binding decl had other subexpressions, those would still get their location applied & it'd likely be wrong. So maybe that's a direction to go with to productionize this - add a new location scoped device that suppresses any overriding - this might be more robust. How do people feel about that? (cherry picked from commit 665e875)
1 parent 7a077a1 commit b4274c3

File tree

4 files changed

+69
-16
lines changed

4 files changed

+69
-16
lines changed

clang/lib/CodeGen/CGCall.cpp

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4787,19 +4787,6 @@ struct DestroyUnpassedArg final : EHScopeStack::Cleanup {
47874787
}
47884788
};
47894789

4790-
struct DisableDebugLocationUpdates {
4791-
CodeGenFunction &CGF;
4792-
bool disabledDebugInfo;
4793-
DisableDebugLocationUpdates(CodeGenFunction &CGF, const Expr *E) : CGF(CGF) {
4794-
if ((disabledDebugInfo = isa<CXXDefaultArgExpr>(E) && CGF.getDebugInfo()))
4795-
CGF.disableDebugInfo();
4796-
}
4797-
~DisableDebugLocationUpdates() {
4798-
if (disabledDebugInfo)
4799-
CGF.enableDebugInfo();
4800-
}
4801-
};
4802-
48034790
} // end anonymous namespace
48044791

48054792
RValue CallArg::getRValue(CodeGenFunction &CGF) const {
@@ -4836,7 +4823,9 @@ void CodeGenFunction::EmitWritebacks(const CallArgList &args) {
48364823

48374824
void CodeGenFunction::EmitCallArg(CallArgList &args, const Expr *E,
48384825
QualType type) {
4839-
DisableDebugLocationUpdates Dis(*this, E);
4826+
std::optional<DisableDebugLocationUpdates> Dis;
4827+
if (isa<CXXDefaultArgExpr>(E))
4828+
Dis.emplace(*this);
48404829
if (const ObjCIndirectCopyRestoreExpr *CRE =
48414830
dyn_cast<ObjCIndirectCopyRestoreExpr>(E)) {
48424831
assert(getLangOpts().ObjCAutoRefCount);
@@ -6229,3 +6218,12 @@ RValue CodeGenFunction::EmitVAArg(VAArgExpr *VE, Address &VAListAddr,
62296218
return CGM.getABIInfo().EmitMSVAArg(*this, VAListAddr, Ty, Slot);
62306219
return CGM.getABIInfo().EmitVAArg(*this, VAListAddr, Ty, Slot);
62316220
}
6221+
6222+
DisableDebugLocationUpdates::DisableDebugLocationUpdates(CodeGenFunction &CGF)
6223+
: CGF(CGF) {
6224+
CGF.disableDebugInfo();
6225+
}
6226+
6227+
DisableDebugLocationUpdates::~DisableDebugLocationUpdates() {
6228+
CGF.enableDebugInfo();
6229+
}

clang/lib/CodeGen/CGCall.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -457,6 +457,12 @@ inline FnInfoOpts &operator&=(FnInfoOpts &A, FnInfoOpts B) {
457457
return A;
458458
}
459459

460+
struct DisableDebugLocationUpdates {
461+
CodeGenFunction &CGF;
462+
DisableDebugLocationUpdates(CodeGenFunction &CGF);
463+
~DisableDebugLocationUpdates();
464+
};
465+
460466
} // end namespace CodeGen
461467
} // end namespace clang
462468

clang/lib/CodeGen/CGExpr.cpp

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3314,7 +3314,14 @@ LValue CodeGenFunction::EmitDeclRefLValue(const DeclRefExpr *E) {
33143314
auto *FD = LambdaCaptureFields.lookup(BD);
33153315
return EmitCapturedFieldLValue(*this, FD, CXXABIThisValue);
33163316
}
3317-
return EmitLValue(BD->getBinding());
3317+
// Suppress debug location updates when visiting the binding, since the
3318+
// binding may emit instructions that would otherwise be associated with the
3319+
// binding itself, rather than the expression referencing the binding. (this
3320+
// leads to jumpy debug stepping behavior where the location/debugger jump
3321+
// back to the binding declaration, then back to the expression referencing
3322+
// the binding)
3323+
DisableDebugLocationUpdates D(*this);
3324+
return EmitLValue(BD->getBinding(), NotKnownNonNull);
33183325
}
33193326

33203327
// We can form DeclRefExprs naming GUID declarations when reconstituting

clang/test/CodeGenCXX/debug-info-structured-binding.cpp

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,12 @@
77
// CHECK: #dbg_declare(ptr %{{[0-9]+}}, ![[VAR_4:[0-9]+]], !DIExpression(DW_OP_deref, DW_OP_plus_uconst, 4),
88
// CHECK: #dbg_declare(ptr %z1, ![[VAR_5:[0-9]+]], !DIExpression()
99
// CHECK: #dbg_declare(ptr %z2, ![[VAR_6:[0-9]+]], !DIExpression()
10+
// CHECK: getelementptr inbounds nuw %struct.A, ptr {{.*}}, i32 0, i32 1, !dbg ![[Y1_DEBUG_LOC:[0-9]+]]
11+
// CHECK: getelementptr inbounds nuw %struct.A, ptr {{.*}}, i32 0, i32 1, !dbg ![[Y2_DEBUG_LOC:[0-9]+]]
12+
// CHECK: load ptr, ptr %z2, {{.*}}!dbg ![[Z2_DEBUG_LOC:[0-9]+]]
13+
// CHECK: getelementptr inbounds [2 x i32], ptr {{.*}}, i64 0, i64 1, !dbg ![[A2_DEBUG_LOC:[0-9]+]]
14+
// CHECK: getelementptr inbounds nuw { i32, i32 }, ptr {{.*}}, i32 0, i32 1, !dbg ![[C2_DEBUG_LOC:[0-9]+]]
15+
// CHECK: extractelement <2 x i32> {{.*}}, i32 1, !dbg ![[V2_DEBUG_LOC:[0-9]+]]
1016
// CHECK: ![[VAR_0]] = !DILocalVariable(name: "a"
1117
// CHECK: ![[VAR_1]] = !DILocalVariable(name: "x1", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: !{{[0-9]+}})
1218
// CHECK: ![[VAR_2]] = !DILocalVariable(name: "y1", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: !{{[0-9]+}})
@@ -46,5 +52,41 @@ int f() {
4652
auto [x1, y1] = a;
4753
auto &[x2, y2] = a;
4854
auto [z1, z2] = B{1, 2};
49-
return x1 + y1 + x2 + y2 + z1 + z2;
55+
int array[2] = {3, 4};
56+
auto &[a1, a2] = array;
57+
_Complex int cmplx = {1, 2};
58+
auto &[c1, c2] = cmplx;
59+
int vctr __attribute__ ((vector_size (sizeof(int)*2)))= {1, 2};
60+
auto &[v1, v2] = vctr;
61+
return //
62+
x1 //
63+
+ //
64+
// CHECK: ![[Y1_DEBUG_LOC]] = !DILocation(line: [[@LINE+1]]
65+
y1 //
66+
+ //
67+
x2 //
68+
+ //
69+
// CHECK: ![[Y2_DEBUG_LOC]] = !DILocation(line: [[@LINE+1]]
70+
y2 //
71+
+ //
72+
z1 //
73+
+ //
74+
// CHECK: ![[Z2_DEBUG_LOC]] = !DILocation(line: [[@LINE+1]]
75+
z2 //
76+
+ //
77+
a1 //
78+
+ //
79+
// CHECK: ![[A2_DEBUG_LOC]] = !DILocation(line: [[@LINE+1]]
80+
a2 //
81+
+ //
82+
c1 //
83+
+ //
84+
// CHECK: ![[C2_DEBUG_LOC]] = !DILocation(line: [[@LINE+1]]
85+
c2 //
86+
+ //
87+
v1 //
88+
+ //
89+
// CHECK: ![[V2_DEBUG_LOC]] = !DILocation(line: [[@LINE+1]]
90+
v2 //
91+
;
5092
}

0 commit comments

Comments
 (0)