<div dir="ltr">This change seems to fix the above example:<br><br><div><font face="monospace">diff --git lib/DebugInfo/DWARF/DWARFUnit.cpp lib/DebugInfo/DWARF/DWARFUnit.cpp</font></div><div><font face="monospace">index df55d7debf9..6c53c2a2b04 100644</font></div><div><font face="monospace">--- lib/DebugInfo/DWARF/DWARFUnit.cpp</font></div><div><font face="monospace">+++ lib/DebugInfo/DWARF/DWARFUnit.cpp</font></div><div><font face="monospace">@@ -615,7 +615,7 @@ void DWARFUnit::buildInlinedSubroutineDIEAddrMap(</font></div><div><font face="monospace">             PI == ParentIntervalsRange.end())</font></div><div><font face="monospace">           continue;</font></div><div><font face="monospace"> </font></div><div><font face="monospace">-        ParentIntervalIdx = PI - AddrMap.begin();</font></div><div><font face="monospace">+        ParentIntervalIdx = PI - AddrMap.begin() - 1;</font></div><div><font face="monospace">         int32_t &ParentIntervalDieIdx = std::prev(PI)->second;</font></div><div><font face="monospace">         uint32_t &ParentIntervalStart = std::prev(PI)->first;</font></div><div><font face="monospace">         const uint32_t ParentIntervalEnd = PI->first;</font><br><br>Though I don't fully understand the code well enough to have entire confidence in this change, though it makes some sense to me (perhaps not enough to write it down well - but I can gesticulate wildly in person to try to hand wave through my rough understanding).<br><br>But I also tried doing a bit more A/B testing against the symbolizer without this change & found another issue:<br><br><div><font face="monospace">void f1();</font></div><div><font face="monospace">inline __attribute__((always_inline)) void f2() { f1(); }</font></div><div><font face="monospace">inline __attribute__((always_inline)) void f3() { f1(); }</font></div><div><font face="monospace">inline __attribute__((always_inline)) void f4() {</font></div><div><font face="monospace">  f2();</font></div><div><font face="monospace">  f3();</font></div><div><font face="monospace">}</font></div><div><font face="monospace">void f5() {</font></div><div><font face="monospace">  f4();</font></div><div><font face="monospace">}</font><br><br>Then symbolize any address in the call to f3 (which, at least on my machine, is the range [0x9, 0xe)) - it misses the f3 portion of the stack, and only gives f4 and f5.<br><br>I /think/ I can explain this one a bit better, but again, maybe easier in person.</div></div></div><br><div class="gmail_quote"><div dir="ltr">On Tue, Feb 27, 2018 at 2:22 PM David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">OK, think I have an easily reproducible test case now:<br><br><div><font face="monospace">void f1();</font></div><div><font face="monospace">inline __attribute__((always_inline)) void f2() {</font></div><div><font face="monospace">  f1();</font></div><div><font face="monospace">  f1();</font></div><div><font face="monospace">}</font></div><div><font face="monospace">inline __attribute__((always_inline)) void f3() {</font></div><div><font face="monospace">  f2();</font></div><div><font face="monospace">  f1();</font></div><div><font face="monospace">  f1();</font></div><div><font face="monospace">}</font></div><div><font face="monospace">void f4() { f3(); }</font><br><br>Compile that to IR (letting the always-inliner inline things), then swap the middle two calls to <font face="monospace">f1()</font> (so it goes: call from inlined <font face="monospace">f2</font>, call from inlined <font face="monospace">f3</font>, call from inlined <font face="monospace">f2</font>, call from inlined <font face="monospace">f3</font>)<br><br>Then symbolizer the address of the 4th call to <font face="monospace">f1</font>. With this change, it isn't attributed to any inlining. Without this change it's correctly attributed to <font face="monospace">f3</font>.<br><br>I'll keep working on debugging this further - but there it is, again, in case anything pops out at you.<br><br>- Dave</div></div><div dir="ltr"><br><div class="gmail_quote"><div dir="ltr">On Tue, Feb 27, 2018 at 2:09 PM David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">I'm still debugging this, but wondering if this data set might spark some insight for you based on your implementation:<br><br>Here are the ranges of the inlined subroutines I'm dealing with:<br><br><div><font face="monospace">[3f0, f89)</font></div><div><font face="monospace">  [3f0, 64b)</font></div><div><font face="monospace">    [460, 5eb)</font></div><div><font face="monospace">      [460, 533)</font></div><div><font face="monospace">        [460, 533)</font></div><div><font face="monospace">          [4bf, 4c3)</font></div><div><font face="monospace">          [4e2, 4e7)</font></div><div><br></div><div>The address being symbolized is 4e7. It ends up being assigned no inlining at all - as though it was not nested within the first 5 inlined subroutines.<br><br>I've tested a variety of values, and it looks like only the [4e7, 533) range is not being symbolized correctly.<br><br>Does anything stand out to you - perhaps this gives you enough of a hint to sniff out what the bug is?<br><br>I'll continue working on it (: Might be able to reduce this a bit further to make more sense.</div></div><br><div class="gmail_quote"><div dir="ltr">On Mon, Feb 12, 2018 at 5:56 PM David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Currently reverted in r324981 due to some cases of missing inlining in symbolized results. Will work on getting this sorted out and recommitted with tests+fixes as soon as possible.</div><br><div class="gmail_quote"><div dir="ltr">On Thu, Dec 21, 2017 at 10:42 PM Chandler Carruth via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: chandlerc<br>
Date: Thu Dec 21 22:41:23 2017<br>
New Revision: 321345<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=321345&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=321345&view=rev</a><br>
Log:<br>
Rewrite the cached map used for locating the most precise DIE among<br>
inlined subroutines for a given address.<br>
<br>
This is essentially the hot path of llvm-symbolizer when extracting<br>
inlined frames during symbolization. Previously, we would read every<br>
subprogram and every inlined subroutine, building a std::map across the<br>
entire PC space to the best DIE, and then do only a handful of queries<br>
as we symbolized a backtrace. A huge fraction of the time was spent<br>
building the map itself.<br>
<br>
This patch changes it two a two-level system. First, we just build a map<br>
from PC-interval to DWARF subprograms. These are required to be disjoint<br>
and so constructing this is pretty easy. Second, we build a map *just*<br>
for the inlined subroutines within the subprogram containing the query<br>
address. This allows us to look at far fewer DIEs and build a *much*<br>
smaller set of cached maps in the llvm-symbolizer case where only a few<br>
address get symbolized during the entire run.<br>
<br>
It also builds both interval maps in a very different way. It constructs<br>
a single flat vector of pairs that maps from offset -> index. The<br>
indices point into collections of DIE objects, but can also be<br>
"tombstones" (-1) to mark gaps. In the case of subprograms, this mostly<br>
just simplifies the data structure a bit. For inlined subroutines,<br>
because we carefully split them as we build the map, we end up in many<br>
cases having no holes and not having to store both start and stop<br>
offsets.<br>
<br>
Finally, the PC ranges for the inlined subroutines are compressed into<br>
32-bits by making them relative to the base PC of the outer subprogram.<br>
This means that if you have a single function body with over 2gb of<br>
executable code in it, we will stop mapping address past the first 2gb<br>
of that function into inlined subroutines and just give you the<br>
subprogram. This doesn't seem like a problem. ;]<br>
<br>
All of this combines to make llvm-symbolizer *well* over 2x faster for<br>
symbolizing backtraces out of LLVM's unittests. Death-test heavy unit<br>
tests are running >2x faster. I'm still going to look at completely<br>
disabling symbolization there, but figured while I had a good benchmark<br>
we should make symbolization a bit better.<br>
<br>
Sadly, the logic to build the flat interval map for the inlined<br>
subroutines is fairly complex. I'm not super happy about this and<br>
welcome any simplifying suggestions.<br>
<br>
Huge thanks to Dave Blaikie who helped walk me through what the various<br>
things I needed to do in DWARF to make this work.<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D40987" rel="noreferrer" target="_blank">https://reviews.llvm.org/D40987</a><br>
<br>
Modified:<br>
    llvm/trunk/include/llvm/DebugInfo/DWARF/DWARFUnit.h<br>
    llvm/trunk/lib/DebugInfo/DWARF/DWARFUnit.cpp<br>
<br>
Modified: llvm/trunk/include/llvm/DebugInfo/DWARF/DWARFUnit.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/DebugInfo/DWARF/DWARFUnit.h?rev=321345&r1=321344&r2=321345&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/DebugInfo/DWARF/DWARFUnit.h?rev=321345&r1=321344&r2=321345&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/DebugInfo/DWARF/DWARFUnit.h (original)<br>
+++ llvm/trunk/include/llvm/DebugInfo/DWARF/DWARFUnit.h Thu Dec 21 22:41:23 2017<br>
@@ -220,10 +220,40 @@ class DWARFUnit {<br>
   /// The compile unit debug information entry items.<br>
   std::vector<DWARFDebugInfoEntry> DieArray;<br>
<br>
-  /// Map from range's start address to end address and corresponding DIE.<br>
-  /// IntervalMap does not support range removal, as a result, we use the<br>
-  /// std::map::upper_bound for address range lookup.<br>
-  std::map<uint64_t, std::pair<uint64_t, DWARFDie>> AddrDieMap;<br>
+  /// The vector of inlined subroutine DIEs that we can map directly to from<br>
+  /// their subprogram below.<br>
+  std::vector<DWARFDie> InlinedSubroutineDIEs;<br>
+<br>
+  /// A type representing a subprogram DIE and a map (built using a sorted<br>
+  /// vector) into that subprogram's inlined subroutine DIEs.<br>
+  struct SubprogramDIEAddrInfo {<br>
+    DWARFDie SubprogramDIE;<br>
+<br>
+    uint64_t SubprogramBasePC;<br>
+<br>
+    /// A vector sorted to allow mapping from a relative PC to the inlined<br>
+    /// subroutine DIE with the most specific address range covering that PC.<br>
+    ///<br>
+    /// The PCs are relative to the `SubprogramBasePC`.<br>
+    ///<br>
+    /// The vector is sorted in ascending order of the first int which<br>
+    /// represents the relative PC for an interval in the map. The second int<br>
+    /// represents the index into the `InlinedSubroutineDIEs` vector of the DIE<br>
+    /// that interval maps to. An index of '-1` indicates an empty mapping. The<br>
+    /// interval covered is from the `.first` relative PC to the next entry's<br>
+    /// `.first` relative PC.<br>
+    std::vector<std::pair<uint32_t, int32_t>> InlinedSubroutineDIEAddrMap;<br>
+  };<br>
+<br>
+  /// Vector of the subprogram DIEs and their subroutine address maps.<br>
+  std::vector<SubprogramDIEAddrInfo> SubprogramDIEAddrInfos;<br>
+<br>
+  /// A vector sorted to allow mapping from a PC to the subprogram DIE (and<br>
+  /// associated addr map) index. Subprograms with overlapping PC ranges aren't<br>
+  /// supported here. Nothing will crash, but the mapping may be inaccurate.<br>
+  /// This vector may also contain "empty" ranges marked by an address with<br>
+  /// a DIE index of '-1'.<br>
+  std::vector<std::pair<uint64_t, int64_t>> SubprogramDIEAddrMap;<br>
<br>
   using die_iterator_range =<br>
       iterator_range<std::vector<DWARFDebugInfoEntry>::iterator>;<br>
@@ -282,9 +312,6 @@ public:<br>
     AddrOffsetSectionBase = Base;<br>
   }<br>
<br>
-  /// Recursively update address to Die map.<br>
-  void updateAddressDieMap(DWARFDie Die);<br>
-<br>
   void setRangesSection(const DWARFSection *RS, uint32_t Base) {<br>
     RangeSection = RS;<br>
     RangeSectionBase = Base;<br>
@@ -480,6 +507,9 @@ private:<br>
   /// parseDWO - Parses .dwo file for current compile unit. Returns true if<br>
   /// it was actually constructed.<br>
   bool parseDWO();<br>
+<br>
+  void buildSubprogramDIEAddrMap();<br>
+  void buildInlinedSubroutineDIEAddrMap(SubprogramDIEAddrInfo &SPInfo);<br>
 };<br>
<br>
 } // end namespace llvm<br>
<br>
Modified: llvm/trunk/lib/DebugInfo/DWARF/DWARFUnit.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/DWARF/DWARFUnit.cpp?rev=321345&r1=321344&r2=321345&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/DWARF/DWARFUnit.cpp?rev=321345&r1=321344&r2=321345&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/DebugInfo/DWARF/DWARFUnit.cpp (original)<br>
+++ llvm/trunk/lib/DebugInfo/DWARF/DWARFUnit.cpp Thu Dec 21 22:41:23 2017<br>
@@ -8,6 +8,7 @@<br>
 //===----------------------------------------------------------------------===//<br>
<br>
 #include "llvm/DebugInfo/DWARF/DWARFUnit.h"<br>
+#include "llvm/ADT/STLExtras.h"<br>
 #include "llvm/ADT/SmallString.h"<br>
 #include "llvm/ADT/StringRef.h"<br>
 #include "llvm/DebugInfo/DWARF/DWARFAbbreviationDeclaration.h"<br>
@@ -359,45 +360,378 @@ void DWARFUnit::collectAddressRanges(DWA<br>
     clearDIEs(true);<br>
 }<br>
<br>
-void DWARFUnit::updateAddressDieMap(DWARFDie Die) {<br>
-  if (Die.isSubroutineDIE()) {<br>
+// Populates a map from PC addresses to subprogram DIEs.<br>
+//<br>
+// This routine tries to look at the smallest amount of the debug info it can<br>
+// to locate the DIEs. This is because many subprograms will never end up being<br>
+// read or needed at all. We want to be as lazy as possible.<br>
+void DWARFUnit::buildSubprogramDIEAddrMap() {<br>
+  assert(SubprogramDIEAddrMap.empty() && "Must only build this map once!");<br>
+  SmallVector<DWARFDie, 16> Worklist;<br>
+  Worklist.push_back(getUnitDIE());<br>
+  do {<br>
+    DWARFDie Die = Worklist.pop_back_val();<br>
+<br>
+    // Queue up child DIEs to recurse through.<br>
+    // FIXME: This causes us to read a lot more debug info than we really need.<br>
+    // We should look at pruning out DIEs which cannot transitively hold<br>
+    // separate subprograms.<br>
+    for (DWARFDie Child : Die.children())<br>
+      Worklist.push_back(Child);<br>
+<br>
+    // If handling a non-subprogram DIE, nothing else to do.<br>
+    if (!Die.isSubprogramDIE())<br>
+      continue;<br>
+<br>
+    // For subprogram DIEs, store them, and insert relevant markers into the<br>
+    // address map. We don't care about overlap at all here as DWARF doesn't<br>
+    // meaningfully support that, so we simply will insert a range with no DIE<br>
+    // starting from the high PC. In the event there are overlaps, sorting<br>
+    // these may truncate things in surprising ways but still will allow<br>
+    // lookups to proceed.<br>
+    int DIEIndex = SubprogramDIEAddrInfos.size();<br>
+    SubprogramDIEAddrInfos.push_back({Die, (uint64_t)-1, {}});<br>
     for (const auto &R : Die.getAddressRanges()) {<br>
       // Ignore 0-sized ranges.<br>
       if (R.LowPC == R.HighPC)<br>
         continue;<br>
-      auto B = AddrDieMap.upper_bound(R.LowPC);<br>
-      if (B != AddrDieMap.begin() && R.LowPC < (--B)->second.first) {<br>
-        // The range is a sub-range of existing ranges, we need to split the<br>
-        // existing range.<br>
-        if (R.HighPC < B->second.first)<br>
-          AddrDieMap[R.HighPC] = B->second;<br>
-        if (R.LowPC > B->first)<br>
-          AddrDieMap[B->first].first = R.LowPC;<br>
+<br>
+      SubprogramDIEAddrMap.push_back({R.LowPC, DIEIndex});<br>
+      SubprogramDIEAddrMap.push_back({R.HighPC, -1});<br>
+<br>
+      if (R.LowPC < SubprogramDIEAddrInfos.back().SubprogramBasePC)<br>
+        SubprogramDIEAddrInfos.back().SubprogramBasePC = R.LowPC;<br>
+    }<br>
+  } while (!Worklist.empty());<br>
+<br>
+  if (SubprogramDIEAddrMap.empty()) {<br>
+    // If we found no ranges, create a no-op map so that lookups remain simple<br>
+    // but never find anything.<br>
+    SubprogramDIEAddrMap.push_back({0, -1});<br>
+    return;<br>
+  }<br>
+<br>
+  // Next, sort the ranges and remove both exact duplicates and runs with the<br>
+  // same DIE index. We order the ranges so that non-empty ranges are<br>
+  // preferred. Because there may be ties, we also need to use stable sort.<br>
+  std::stable_sort(SubprogramDIEAddrMap.begin(), SubprogramDIEAddrMap.end(),<br>
+                   [](const std::pair<uint64_t, int64_t> &LHS,<br>
+                      const std::pair<uint64_t, int64_t> &RHS) {<br>
+                     if (LHS.first < RHS.first)<br>
+                       return true;<br>
+                     if (LHS.first > RHS.first)<br>
+                       return false;<br>
+<br>
+                     // For ranges that start at the same address, keep the one<br>
+                     // with a DIE.<br>
+                     if (LHS.second != -1 && RHS.second == -1)<br>
+                       return true;<br>
+<br>
+                     return false;<br>
+                   });<br>
+  SubprogramDIEAddrMap.erase(<br>
+      std::unique(SubprogramDIEAddrMap.begin(), SubprogramDIEAddrMap.end(),<br>
+                  [](const std::pair<uint64_t, int64_t> &LHS,<br>
+                     const std::pair<uint64_t, int64_t> &RHS) {<br>
+                    // If the start addresses are exactly the same, we can<br>
+                    // remove all but the first one as it is the only one that<br>
+                    // will be found and used.<br>
+                    //<br>
+                    // If the DIE indices are the same, we can "merge" the<br>
+                    // ranges by eliminating the second.<br>
+                    return LHS.first == RHS.first || LHS.second == RHS.second;<br>
+                  }),<br>
+      SubprogramDIEAddrMap.end());<br>
+<br>
+  assert(SubprogramDIEAddrMap.back().second == -1 &&<br>
+         "The last interval must not have a DIE as each DIE's address range is "<br>
+         "bounded.");<br>
+}<br>
+<br>
+// Build the second level of mapping from PC to DIE, specifically one that maps<br>
+// a PC *within* a particular DWARF subprogram into a precise, maximally nested<br>
+// inlined subroutine DIE (if any exists). We build a separate map for each<br>
+// subprogram because many subprograms will never get queried for an address<br>
+// and this allows us to be significantly lazier in reading the DWARF itself.<br>
+void DWARFUnit::buildInlinedSubroutineDIEAddrMap(<br>
+    SubprogramDIEAddrInfo &SPInfo) {<br>
+  auto &AddrMap = SPInfo.InlinedSubroutineDIEAddrMap;<br>
+  uint64_t BasePC = SPInfo.SubprogramBasePC;<br>
+<br>
+  auto SubroutineAddrMapSorter = [](const std::pair<int, int> &LHS,<br>
+                                    const std::pair<int, int> &RHS) {<br>
+    if (LHS.first < RHS.first)<br>
+      return true;<br>
+    if (LHS.first > RHS.first)<br>
+      return false;<br>
+<br>
+    // For ranges that start at the same address, keep the<br>
+    // non-empty one.<br>
+    if (LHS.second != -1 && RHS.second == -1)<br>
+      return true;<br>
+<br>
+    return false;<br>
+  };<br>
+  auto SubroutineAddrMapUniquer = [](const std::pair<int, int> &LHS,<br>
+                                     const std::pair<int, int> &RHS) {<br>
+    // If the start addresses are exactly the same, we can<br>
+    // remove all but the first one as it is the only one that<br>
+    // will be found and used.<br>
+    //<br>
+    // If the DIE indices are the same, we can "merge" the<br>
+    // ranges by eliminating the second.<br>
+    return LHS.first == RHS.first || LHS.second == RHS.second;<br>
+  };<br>
+<br>
+  struct DieAndParentIntervalRange {<br>
+    DWARFDie Die;<br>
+    int ParentIntervalsBeginIdx, ParentIntervalsEndIdx;<br>
+  };<br>
+<br>
+  SmallVector<DieAndParentIntervalRange, 16> Worklist;<br>
+  auto EnqueueChildDIEs = [&](const DWARFDie &Die, int ParentIntervalsBeginIdx,<br>
+                              int ParentIntervalsEndIdx) {<br>
+    for (DWARFDie Child : Die.children())<br>
+      Worklist.push_back(<br>
+          {Child, ParentIntervalsBeginIdx, ParentIntervalsEndIdx});<br>
+  };<br>
+  EnqueueChildDIEs(SPInfo.SubprogramDIE, 0, 0);<br>
+  while (!Worklist.empty()) {<br>
+    DWARFDie Die = Worklist.back().Die;<br>
+    int ParentIntervalsBeginIdx = Worklist.back().ParentIntervalsBeginIdx;<br>
+    int ParentIntervalsEndIdx = Worklist.back().ParentIntervalsEndIdx;<br>
+    Worklist.pop_back();<br>
+<br>
+    // If we encounter a nested subprogram, simply ignore it. We map to<br>
+    // (disjoint) subprograms before arriving here and we don't want to examine<br>
+    // any inlined subroutines of an unrelated subpragram.<br>
+    if (Die.getTag() == DW_TAG_subprogram)<br>
+      continue;<br>
+<br>
+    // For non-subroutines, just recurse to keep searching for inlined<br>
+    // subroutines.<br>
+    if (Die.getTag() != DW_TAG_inlined_subroutine) {<br>
+      EnqueueChildDIEs(Die, ParentIntervalsBeginIdx, ParentIntervalsEndIdx);<br>
+      continue;<br>
+    }<br>
+<br>
+    // Capture the inlined subroutine DIE that we will reference from the map.<br>
+    int DIEIndex = InlinedSubroutineDIEs.size();<br>
+    InlinedSubroutineDIEs.push_back(Die);<br>
+<br>
+    int DieIntervalsBeginIdx = AddrMap.size();<br>
+    // First collect the PC ranges for this DIE into our subroutine interval<br>
+    // map.<br>
+    for (auto R : Die.getAddressRanges()) {<br>
+      // Clamp the PCs to be above the base.<br>
+      R.LowPC = std::max(R.LowPC, BasePC);<br>
+      R.HighPC = std::max(R.HighPC, BasePC);<br>
+      // Compute relative PCs from the subprogram base and drop down to an<br>
+      // unsigned 32-bit int to represent them within the data structure. This<br>
+      // lets us cover a 4gb single subprogram. Because subprograms may be<br>
+      // partitioned into distant parts of a binary (think hot/cold<br>
+      // partitioning) we want to preserve as much as we can here without<br>
+      // burning extra memory. Past that, we will simply truncate and lose the<br>
+      // ability to map those PCs to a DIE more precise than the subprogram.<br>
+      const uint32_t MaxRelativePC = std::numeric_limits<uint32_t>::max();<br>
+      uint32_t RelativeLowPC = (R.LowPC - BasePC) > (uint64_t)MaxRelativePC<br>
+                                   ? MaxRelativePC<br>
+                                   : (uint32_t)(R.LowPC - BasePC);<br>
+      uint32_t RelativeHighPC = (R.HighPC - BasePC) > (uint64_t)MaxRelativePC<br>
+                                    ? MaxRelativePC<br>
+                                    : (uint32_t)(R.HighPC - BasePC);<br>
+      // Ignore empty or bogus ranges.<br>
+      if (RelativeLowPC >= RelativeHighPC)<br>
+        continue;<br>
+      AddrMap.push_back({RelativeLowPC, DIEIndex});<br>
+      AddrMap.push_back({RelativeHighPC, -1});<br>
+    }<br>
+<br>
+    // If there are no address ranges, there is nothing to do to map into them<br>
+    // and there cannot be any child subroutine DIEs with address ranges of<br>
+    // interest as those would all be required to nest within this DIE's<br>
+    // non-existent ranges, so we can immediately continue to the next DIE in<br>
+    // the worklist.<br>
+    if (DieIntervalsBeginIdx == (int)AddrMap.size())<br>
+      continue;<br>
+<br>
+    // The PCs from this DIE should never overlap, so we can easily sort them<br>
+    // here.<br>
+    std::sort(AddrMap.begin() + DieIntervalsBeginIdx, AddrMap.end(),<br>
+              SubroutineAddrMapSorter);<br>
+    // Remove any dead ranges. These should only come from "empty" ranges that<br>
+    // were clobbered by some other range.<br>
+    AddrMap.erase(std::unique(AddrMap.begin() + DieIntervalsBeginIdx,<br>
+                              AddrMap.end(), SubroutineAddrMapUniquer),<br>
+                  AddrMap.end());<br>
+<br>
+    // Compute the end index of this DIE's addr map intervals.<br>
+    int DieIntervalsEndIdx = AddrMap.size();<br>
+<br>
+    assert(DieIntervalsBeginIdx != DieIntervalsEndIdx &&<br>
+           "Must not have an empty map for this layer!");<br>
+    assert(AddrMap.back().second == -1 && "Must end with an empty range!");<br>
+    assert(std::is_sorted(AddrMap.begin() + DieIntervalsBeginIdx, AddrMap.end(),<br>
+                          less_first()) &&<br>
+           "Failed to sort this DIE's interals!");<br>
+<br>
+    // If we have any parent intervals, walk the newly added ranges and find<br>
+    // the parent ranges they were inserted into. Both of these are sorted and<br>
+    // neither has any overlaps. We need to append new ranges to split up any<br>
+    // parent ranges these new ranges would overlap when we merge them.<br>
+    if (ParentIntervalsBeginIdx != ParentIntervalsEndIdx) {<br>
+      int ParentIntervalIdx = ParentIntervalsBeginIdx;<br>
+      for (int i = DieIntervalsBeginIdx, e = DieIntervalsEndIdx - 1; i < e;<br>
+           ++i) {<br>
+        const uint32_t IntervalStart = AddrMap[i].first;<br>
+        const uint32_t IntervalEnd = AddrMap[i + 1].first;<br>
+        const int IntervalDieIdx = AddrMap[i].second;<br>
+        if (IntervalDieIdx == -1) {<br>
+          // For empty intervals, nothing is required. This is a bit surprising<br>
+          // however. If the prior interval overlaps a parent interval and this<br>
+          // would be necessary to mark the end, we will synthesize a new end<br>
+          // that switches back to the parent DIE below. And this interval will<br>
+          // get dropped in favor of one with a DIE attached. However, we'll<br>
+          // still include this and so worst-case, it will still end the prior<br>
+          // interval.<br>
+          continue;<br>
+        }<br>
+<br>
+        // We are walking the new ranges in order, so search forward from the<br>
+        // last point for a parent range that might overlap.<br>
+        auto ParentIntervalsRange =<br>
+            make_range(AddrMap.begin() + ParentIntervalIdx,<br>
+                       AddrMap.begin() + ParentIntervalsEndIdx);<br>
+        assert(std::is_sorted(ParentIntervalsRange.begin(),<br>
+                              ParentIntervalsRange.end(), less_first()) &&<br>
+               "Unsorted parent intervals can't be searched!");<br>
+        auto PI = std::upper_bound(<br>
+            ParentIntervalsRange.begin(), ParentIntervalsRange.end(),<br>
+            IntervalStart,<br>
+            [](uint32_t LHS, const std::pair<uint32_t, int32_t> &RHS) {<br>
+              return LHS < RHS.first;<br>
+            });<br>
+        if (PI == ParentIntervalsRange.begin() ||<br>
+            PI == ParentIntervalsRange.end())<br>
+          continue;<br>
+<br>
+        ParentIntervalIdx = PI - AddrMap.begin();<br>
+        int32_t &ParentIntervalDieIdx = std::prev(PI)->second;<br>
+        uint32_t &ParentIntervalStart = std::prev(PI)->first;<br>
+        const uint32_t ParentIntervalEnd = PI->first;<br>
+<br>
+        // If the new range starts exactly at the position of the parent range,<br>
+        // we need to adjust the parent range. Note that these collisions can<br>
+        // only happen with the original parent range because we will merge any<br>
+        // adjacent ranges in the child.<br>
+        if (IntervalStart == ParentIntervalStart) {<br>
+          // If there will be a tail, just shift the start of the parent<br>
+          // forward. Note that this cannot change the parent ordering.<br>
+          if (IntervalEnd < ParentIntervalEnd) {<br>
+            ParentIntervalStart = IntervalEnd;<br>
+            continue;<br>
+          }<br>
+          // Otherwise, mark this as becoming empty so we'll remove it and<br>
+          // prefer the child range.<br>
+          ParentIntervalDieIdx = -1;<br>
+          continue;<br>
+        }<br>
+<br>
+        // Finally, if the parent interval will need to remain as a prefix to<br>
+        // this one, insert a new interval to cover any tail.<br>
+        if (IntervalEnd < ParentIntervalEnd)<br>
+          AddrMap.push_back({IntervalEnd, ParentIntervalDieIdx});<br>
       }<br>
-      AddrDieMap[R.LowPC] = std::make_pair(R.HighPC, Die);<br>
     }<br>
+<br>
+    // Note that we don't need to re-sort even this DIE's address map intervals<br>
+    // after this. All of the newly added intervals actually fill in *gaps* in<br>
+    // this DIE's address map, and we know that children won't need to lookup<br>
+    // into those gaps.<br>
+<br>
+    // Recurse through its children, giving them the interval map range of this<br>
+    // DIE to use as their parent intervals.<br>
+    EnqueueChildDIEs(Die, DieIntervalsBeginIdx, DieIntervalsEndIdx);<br>
   }<br>
-  // Parent DIEs are added to the AddrDieMap prior to the Children DIEs to<br>
-  // simplify the logic to update AddrDieMap. The child's range will always<br>
-  // be equal or smaller than the parent's range. With this assumption, when<br>
-  // adding one range into the map, it will at most split a range into 3<br>
-  // sub-ranges.<br>
-  for (DWARFDie Child = Die.getFirstChild(); Child; Child = Child.getSibling())<br>
-    updateAddressDieMap(Child);<br>
+<br>
+  if (AddrMap.empty()) {<br>
+    AddrMap.push_back({0, -1});<br>
+    return;<br>
+  }<br>
+<br>
+  // Now that we've added all of the intervals needed, we need to resort and<br>
+  // unique them. Most notably, this will remove all the empty ranges that had<br>
+  // a parent range covering, etc. We only expect a single non-empty interval<br>
+  // at any given start point, so we just use std::sort. This could potentially<br>
+  // produce non-deterministic maps for invalid DWARF.<br>
+  std::sort(AddrMap.begin(), AddrMap.end(), SubroutineAddrMapSorter);<br>
+  AddrMap.erase(<br>
+      std::unique(AddrMap.begin(), AddrMap.end(), SubroutineAddrMapUniquer),<br>
+      AddrMap.end());<br>
 }<br>
<br>
 DWARFDie DWARFUnit::getSubroutineForAddress(uint64_t Address) {<br>
   extractDIEsIfNeeded(false);<br>
-  if (AddrDieMap.empty())<br>
-    updateAddressDieMap(getUnitDIE());<br>
-  auto R = AddrDieMap.upper_bound(Address);<br>
-  if (R == AddrDieMap.begin())<br>
+<br>
+  // We use a two-level mapping structure to locate subroutines for a given PC<br>
+  // address.<br>
+  //<br>
+  // First, we map the address to a subprogram. This can be done more cheaply<br>
+  // because subprograms cannot nest within each other. It also allows us to<br>
+  // avoid detailed examination of many subprograms, instead only focusing on<br>
+  // the ones which we end up actively querying.<br>
+  if (SubprogramDIEAddrMap.empty())<br>
+    buildSubprogramDIEAddrMap();<br>
+<br>
+  assert(!SubprogramDIEAddrMap.empty() &&<br>
+         "We must always end up with a non-empty map!");<br>
+<br>
+  auto I = std::upper_bound(<br>
+      SubprogramDIEAddrMap.begin(), SubprogramDIEAddrMap.end(), Address,<br>
+      [](uint64_t LHS, const std::pair<uint64_t, int64_t> &RHS) {<br>
+        return LHS < RHS.first;<br>
+      });<br>
+  // If we find the beginning, then the address is before the first subprogram.<br>
+  if (I == SubprogramDIEAddrMap.begin())<br>
     return DWARFDie();<br>
-  // upper_bound's previous item contains Address.<br>
-  --R;<br>
-  if (Address >= R->second.first)<br>
+  // Back up to the interval containing the address and see if it<br>
+  // has a DIE associated with it.<br>
+  --I;<br>
+  if (I->second == -1)<br>
     return DWARFDie();<br>
-  return R->second.second;<br>
+<br>
+  auto &SPInfo = SubprogramDIEAddrInfos[I->second];<br>
+<br>
+  // Now that we have the subprogram for this address, we do the second level<br>
+  // mapping by building a map within a subprogram's PC range to any specific<br>
+  // inlined subroutine.<br>
+  if (SPInfo.InlinedSubroutineDIEAddrMap.empty())<br>
+    buildInlinedSubroutineDIEAddrMap(SPInfo);<br>
+<br>
+  // We lookup within the inlined subroutine using a subprogram-relative<br>
+  // address.<br>
+  assert(Address >= SPInfo.SubprogramBasePC &&<br>
+         "Address isn't above the start of the subprogram!");<br>
+  uint32_t RelativeAddr = ((Address - SPInfo.SubprogramBasePC) ><br>
+                           (uint64_t)std::numeric_limits<uint32_t>::max())<br>
+                              ? std::numeric_limits<uint32_t>::max()<br>
+                              : (uint32_t)(Address - SPInfo.SubprogramBasePC);<br>
+<br>
+  auto J =<br>
+      std::upper_bound(SPInfo.InlinedSubroutineDIEAddrMap.begin(),<br>
+                       SPInfo.InlinedSubroutineDIEAddrMap.end(), RelativeAddr,<br>
+                       [](uint32_t LHS, const std::pair<uint32_t, int32_t> &RHS) {<br>
+                         return LHS < RHS.first;<br>
+                       });<br>
+  // If we find the beginning, the address is before any inlined subroutine so<br>
+  // return the subprogram DIE.<br>
+  if (J == SPInfo.InlinedSubroutineDIEAddrMap.begin())<br>
+    return SPInfo.SubprogramDIE;<br>
+  // Back up `J` and return the inlined subroutine if we have one or the<br>
+  // subprogram if we don't.<br>
+  --J;<br>
+  return J->second == -1 ? SPInfo.SubprogramDIE<br>
+                         : InlinedSubroutineDIEs[J->second];<br>
 }<br>
<br>
 void<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div></blockquote></div></blockquote></div></div></blockquote></div>