<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Thu, Aug 31, 2017 at 1:08 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/DWARFUnit.h:271-272<br>
+  void setBaseAddress(BaseAddress BaseAddr) {<br>
+    if (!BaseAddr.Address && BaseAddr.SectionIndex == -1ULL)<br>
+      return;<br>
+    this->BaseAddr = BaseAddr;<br>
----------------<br>
dblaikie wrote:<br>
> So why is this bit necessary? Should callers just not try to set a base address if they don't want to, rather than having setBaseAddress ignore the request?<br>
It is for dwarfdump-ranges-unrelocated.s testcase. It is based on following code:<br>
<br>
```<br>
test.cpp:<br>
   void foo1() { }<br>
   void foo2() { }<br>
clang test.cpp -S -o test.s -gmlt -ffunction-sections<br>
```<br>
<br>
And output produced is:<br>
<br>
```<br>
        .section        .debug_info,"",@progbits<br>
.Lcu_begin0:<br>
        .long   38                      # Length of Unit<br>
        .short  4                       # DWARF version number<br>
        .long   .debug_abbrev           # Offset Into Abbrev. Section<br>
        .byte   8                       # Address Size (in bytes)<br>
        .byte   1                       # Abbrev [1] 0xb:0x1f DW_TAG_compile_unit<br>
        .long   .Linfo_string0          # DW_AT_producer<br>
        .short  4                       # DW_AT_language<br>
        .long   .Linfo_string1          # DW_AT_name<br>
        .long   .Lline_table_start0     # DW_AT_stmt_list<br>
        .long   .Linfo_string2          # DW_AT_comp_dir<br>
        .quad   0                       # DW_AT_low_pc<br>
        .long   .Ldebug_ranges0         # DW_AT_ranges<br>
        .section        .debug_ranges,"",@progbits<br>
.Ldebug_ranges0:<br>
        .quad   .Lfunc_begin0<br>
        .quad   .Lfunc_end0<br>
        .quad   .Lfunc_begin1<br>
        .quad   .Lfunc_end1<br>
        .quad   0<br>
        .quad   0<br>
```<br>
<br>
So it has ranges but also unrelocated zero base address is emited in debug info. Since it has no<br>
corresponding relocation (`SectionIndex` == -1ULL), I am ignoring it to prevent using<br>
-1ULL as section index name.</blockquote><div><br>Why would this need to be avoided? Shouldn't that be the default value when no base address is specified and its run on a fully relocated object? (no section index in either the base address nor in the actual addresses in the range list)<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> That looks a bit hacky, but I currently see no other way to distinguish<br>
between such "dummy" base address and real one.<br>
In testcase it is shown that base address can be 0 after relocation is applied, so I think we need<br>
to check both `SectionIndex` and relocated value, just like I do.<br>
<br>
<br>
I moved that code to caller side, thats probably a bit more clear and localizes the hack.<br>
<br>
<br>
================<br>
Comment at: lib/DebugInfo/DWARF/DWARFDebugRangeList.cpp:71<br>
     if (RLE.isBaseAddressSelectionEntry(AddressSize)) {<br>
-      BaseAddress = RLE.EndAddress;<br>
-    } else {<br>
-      Res.push_back({BaseAddress + RLE.StartAddress,<br>
-                     BaseAddress + RLE.EndAddress, RLE.SectionIndex});<br>
+      BaseAddr = { RLE.EndAddress, RLE.SectionIndex };<br>
+      continue;<br>
----------------<br>
dblaikie wrote:<br>
> Is this clang-formatted? I'd expect no spaces inner sides of the {}<br>
You right, fixed.<br>
<br>
<br>
<a href="https://reviews.llvm.org/D37214" rel="noreferrer" target="_blank">https://reviews.llvm.org/D37214</a><br>
<br>
<br>
<br>
</blockquote></div></div>