[PATCH] D33046: [ELF] - Set DF_TEXTREL flag properly.

[Davide Italiano via Phabricator via llvm-commits]

davide added a comment.

In https://reviews.llvm.org/D33046#751085, @grimar wrote:

> In https://reviews.llvm.org/D33046#751071, @davide wrote:
>
> > Side note: if everything works just fine as is (TM), why do we need to make this change?
> >  It buys us a new shiny flag that is pretty much equivalent to the old one for systems supporting both the `DT_` and the `DF_` variant, but it potentially breaks backcompatibility with older loaders not supporting the new form.
> >  I'm not entirely sure this risk is worth it.
>
>
> We implemented -z notext initially because had no way to produce relocations to readonly segment (here LLD differs in default behavior with bfd). That was https://reviews.llvm.org/D30530 and feature was intended to be used for linux kernel.
>  There was no way to link it at all that time. And DT_TEXTREL was added just as a part of a patch, but in fact kernel does not use the flags or anything, it do all things on its side and just
>  needs something like this option to complete link. Later, I think I saw in llvm-mails that there are other possible users of feature, not sure what they exactly need.
>
> And my point was that if there is no real users of depricated flag, why not to try to remove it ? That will be consistent with what spec says.
>  In LLD we usually did not implement things that are not known to be useful, I think when I implemented https://reviews.llvm.org/D30530 I should have use DF_ and not DT_ at first place.


If there's only one user, and that works with `DF_` then feel free to switch.
But:

1. Make sure you point out in the review next time so people don't have to guess.
2. Please double check there are no other reasonable uses of this flag before doing the switch.

(2) is less important, as we can always add the flag back).


https://reviews.llvm.org/D33046