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

Tue May 7 01:37:30 PDT 2019

avl added a comment.

> 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

Sure. I though you have in mind something concrete.

I checked behavior of addr2line and gdb -they are working correctly with this patch.

Could you tell which check would be good from your opinion to do more, please ?

> 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?

Originally that patch was limited to only .debug* sections. But it was requested to extend it to all non-allocated section.
https://reviews.llvm.org/D59553#1445935
Since there is no requirement that certain sections should be assigned to 0 then I agree it should not be a problem to assign any other suitable value for them.

> 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.

actually other DWARF consumers  are not happy. They implemented various heuristics.
Instead of implementing various, potentially not matching, heuristics it is possible to just avoid address ranges clash.

Could you confirm that solution suggested in D60470 <https://reviews.llvm.org/D60470> could not handle following situation correctly ?

One piece of debug info references removed address range DW_AT_low_pc=0x0 DW_AT_high_pc=0x10000
Another piece of debug info references correct address range DW_AT_low_pc=0x0 DW_AT_high_pc=0x10000

> 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.

One piece of debug info references removed address range DW_AT_low_pc=0x0 DW_AT_high_pc=0x350000
Another piece of debug info references correct address range DW_AT_low_pc=0x300000 DW_AT_high_pc=0x390000

in this case DW_AT_high_pc of discarded text section larger than the DW_AT_low_pc of a good compilation unit.
lld put tables which size is 0x100000 before .text section so DW_AT_low_pc=0x300000.
lld generated executable would have a problem. ld and gold generated would not.

> 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.

they matter if they are significantly large - otherwise they are not.

One piece of debug info references removed address range DW_AT_low_pc=0x0 DW_AT_high_pc=0x280000
Another piece of debug info references correct address range DW_AT_low_pc=0x202000 DW_AT_high_pc=204000

in this case DW_AT_high_pc of discarded text section larger than the DW_AT_low_pc of a good compilation unit.
lld put tables which size is 0x2000 before .text section so DW_AT_low_pc=202000.
lld generated executable would have a problem. ld and gold generated would not.

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

https://reviews.llvm.org/D59553