Skip to content

Commit 12a7aea

Browse files
committed
[DebugInfo] Merge partially matching chains of inlined locations
For example, if you have a chain of inlined funtions like this: 1 #include <stdlib.h> 2 int g1 = 4, g2 = 6; 3 4 static inline void bar(int q) { 5 if (q > 5) 6 abort(); 7 } 8 9 static inline void foo(int q) { 10 bar(q); 11 } 12 13 int main() { 14 foo(g1); 15 foo(g2); 16 return 0; 17 } with optimizations you could end up with a single abort call for the two inlined instances of foo(). When merging the locations for those inlined instances you would previously end up with a 0:0 location in main(). Leaving out that inlined chain from the location for the abort call could make troubleshooting difficult in some cases. This patch changes DILocation::getMergedLocation() to try to handle such cases. The function is rewritten to first find a common starting point for the two locations (same subprogram and inlined-at location), and then in reverse traverses the inlined-at chain looking for matches in each subprogram. For each subprogram, the merge function will find the nearest common scope for the two locations, and matching line and column (or set them to 0 if not matching). In the example above, you will for the abort call get a location in bar() at 6:5, inlined in foo() at 10:3, inlined in main() at 0:0 (since the two inlined functions are on different lines, but in the same scope). I have not seen anything in the DWARF standard that would disallow inlining a non-zero location at 0:0 in the inlined-at function, and both LLDB and GDB seem to accept these locations (with D142552 needed for LLDB to handle cases where the file, line and column number are all 0). One incompatibility with GDB is that it seems to ignore 0-line locations in some cases, but I am not aware of any specific issue that this patch produces related to that. With x86-64 LLDB (trunk) you previously got: frame #0: 0x00007ffff7a44930 libc.so.6`abort frame #1: 0x00005555555546ec a.out`main at merge.c:0 and will now get: frame #0: 0x[...] libc.so.6`abort frame #1: 0x[...] a.out`main [inlined] bar(q=<unavailable>) at merge.c:6:5 frame #2: 0x[...] a.out`main [inlined] foo(q=<unavailable>) at merge.c:10:3 frame #3: 0x[...] a.out`main at merge.c:0 and with x86-64 GDB (11.1) you will get: (gdb) bt #0 0x00007ffff7a44930 in abort () from /lib64/libc.so.6 #1 0x00005555555546ec in bar (q=<optimized out>) at merge.c:6 #2 foo (q=<optimized out>) at merge.c:10 #3 0x00005555555546ec in main () Reviewed By: aprantl, dblaikie Differential Revision: https://reviews.llvm.org/D142556
1 parent 98c3dc3 commit 12a7aea

File tree

7 files changed

+1187
-55
lines changed

7 files changed

+1187
-55
lines changed

llvm/lib/IR/DebugInfoMetadata.cpp

Lines changed: 110 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include "llvm/IR/DebugInfoMetadata.h"
1414
#include "LLVMContextImpl.h"
1515
#include "MetadataImpl.h"
16+
#include "llvm/ADT/SmallPtrSet.h"
1617
#include "llvm/ADT/SmallSet.h"
1718
#include "llvm/ADT/StringSwitch.h"
1819
#include "llvm/BinaryFormat/Dwarf.h"
@@ -114,63 +115,122 @@ const DILocation *DILocation::getMergedLocation(const DILocation *LocA,
114115
return LocA;
115116

116117
LLVMContext &C = LocA->getContext();
117-
SmallDenseMap<std::pair<DILocalScope *, DILocation *>,
118-
std::pair<unsigned, unsigned>, 4>
119-
Locations;
120-
121-
DIScope *S = LocA->getScope();
122-
DILocation *L = LocA->getInlinedAt();
123-
unsigned Line = LocA->getLine();
124-
unsigned Col = LocA->getColumn();
125-
126-
// Walk from the current source locaiton until the file scope;
127-
// then, do the same for the inlined-at locations.
128-
auto AdvanceToParentLoc = [&S, &L, &Line, &Col]() {
129-
S = S->getScope();
130-
if (!S && L) {
131-
Line = L->getLine();
132-
Col = L->getColumn();
133-
S = L->getScope();
134-
L = L->getInlinedAt();
135-
}
136-
};
137118

138-
while (S) {
139-
if (auto *LS = dyn_cast<DILocalScope>(S))
140-
Locations.try_emplace(std::make_pair(LS, L), std::make_pair(Line, Col));
141-
AdvanceToParentLoc();
119+
using LocVec = SmallVector<const DILocation *>;
120+
LocVec ALocs;
121+
LocVec BLocs;
122+
SmallDenseMap<std::pair<const DISubprogram *, const DILocation *>, unsigned,
123+
4>
124+
ALookup;
125+
126+
// Walk through LocA and its inlined-at locations, populate them in ALocs and
127+
// save the index for the subprogram and inlined-at pair, which we use to find
128+
// a matching starting location in LocB's chain.
129+
for (auto [L, I] = std::make_pair(LocA, 0U); L; L = L->getInlinedAt(), I++) {
130+
ALocs.push_back(L);
131+
auto Res = ALookup.try_emplace(
132+
{L->getScope()->getSubprogram(), L->getInlinedAt()}, I);
133+
assert(Res.second && "Multiple <SP, InlinedAt> pairs in a location chain?");
134+
(void)Res;
142135
}
143136

144-
// Walk the source locations of LocB until a match with LocA is found.
145-
S = LocB->getScope();
146-
L = LocB->getInlinedAt();
147-
Line = LocB->getLine();
148-
Col = LocB->getColumn();
149-
while (S) {
150-
if (auto *LS = dyn_cast<DILocalScope>(S)) {
151-
auto MatchLoc = Locations.find(std::make_pair(LS, L));
152-
if (MatchLoc != Locations.end()) {
153-
// If the lines match, keep the line, but set the column to '0'
154-
// If the lines don't match, pick a "line 0" location but keep
155-
// the current scope and inlined-at.
156-
bool SameLine = Line == MatchLoc->second.first;
157-
bool SameCol = Col == MatchLoc->second.second;
158-
Line = SameLine ? Line : 0;
159-
Col = SameLine && SameCol ? Col : 0;
160-
break;
161-
}
162-
}
163-
AdvanceToParentLoc();
137+
LocVec::reverse_iterator ARIt = ALocs.rend();
138+
LocVec::reverse_iterator BRIt = BLocs.rend();
139+
140+
// Populate BLocs and look for a matching starting location, the first
141+
// location with the same subprogram and inlined-at location as in LocA's
142+
// chain. Since the two locations have the same inlined-at location we do
143+
// not need to look at those parts of the chains.
144+
for (auto [L, I] = std::make_pair(LocB, 0U); L; L = L->getInlinedAt(), I++) {
145+
BLocs.push_back(L);
146+
147+
if (ARIt != ALocs.rend())
148+
// We have already found a matching starting location.
149+
continue;
150+
151+
auto IT = ALookup.find({L->getScope()->getSubprogram(), L->getInlinedAt()});
152+
if (IT == ALookup.end())
153+
continue;
154+
155+
// The + 1 is to account for the &*rev_it = &(it - 1) relationship.
156+
ARIt = LocVec::reverse_iterator(ALocs.begin() + IT->second + 1);
157+
BRIt = LocVec::reverse_iterator(BLocs.begin() + I + 1);
158+
159+
// If we have found a matching starting location we do not need to add more
160+
// locations to BLocs, since we will only look at location pairs preceding
161+
// the matching starting location, and adding more elements to BLocs could
162+
// invalidate the iterator that we initialized here.
163+
break;
164164
}
165165

166-
if (!S) {
167-
// If the two locations are irreconsilable, pick any scope,
168-
// and return a "line 0" location.
169-
Line = Col = 0;
170-
S = LocA->getScope();
166+
// Merge the two locations if possible, using the supplied
167+
// inlined-at location for the created location.
168+
auto MergeLocPair = [&C](const DILocation *L1, const DILocation *L2,
169+
DILocation *InlinedAt) -> DILocation * {
170+
if (L1 == L2)
171+
return DILocation::get(C, L1->getLine(), L1->getColumn(), L1->getScope(),
172+
InlinedAt);
173+
174+
// If the locations originate from different subprograms we can't produce
175+
// a common location.
176+
if (L1->getScope()->getSubprogram() != L2->getScope()->getSubprogram())
177+
return nullptr;
178+
179+
// Return the nearest common scope inside a subprogram.
180+
auto GetNearestCommonScope = [](DIScope *S1, DIScope *S2) -> DIScope * {
181+
SmallPtrSet<DIScope *, 8> Scopes;
182+
for (; S1; S1 = S1->getScope()) {
183+
Scopes.insert(S1);
184+
if (isa<DISubprogram>(S1))
185+
break;
186+
}
187+
188+
for (; S2; S2 = S2->getScope()) {
189+
if (Scopes.count(S2))
190+
return S2;
191+
if (isa<DISubprogram>(S2))
192+
break;
193+
}
194+
195+
return nullptr;
196+
};
197+
198+
auto Scope = GetNearestCommonScope(L1->getScope(), L2->getScope());
199+
assert(Scope && "No common scope in the same subprogram?");
200+
201+
bool SameLine = L1->getLine() == L2->getLine();
202+
bool SameCol = L1->getColumn() == L2->getColumn();
203+
unsigned Line = SameLine ? L1->getLine() : 0;
204+
unsigned Col = SameLine && SameCol ? L1->getColumn() : 0;
205+
206+
return DILocation::get(C, Line, Col, Scope, InlinedAt);
207+
};
208+
209+
DILocation *Result = ARIt != ALocs.rend() ? (*ARIt)->getInlinedAt() : nullptr;
210+
211+
// If we have found a common starting location, walk up the inlined-at chains
212+
// and try to produce common locations.
213+
for (; ARIt != ALocs.rend() && BRIt != BLocs.rend(); ++ARIt, ++BRIt) {
214+
DILocation *Tmp = MergeLocPair(*ARIt, *BRIt, Result);
215+
216+
if (!Tmp)
217+
// We have walked up to a point in the chains where the two locations
218+
// are irreconsilable. At this point Result contains the nearest common
219+
// location in the inlined-at chains of LocA and LocB, so we break here.
220+
break;
221+
222+
Result = Tmp;
171223
}
172224

173-
return DILocation::get(C, Line, Col, S, L);
225+
if (Result)
226+
return Result;
227+
228+
// We ended up with LocA and LocB as irreconsilable locations. Produce a
229+
// location at 0:0 with one of the locations' scope. The function has
230+
// historically picked A's scope, and a nullptr inlined-at location, so that
231+
// behavior is mimicked here but I am not sure if this is always the correct
232+
// way to handle this.
233+
return DILocation::get(C, 0, 0, LocA->getScope(), nullptr);
174234
}
175235

176236
std::optional<unsigned>

0 commit comments

Comments
 (0)