Skip to content

Commit

Permalink
[DebugInfo] Merge partially matching chains of inlined locations
Browse files Browse the repository at this point in the history
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
  • Loading branch information
dstenb committed Mar 6, 2023
1 parent 98c3dc3 commit 12a7aea
Show file tree
Hide file tree
Showing 7 changed files with 1,187 additions and 55 deletions.
160 changes: 110 additions & 50 deletions llvm/lib/IR/DebugInfoMetadata.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "llvm/IR/DebugInfoMetadata.h"
#include "LLVMContextImpl.h"
#include "MetadataImpl.h"
#include "llvm/ADT/SmallPtrSet.h"
#include "llvm/ADT/SmallSet.h"
#include "llvm/ADT/StringSwitch.h"
#include "llvm/BinaryFormat/Dwarf.h"
Expand Down Expand Up @@ -114,63 +115,122 @@ const DILocation *DILocation::getMergedLocation(const DILocation *LocA,
return LocA;

LLVMContext &C = LocA->getContext();
SmallDenseMap<std::pair<DILocalScope *, DILocation *>,
std::pair<unsigned, unsigned>, 4>
Locations;

DIScope *S = LocA->getScope();
DILocation *L = LocA->getInlinedAt();
unsigned Line = LocA->getLine();
unsigned Col = LocA->getColumn();

// Walk from the current source locaiton until the file scope;
// then, do the same for the inlined-at locations.
auto AdvanceToParentLoc = [&S, &L, &Line, &Col]() {
S = S->getScope();
if (!S && L) {
Line = L->getLine();
Col = L->getColumn();
S = L->getScope();
L = L->getInlinedAt();
}
};

while (S) {
if (auto *LS = dyn_cast<DILocalScope>(S))
Locations.try_emplace(std::make_pair(LS, L), std::make_pair(Line, Col));
AdvanceToParentLoc();
using LocVec = SmallVector<const DILocation *>;
LocVec ALocs;
LocVec BLocs;
SmallDenseMap<std::pair<const DISubprogram *, const DILocation *>, unsigned,
4>
ALookup;

// Walk through LocA and its inlined-at locations, populate them in ALocs and
// save the index for the subprogram and inlined-at pair, which we use to find
// a matching starting location in LocB's chain.
for (auto [L, I] = std::make_pair(LocA, 0U); L; L = L->getInlinedAt(), I++) {
ALocs.push_back(L);
auto Res = ALookup.try_emplace(
{L->getScope()->getSubprogram(), L->getInlinedAt()}, I);
assert(Res.second && "Multiple <SP, InlinedAt> pairs in a location chain?");
(void)Res;
}

// Walk the source locations of LocB until a match with LocA is found.
S = LocB->getScope();
L = LocB->getInlinedAt();
Line = LocB->getLine();
Col = LocB->getColumn();
while (S) {
if (auto *LS = dyn_cast<DILocalScope>(S)) {
auto MatchLoc = Locations.find(std::make_pair(LS, L));
if (MatchLoc != Locations.end()) {
// If the lines match, keep the line, but set the column to '0'
// If the lines don't match, pick a "line 0" location but keep
// the current scope and inlined-at.
bool SameLine = Line == MatchLoc->second.first;
bool SameCol = Col == MatchLoc->second.second;
Line = SameLine ? Line : 0;
Col = SameLine && SameCol ? Col : 0;
break;
}
}
AdvanceToParentLoc();
LocVec::reverse_iterator ARIt = ALocs.rend();
LocVec::reverse_iterator BRIt = BLocs.rend();

// Populate BLocs and look for a matching starting location, the first
// location with the same subprogram and inlined-at location as in LocA's
// chain. Since the two locations have the same inlined-at location we do
// not need to look at those parts of the chains.
for (auto [L, I] = std::make_pair(LocB, 0U); L; L = L->getInlinedAt(), I++) {
BLocs.push_back(L);

if (ARIt != ALocs.rend())
// We have already found a matching starting location.
continue;

auto IT = ALookup.find({L->getScope()->getSubprogram(), L->getInlinedAt()});
if (IT == ALookup.end())
continue;

// The + 1 is to account for the &*rev_it = &(it - 1) relationship.
ARIt = LocVec::reverse_iterator(ALocs.begin() + IT->second + 1);
BRIt = LocVec::reverse_iterator(BLocs.begin() + I + 1);

// If we have found a matching starting location we do not need to add more
// locations to BLocs, since we will only look at location pairs preceding
// the matching starting location, and adding more elements to BLocs could
// invalidate the iterator that we initialized here.
break;
}

if (!S) {
// If the two locations are irreconsilable, pick any scope,
// and return a "line 0" location.
Line = Col = 0;
S = LocA->getScope();
// Merge the two locations if possible, using the supplied
// inlined-at location for the created location.
auto MergeLocPair = [&C](const DILocation *L1, const DILocation *L2,
DILocation *InlinedAt) -> DILocation * {
if (L1 == L2)
return DILocation::get(C, L1->getLine(), L1->getColumn(), L1->getScope(),
InlinedAt);

// If the locations originate from different subprograms we can't produce
// a common location.
if (L1->getScope()->getSubprogram() != L2->getScope()->getSubprogram())
return nullptr;

// Return the nearest common scope inside a subprogram.
auto GetNearestCommonScope = [](DIScope *S1, DIScope *S2) -> DIScope * {
SmallPtrSet<DIScope *, 8> Scopes;
for (; S1; S1 = S1->getScope()) {
Scopes.insert(S1);
if (isa<DISubprogram>(S1))
break;
}

for (; S2; S2 = S2->getScope()) {
if (Scopes.count(S2))
return S2;
if (isa<DISubprogram>(S2))
break;
}

return nullptr;
};

auto Scope = GetNearestCommonScope(L1->getScope(), L2->getScope());
assert(Scope && "No common scope in the same subprogram?");

bool SameLine = L1->getLine() == L2->getLine();
bool SameCol = L1->getColumn() == L2->getColumn();
unsigned Line = SameLine ? L1->getLine() : 0;
unsigned Col = SameLine && SameCol ? L1->getColumn() : 0;

return DILocation::get(C, Line, Col, Scope, InlinedAt);
};

DILocation *Result = ARIt != ALocs.rend() ? (*ARIt)->getInlinedAt() : nullptr;

// If we have found a common starting location, walk up the inlined-at chains
// and try to produce common locations.
for (; ARIt != ALocs.rend() && BRIt != BLocs.rend(); ++ARIt, ++BRIt) {
DILocation *Tmp = MergeLocPair(*ARIt, *BRIt, Result);

if (!Tmp)
// We have walked up to a point in the chains where the two locations
// are irreconsilable. At this point Result contains the nearest common
// location in the inlined-at chains of LocA and LocB, so we break here.
break;

Result = Tmp;
}

return DILocation::get(C, Line, Col, S, L);
if (Result)
return Result;

// We ended up with LocA and LocB as irreconsilable locations. Produce a
// location at 0:0 with one of the locations' scope. The function has
// historically picked A's scope, and a nullptr inlined-at location, so that
// behavior is mimicked here but I am not sure if this is always the correct
// way to handle this.
return DILocation::get(C, 0, 0, LocA->getScope(), nullptr);
}

std::optional<unsigned>
Expand Down
Loading

0 comments on commit 12a7aea

Please sign in to comment.