[PATCH] D84825: [release/11.x only][ELF] Change tombstone value -1 to 0

Wed Jul 29 12:38:18 PDT 2020

MaskRay added a subscriber: thakis.
MaskRay added a comment.

In D84825#2182896 <https://reviews.llvm.org/D84825#2182896>, @dblaikie wrote:

> In D84825#2182878 <https://reviews.llvm.org/D84825#2182878>, @MaskRay wrote:
>
>> In D84825#2182460 <https://reviews.llvm.org/D84825#2182460>, @dblaikie wrote:
>>
>>> Please implement a clear flag for
>>>
>>> - BFD semantics
>>> - New (-1/-2) semantics
>>>
>>> And default to the BFD semantics for now.
>>>
>>> The change in semantics is not ready to ship by default - there are lots of DWARF consumers out there and lots of people don't update them in lock-step with their compiler.
>>
>> I don't know what the request is. The patch implemented:
>>
>> - .debug_ranges & .debug_loc: -2
>> - .debug_*: 0  (GNU ld semantics)
>>
>> If you meant full BFD semantics:
>>
>> - .debug_ranges: 1
>> - .debug_*: 0
>
> Yes, that's what I meant. (though probably applying the debug_ranges workaround to debug_loc as well would be suitable but not necessary, I think)

Did you mean

  - * .debug_ranges & .debug_loc: -2
  + * .debug_ranges & .debug_loc: 1

?
I think this has gone too far a step. We really need more evidence that 1 can make DWARF consumers happier.

>> I also don't want to add another option like `--gnu-ld-debug-reloc-semantics` or similar.
>>
>> If you meant a shortcut to
>>
>> - -z 'dead-reloc-in-nonalloc=.debug_loc=0xfffffffffffffffe'
>> - -z 'dead-reloc-in-nonalloc=.debug_ranges=0xfffffffffffffffe'
>> - -z 'dead-reloc-in-nonalloc=.debug_*=0xffffffffffffffff'
>>
>> I don't that is necessary either. That option will be our compatibility burden. If would cause pain when we want to remove it in LLD 12 or 13.
>
> If we continue to support a broader/more configurable flag, I'm not sure how much support burden having a narrower flag would cause.
>
> Actually having the broad flag is concerning to me because it increases the support burden and surface area fo the linker and potentially creates far more diverse DWARF environment than existed before this change (when you could only get one of two semantics - bfds or gold/llds) making interoperability between DWARF consumers more difficult.

`-z dead-reloc-in-nonalloc=` is a more general option. As you see, its name does not even mention `debug` but the `.debug_*` use cases are just a subset. I have mentioned one use case for non-.debug* in D83264 <https://reviews.llvm.org/D83264>. I admit that I committed D83264 <https://reviews.llvm.org/D83264> a bit too quickly, because I was receiving pressure from @thakis who urgently needed the option. I would usually wait longer for @grimar and @psmisth to comment (@psmith had expressed ok in his first comment but @grimar hadn't) Hope they are still happy with the option)

About more diverse DWARF environment: I think the documentation can be improved to mention that fiddling with this needs to be very careful.  The debug info, admitted important, but the option don't contribute too much to the already very wide spectrum we have to support: many linker options can already affected the linked image in a complicate interaction users should be very careful: --dynamic-list --export-dynamic-symbol --no-dynamic-linker --sysroot --version-script --no-rosegment -z separate-code --image-base.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84825