[PATCH] D37214: Another prototype fix for lld DWARF parsing of base address selection entries in range lists

[David Blaikie via llvm-commits]

On Thu, Aug 31, 2017 at 1:08 AM George Rimar via Phabricator <
reviews at reviews.llvm.org> wrote:

> grimar added inline comments.
>
>
> ================
> Comment at: include/llvm/DebugInfo/DWARF/DWARFUnit.h:271-272
> +  void setBaseAddress(BaseAddress BaseAddr) {
> +    if (!BaseAddr.Address && BaseAddr.SectionIndex == -1ULL)
> +      return;
> +    this->BaseAddr = BaseAddr;
> ----------------
> dblaikie wrote:
> > 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?
> It is for dwarfdump-ranges-unrelocated.s testcase. It is based on
> following code:
>
> ```
> test.cpp:
>    void foo1() { }
>    void foo2() { }
> clang test.cpp -S -o test.s -gmlt -ffunction-sections
> ```
>
> And output produced is:
>
> ```
>         .section        .debug_info,"", at progbits
> .Lcu_begin0:
>         .long   38                      # Length of Unit
>         .short  4                       # DWARF version number
>         .long   .debug_abbrev           # Offset Into Abbrev. Section
>         .byte   8                       # Address Size (in bytes)
>         .byte   1                       # Abbrev [1] 0xb:0x1f
> DW_TAG_compile_unit
>         .long   .Linfo_string0          # DW_AT_producer
>         .short  4                       # DW_AT_language
>         .long   .Linfo_string1          # DW_AT_name
>         .long   .Lline_table_start0     # DW_AT_stmt_list
>         .long   .Linfo_string2          # DW_AT_comp_dir
>         .quad   0                       # DW_AT_low_pc
>         .long   .Ldebug_ranges0         # DW_AT_ranges
>         .section        .debug_ranges,"", at progbits
> .Ldebug_ranges0:
>         .quad   .Lfunc_begin0
>         .quad   .Lfunc_end0
>         .quad   .Lfunc_begin1
>         .quad   .Lfunc_end1
>         .quad   0
>         .quad   0
> ```
>
> So it has ranges but also unrelocated zero base address is emited in debug
> info. Since it has no
> corresponding relocation (`SectionIndex` == -1ULL), I am ignoring it to
> prevent using
> -1ULL as section index name.


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)


> That looks a bit hacky, but I currently see no other way to distinguish
> between such "dummy" base address and real one.
> In testcase it is shown that base address can be 0 after relocation is
> applied, so I think we need
> to check both `SectionIndex` and relocated value, just like I do.
>
>
> I moved that code to caller side, thats probably a bit more clear and
> localizes the hack.
>
>
> ================
> Comment at: lib/DebugInfo/DWARF/DWARFDebugRangeList.cpp:71
>      if (RLE.isBaseAddressSelectionEntry(AddressSize)) {
> -      BaseAddress = RLE.EndAddress;
> -    } else {
> -      Res.push_back({BaseAddress + RLE.StartAddress,
> -                     BaseAddress + RLE.EndAddress, RLE.SectionIndex});
> +      BaseAddr = { RLE.EndAddress, RLE.SectionIndex };
> +      continue;
> ----------------
> dblaikie wrote:
> > Is this clang-formatted? I'd expect no spaces inner sides of the {}
> You right, fixed.
>
>
> https://reviews.llvm.org/D37214
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170831/202cd059/attachment.html>