[PATCH] D32228: [DWARF] - Take relocations in account when extracting ranges from .debug_ranges

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 20 10:07:59 PDT 2017


dblaikie added inline comments.


================
Comment at: include/llvm/DebugInfo/DWARF/DWARFContext.h:234
   virtual StringRef getStringOffsetDWOSection() = 0;
-  virtual StringRef getRangeDWOSection() = 0;
+  virtual const DWARFSection &getRangeDWOSection() = 0;
   virtual StringRef getAddrSection() = 0;
----------------
grimar wrote:
> dblaikie wrote:
> > I don't think there is a ranges.dwo section, is there? The ranges section is in the .o file, always. (the DWARF5 rnglists can go in the .dwo, though)
> There is no ranges.dwo in my testcase, but I had to change this API too, because I changed DWARFUnitSectionBase::parseImpl (please see DWARFUnit.h) to take const DWARFSection *RS.
> 
> I think I can't change return type for getRangeSection() without changing return type for getRangeDWOSection(),
> because DWARFUnitSectionBase::parse() pushes getRangeSection(), but DWARFUnitSectionBase::parseDWO() pushes C.getRangeDWOSection() for RS.
> 
Looks like a bug in the code - since there is no range.dwo section this probably causes llvm-dwarfdump to be less good than it could be. If parseDWOCompileUnits passed getRangeSection, then at least when dumping an unsplit split-dwarf file (ie: the object file that comes out of the clang frontend before objcopy is run on it to split the .dwo sections) it would dump ranges correctly, I think.

If you like - you could send a separate patch befoer or after this one to clean that up (create a small example object file with ranges in it - show that changing that call in parseDWOCompileUnits does cause the ranges to be correctly dumped - and then also strip out all the ranges.dwo stuff)

Ah, not a bug, because RangeDWOSection is the same as RangeSection... just confusing.

The support for rnglists.dwo (which I'm guessing is what the original comment that setup RangesDWO was referring to)will look very different from all of this anyway, I think.


================
Comment at: lib/DebugInfo/DWARF/DWARFDebugRangeList.cpp:48
     }
-    if (entry.isEndOfListEntry())
+    if (entry.isEndOfListEntry(Relocs, prev_offset, AddressSize))
       break;
----------------
Isn't this applying the relocation twice? The entry's Start/EndAddress are already relocated above so there shouldn't be any need to look up relocations again here, should there?

This is to handle the case where a function wasn't selected (comdat, etc) & so its base address resolves to zero by the linker - and specifically when such a function was empty anyway? (eg: "int f()" - absolutely no instructions, so begin/end are the same address)? /maybe/ that's correct/useful, but I doubt it. I'm not sure how important it is to support zero-length ranges in a range list... maybe for debugging weird DWARF output, etc.


https://reviews.llvm.org/D32228





More information about the llvm-commits mailing list