[PATCH] Fix DWARFContext::getCompileUnitForOffset().

[David Blaikie]

================
Comment at: lib/DebugInfo/DWARFUnit.h:66
@@ -65,3 +65,3 @@
   UnitType *getUnitForOffset(uint32_t Offset) const {
-    auto *CU = std::lower_bound(this->begin(), this->end(), Offset,
+    auto *CU = std::upper_bound(this->begin(), this->end(), Offset,
                                 UnitOffsetComparator());
----------------
friss wrote:
> samsonov wrote:
> > friss wrote:
> > > samsonov wrote:
> > > > You still need to check that Offset you're looking for is inside the CU.
> > > I might be wrong, but as I answered last time you told me that, I think this is not true. The admissible offsets go from 0 to lastUnit->getNextUnitOffset() exclusive. There is no hole in the section.
> > Sorry, I've received both answers at the same time (probably, your first comment was saved as draft but not submitted).
> > 
> > You are right - there are no holes. Please add a comment about it and feel free to submit this.
> Thanks! And sorry, I should have checked that the old review reply got sent out.
> David are you also OK with this? I gathered that maybe you wanted to look more into the implications of this.
Thanks for looping back around/checking.

I'm still unclear on a few things here:

Do we need all 3 comparison operations? (if we only need one, I won't have to think so hard about the others - if we only need one, which I think we do (specifically the "(unique_ptr<Unit>, unsigned offset)")) - if we don't, please just delete the two unneeded ones in a preliminary cleanup patch to simplify this one.

getNextUnitOffset < Offset seems... OK, thinking about it I see that it makes sense. I'm not sure if a comment would be helpful to explain why (I can't think of a simple comment that really helps).

Looks fine to me with the optional preliminary cleanup mentioned above.

(we should be able to use code coverage to see that the other two comparison functions are never executed... someone maintains a code coverage website for LLVM but I forget the URL)

http://reviews.llvm.org/D5262