<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Thu, Apr 20, 2017 at 2:36 AM George Rimar via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">grimar added inline comments.<br>
<br>
<br>
================<br>
Comment at: include/llvm/DebugInfo/DWARF/DWARFContext.h:234<br>
   virtual StringRef getStringOffsetDWOSection() = 0;<br>
-  virtual StringRef getRangeDWOSection() = 0;<br>
+  virtual const DWARFSection &getRangeDWOSection() = 0;<br>
   virtual StringRef getAddrSection() = 0;<br>
----------------<br>
dblaikie wrote:<br>
> 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)<br>
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.<br>
<br>
I think I can't change return type for getRangeSection() without changing return type for getRangeDWOSection(),<br>
because DWARFUnitSectionBase::parse() pushes getRangeSection(), but DWARFUnitSectionBase::parseDWO() pushes C.getRangeDWOSection() for RS.<br>
<br>
<br>
<br>
================<br>
Comment at: lib/DebugInfo/DWARF/DWARFDebugRangeList.cpp:25-31<br>
+static uint64_t getAddress(DataExtractor &Data, uint32_t *Off,<br>
+                           const RelocAddrMap &Relocs) {<br>
+  RelocAddrMap::const_iterator AI = Relocs.find(*Off);<br>
+  if (AI != Relocs.end())<br>
+    return Data.getAddress(Off) + AI->second.second;<br>
+  return Data.getAddress(Off);<br>
+}<br>
----------------<br>
dblaikie wrote:<br>
> Should/can/could this be refactored to use the same code as the case you mentioned was already in place for DW_FORM_addr (high/low pc)?<br>
Done. There was many places that could use some helper method like getAddress() instead of inlined applying relocations to values. All of them now uses getRelocatedValue() introduced in this patch.<br>
<br>
<br>
================<br>
Comment at: test/DebugInfo/dwarfdump-ranges-unrelocated.s:13-15<br>
+#   int foo1() { return 1; }<br>
+#   int foo2() { return 1; }<br>
+#   int foo3() { return 1; }<br>
----------------<br>
dblaikie wrote:<br>
> Would 2 functions (also probably simpler for them to be void and empty functions) would suffice to give the CU a DW_AT_ranges rather than high/low pc?<br>
Yes, fixed. I added nop by myself to asm code though, thats to show that ranges are different explicitly.<br>
I leaved single empty function to show we can dump ranges [0x0, 0x0] correctly now.<br>
Previous diff was not able to do that, though that was not a real problem probably, LLD anyways supposed to use<br>
LoadedObjectInfo::getSectionLoadAddress, so values would never be [0x0, 0x0] even for empty method, not sure about other possible users though.<br></blockquote><div><br>I'm not sure I understand - an empty void function (void f() {}) should not be zero-length. It would contain a single 'ret' instruction, no?<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<br>
<a href="https://reviews.llvm.org/D32228" rel="noreferrer" target="_blank">https://reviews.llvm.org/D32228</a><br>
<br>
<br>
<br>
</blockquote></div></div>