[PATCH] D120045: [lld][ELF] support nested special directives in output sections

[Luca Boccassi via Phabricator via llvm-commits]

bluca added a comment.

In D120045#3330944 <https://reviews.llvm.org/D120045#3330944>, @MaskRay wrote:

> In D120045#3330924 <https://reviews.llvm.org/D120045#3330924>, @bluca wrote:
>
>> In D120045#3330921 <https://reviews.llvm.org/D120045#3330921>, @MaskRay wrote:
>>
>>> In D120045#3330873 <https://reviews.llvm.org/D120045#3330873>, @bluca wrote:
>>>
>>>> In D120045#3330562 <https://reviews.llvm.org/D120045#3330562>, @MaskRay wrote:
>>>>
>>>>> I added the `(TYPE=...)` syntax for the sole purpose that Fedora or systemd (https://github.com/systemd/package-notes) has a proper way to mark `.note*` sections `SHT_NOTE`.
>>>>
>>>> And unless this fix or something along those lines is included too, it will not be used and it will serve no purpose whatsoever. Note how this patch doesn't mention _any specific directive_ but it simply skips any unknown ones. See it as future proofing, and compatibility with the documented bfd format. It's not going to get any closer to what you want - take it or leave it.
>>>
>>> Some keywords have been supported. A new keyword likely introduces an interesting enough semantic difference (e.g. changing sh_flags, sh_link, etc) we should not silently ignore.
>>> Ignore the unknown keyword like `FOOBAR` may cause a silently broken binary.
>>> Catching the error is more important than the compatibility with older linkers.
>>>
>>> In addition, the `(type)` syntax is actually ambiguous: it is overloaded with the output section address syntax. Ignoring the keyword will cause trouble if the user intends to use a non-keyword variable in a pair of parentheses for the address.
>>
>> How would something that is not supported and was never supported before produce a broken binary? Anyway, I don't really care _how_ you fix it, feel free to come up with a different solution to fix the same incompatibility if you don't like this one
>
> For example, if we introduce new syntax to change `sh_flags`.

There is no such thing though, so that sounds like an excuse to me. But anyway, feel free to come up with a different way to fix this incompatibility in llvm, I don't mind how it looks like or who implements it. Or not, and let llvm be incompatible with the default setup. Regardless, please stop wasting my time with proposals that do not work and that nobody asked you to provide.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120045