[PATCH] D79269: [NativeSession] Implement NativeSession::findSymbolByAddress.

[Amy Huang via Phabricator via llvm-commits]

akhuang marked an inline comment as done.
akhuang added inline comments.


================
Comment at: llvm/lib/DebugInfo/PDB/Native/SymbolCache.cpp:427
+      uint64_t End = VA + C.Size;
+      IMap.insert(VA, End - 1, C.Imod);
+    }
----------------
amccarth wrote:
> labath wrote:
> > IntervalMap explodes if you try to insert overlapping intervals into it. How sure are you that no input will contain overlapping addresses?
> > 
> > I'm not sure what's the pdb position on asserting on invalid inputs, but it certainly seems like this could happen with a hand-crafted/corrupted pdbs. However, I wouldn't be surprised if that happens for "valid" pdbs too...
> Can you clarify "IntervalMap explodes"?  Crashes, hangs, uses crazy amounts of memory or time, corrupts itself?
> 
> From the IntervalMap comments:
> 
> ```
> /// insert - Add a mapping of [a;b] to y, coalesce with adjacent intervals.
> /// It is assumed that no key in the interval is mapped to another value, but
> /// overlapping intervals already mapped to y will be coalesced.
> ```
> 
> If I understand that correctly, it's not overlapping intervals in general, but ones that give a different value for at least part of the overlap.  Perhaps IntervalMap needs a way to test for a conflicting overlap.
Would it be reasonable to just check for overlap before adding to the map and ignore those?

Also, I hadn't run this on larger pdbs before but it seems like the IntervalMap sometimes segfaults on destruction (in map.visitNodes()). 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79269/new/

https://reviews.llvm.org/D79269