[PATCH] D40558: [ELF] - Trigger error when -R <filename> is given.

Roland McGrath via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 30 15:17:47 PST 2017


The failure mode for treating -R as --rpath when the user wanted
--just-symbols is arcane and confusing, but it is not silent.  So I don't
think there's great risk in leaving -R as it is.
Personally, I think it would be fine for -R to just be an error telling you
to pick --rpath or --just-symbols instead.  A conservative alternative
would be to make it a warning, but continue treating it as --rpath.

On Thu, Nov 30, 2017 at 12:44 PM, Rui Ueyama <ruiu at google.com> wrote:

> A chance to write a new general-purpose linker from scratch happens only
> once in a decade or so, and my wish is to not carry over old hairy command
> line options whenever possible. As a practical linker, we sometimes have to
> implement hairy features, but that doesn't mean we want to copy everything.
> I do not see a strong feature request from many people to copy the GNU's -R
> option yet. Once you add the feature, it is irreversible. I think you
> shouldn't copy the feature at least now.
>
> Dropping the -R option is tempting, but since we had this option as an
> alias to -rpath in the older releases in lld, we should be conservative. I
> don't think it is a good idea to change the existing behavior without a
> strong reason. I'm not so sure that -R is not in use in the wild.
>
> Also, can you please do not implement a feature without discussing whether
> we want it or not in the first place? "Because we already have a patch and
> it's simple" shouldn't be a reason to add a new feature when something else
> matters more than that.
>
> On Thu, Nov 30, 2017 at 12:49 AM, George Rimar <grimar at accesssoftek.com>
> wrote:
>
>> I think I agree that we should not make assumptions on runtime
>> environment (so I would not touch current -rpath behavior).
>>
>> Though my position here is based on safety mostly. It is definetely safe
>> to leave current behavior which is consistent with GNU linkers
>>
>> and is very simple (as does not require checking anything).
>>
>>
>> About -R I really do not know is it common or not (I thought not because
>> we have no testcases for -R),
>>
>> but assuming it is common and taking the fact it shares behavior
>>
>> and LLD produces broken output silently, what about emiting warning like:
>>
>> "-R (-rpath): option is depricated/unsupported. Consider using  -rpath
>> or-just-symbols instead."
>>
>> if -R was used ?
>>
>>
>> We are in good position to change behavior of badly designed things
>> probably. And can do that in a hard or soft way,
>>
>> warning is probably soft safe way. (And I believe it -R can be leaved as
>> alias with this approach).
>>
>>
>> Best regards,
>> George | Developer | Access Softek, Inc
>> ------------------------------
>> *От:* Rui Ueyama <ruiu at google.com>
>> *Отправлено:* 30 ноября 2017 г. 4:30
>> *Кому:* Rafael Avila de Espindola
>> *Копия:* George Rimar; reviews+D40558+public+eb8f52c2
>> c2ffee2b at reviews.llvm.org; Roland McGrath; Evgeny Leviant; llvm-commits;
>> Ed Maste
>> *Тема:* Re: [PATCH] D40558: [ELF] - Trigger error when -R <filename> is
>> given.
>>
>> On Wed, Nov 29, 2017 at 8:59 AM, Rafael Avila de Espindola <
>> rafael.espindola at gmail.com> wrote:
>>
>>> George Rimar <grimar at accesssoftek.com> writes:
>>>
>>> >>I think we shouldn't even emit an error for the second case. -rpath is
>>> the information for the dynamic linker, and the static linker should
>>> blindly store given -rpath >paths to a file header, because the static
>>> linker doesn't know the actual runtime environment. It is a valid use case
>>> that a given -rpath is not a directory in a build >environment but a
>>> directory in a runtime environment.
>>> >>
>>> >>Given that, I'm inclined to not to emit an error or a warning at all
>>> for -R and just make it an alias to -rpath. If you are so familiar with
>>> linker features that you want to >use --just-symbols, you are likely to
>>> know -R, -rpath and -just-symbols. -just-symbols is used very rarely, so in
>>> reality, I think this is not an issue.
>>> >
>>> > Two points from me:
>>> > "Given that, I'm inclined to not to emit an error or a warning at all
>>> for -R and just make it an alias to -rpath. If you are so familiar with
>>> linker features that you want to use --just-symbols, you are likely to know
>>> -R"
>>> >
>>> > Last part means that we should implement -R both for -rpath and
>>> --just-symbols, like original GNU linkers do. As it is what is expected by
>>> user, who familar with both.
>>> > Currently our behavior always behaves as -rpath without warnings or
>>> errors and produce broken output. And that nehavior is definetely not
>>> expected (and comments for PR35067 shows that).
>>> >
>>> > And then making -R an alias for one of these options is just not
>>> correct. If it's intention is to share 2 different behaviors it should be
>>> separate option with corresponding description, no ?
>>> >
>>> > ?Currently no our tests use -R for -rpath. What about if we drop
>>> supporting -R completely ? We can show a error suggesting to use explicitly
>>> use -rpath/--just-symbols instead.
>>>
>>> I *think* that using -R for --rpath is fairly common.
>>>
>>> I think I am back to favoring -R being an alias for rpath and producing
>>> an error for "--rpath file" or "-R file". Yes, the build machine might
>>> have a file and the intended run environment a directory, but that seems
>>> unlikely and easy to avoid.
>>>
>>
>> I'm not in favor of that. Reasons being that (1) assuming that a runtime
>> environment is the same as a build environment is generally a bad idea even
>> though it can be easily avoided, (2) --just-symbols is used very rarely,
>> and (3) the -R option's behavior is undeniably a bit too hacky. Now seems
>> like a good time to kill that tricky behavior.
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171130/36585eb6/attachment.html>


More information about the llvm-commits mailing list