[PATCH] D59553: [LLD][ELF][DebugInfo] llvm-symbolizer shows incorrect source line info if --gc-sections used

Mon May 6 21:49:45 PDT 2019

MaskRay added a comment.

> Could you explicitly describe that case, please ? i.e. When using UINT64_MAX-1 as start of address range for relocations pointing to deleted code would not be good for others ?

You are the one who propose to change the status quo and use UINT64_MAX-1. The burden of proof is on your side. You have the obligation to collect evidence from approaches taken by various DWARF consumers to prove the newly introduced special value wouldn't cause problems. However, I'd be happy to dig up history of various debuggers to learn what they do when I am free.

I also want to make it clear: your code changes the general behavior that applies to any non-allocated section, not something as specific as how a `DW_AT_low_pc` in a `.debug_info` is resolved. `.rela.debug_*` is a case of relocations into non-allocated sections but it isn't the only case. Do we have other such sections that may be thwarted (say -2 might be harmful to them) by your patch?

> llvm-symbolizer and addr2line just do not have enough information to properly select correct address range.

If other DWARF consumers (debuggers, dtrace, profilers, etc) are happy with 0 `DW_AT_low_pc`, the problem is probably on the symbolizers' side. And I have sufficient argument that addr2line doesn't do well and I am about to file a bug.

> taking into account that :
> 
> ld sets default image base to 0x400000 for x86_64.
>  gold default image base is 0x400000 for x86_64.
>  lld default image base is 0x200000 for x86_64
> 
> 1. size of sections put before .text section by lld should be greater than  0x200000 to have a benefit comparing with other linkers. In case size of sections would be less than 0x200000 then lld would have a problem but other linkers would not.
> 2. Actually it is not correct to link size of .text section and size of previously put sections as in above assumption. Size of text sections should be compared with size of deleted code. if there were deleted big piece of data then broken range could be big - 0x0-0x300000 f.e; But correct code could be smaller(size of text section is 1000 f.e) -  in this case there would be overlapping address ranges in-spite of the fact that lld put some sections before .text section.

You arguments here make me think we are not on the same page. The image base is used only by non-PIE executable, neither -pie nor -shared. The .debug_info input section associated with a discarded text section should have its `DW_AT_high_pc` larger than the `DW_AT_low_pc` of a good compilation unit to cause collision, not ` less than 0x200000` as you said.

> size of previously put sections

Actually they matter a lot. lld places the PF_R PT_LOAD segment before PF_R|PF_X PT_LOAD (text segment). The text segment contains compilation units. If the `PF_R` is sufficiently large, it will make the relative address of the text segment larger, thus unlikely to collide.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59553/new/

https://reviews.llvm.org/D59553