[PATCH] D36097: Prototype fix for lld DWARF parsing of base address selection entries in range lists

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 29 13:05:57 PDT 2017


On Mon, Aug 28, 2017 at 8:15 AM George Rimar via Phabricator <
reviews at reviews.llvm.org> wrote:

> grimar added inline comments.
>
>
> ================
> Comment at: lib/DebugInfo/DWARF/DWARFUnit.cpp:247
> +    if (auto BaseAddr = toAddress(LowPCDie))
> +      setBaseAddress(*BaseAddr, LowPCDie->getSectionIndex());
>      AddrOffsetSectionBase =
> toSectionOffset(UnitDie.find(DW_AT_GNU_addr_base), 0);
> ----------------
> I debugged this patch today and tried to use `Optional<BaseAddr>` here for
> `setBaseAddress`,
> where `BaseAddr` was a struct for address and section index pair:
>
> ```
> struct BaseAddress {
>   uint64_t Address;
>   uint64_t SectionIndex;
> };
> ```


> But I faced following issues when
> run tests from `llvm\test\DebugInfo\X86` and I am not sure how to resolve
> them correctly.
>
> 1) I had to implement `setBaseAddress` in next way:
> ```
>   void setBaseAddress(BaseAddress BaseAddr) {
>     if (!BaseAddr.Address && BaseAddr.SectionIndex == -1ULL)
>       return;
>     assert(BaseAddr.SectionIndex != -1ULL);
>     this->BaseAddr = BaseAddr;
>   }
> ```
>
> I can not justify first 2 lines for myself though. it looks that is is
> common that CU has `DW_AT_low_pc` set to 0
> without corresponding relocation.
>
> For example I can take following code and invocation:
>
> ```
> # clang test.cpp -S -o test.s -gmlt -ffunction-sections
> # test.cpp:
> #   void foo1() { }
> #   void foo2() { }
> ```
>
> And .debug_info section will be:
>
> ```
>         .section        .debug_info,"", at progbits
> .Lcu_begin0:
> ......
>         .long   .Linfo_string2          # DW_AT_comp_dir
>         .quad   0                       # DW_AT_low_pc
>         .long   .Ldebug_ranges0         # DW_AT_ranges
>
> ```
>
> So DW_AT_low_pc will present in CU, but be zero and have no relocation.
>
> Can we safely filter such cases out, like I did in my version of
> `setBaseAddress` above ?
> Not sure why DW_AT_low_pc is ever emited then, it looks useless.
>

Not necessarily - a value of zero is easy to ignore, but there could be
non-zero values (either relocatable or not - though it'd be hard to do a
non-relocatable non-zero base address).

DWARF says, in the presence of CU ranges, a low_pc can still be specified
as the base address. (so for example if you have a lot of functions in
.text, but one in .text.foo, so you need to use ranges - but you can still
specify low_pc as the first address in .text and benefit from base-relative
offsets for everything except the code in .text.foo)


>
> 2) Even with above change, there is still testcase that fails my new
> assertion `assert(BaseAddr.SectionIndex != -1ULL);`.
> It is `DebugInfo/X86/stmt-list-multiple-compile-units.ll`.
> It has `DW_AT_low_pc == 0x10` but still no corresponding relocation and so
> it can not find the section index.
>

Seems OK? If the code were at fixed/predefined offsets. What does this code
do when run on executables, where the section index isn't available because
the relocations have already been applied, for example? Why doesn't it
assert then as well?


>
> That happens because `DWARFObjInMemory` has following code that skips
> scanning relocations:
>
> ```
>      // In Mach-o files, the relocations do not need to be applied if
>       // there is no load offset to apply. The value read at the
>       // relocation point already factors in the section address
>       // (actually applying the relocations will produce wrong results
>       // as the section address will be added twice).
>       if (!L && isa<MachOObjectFile>(&Obj))
>         continue;
> ```
>
> So relocations are in the file, but we do not read them at all for MachO.
> And unable to get section index so far.
> I guess correct way would be to enable scanning and storing them, but
> still skip applying somehow.
> Does it make sence ?
>
> FWIW, I uploaded what I have here: D37214. It passes all tests now
> (assertion I mentioned is removed).
>
>
> https://reviews.llvm.org/D36097
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170829/3ff5042b/attachment.html>


More information about the llvm-commits mailing list