This is an archive of the discontinued LLVM Phabricator instance.

[cmake] Append -Wl,-rpath-link conditionally to GNULD
ClosedPublic

Authored by mgorny on Feb 26 2018, 12:15 AM.

Details

Summary

Append -Wl,-rpath-link conditionally to whether GNU ld.bfd is used
rather than the Linux+!gold conditionals. Also move it out of 'else'
branch of *BSD handling. This fixes build failures with ld.bfd
on Gentoo/FreeBSD, and should cause no harm on other systems using
ld.bfd.

This patch improves the original logic by reusing results of linker
detection introduced in r307852.

Diff Detail

Repository
rL LLVM

Event Timeline

mgorny created this revision.Feb 26 2018, 12:15 AM
mgorny added a comment.Mar 2 2018, 1:46 AM

Ping. I'd like to have this reviewed before I roll 6.0.0 final for Gentoo.

emaste added a reviewer: dim.Mar 5 2018, 8:16 AM
emaste added a comment.Mar 5 2018, 8:23 AM

LGTM from the FreeBSD perspective

(Aside, it seems to me -Wl,-z,origin could be applied unconditionally, not only on FreeBSD/DragonFly).

LGTM from the FreeBSD perspective

(Aside, it seems to me -Wl,-z,origin could be applied unconditionally, not only on FreeBSD/DragonFly).

Uconditionally for all Unices? I don't think so.

emaste added a comment.Mar 7 2018, 4:12 PM

Uconditionally for all Unices? I don't think so.

Out of curiosity, what UNIX-like operating systems are supported by LLVM, and support $ORIGIN but not -z origin?

krytarowski added a comment.EditedMar 7 2018, 6:25 PM

ld(1) might accept -z origin, but why do we want it for other OSes? I don't see it used in the NetBSD dynamic ELF loader. DF_ORIGIN is just defined but not used by anything.

FreeBSD seems to be hypercorrect in terms of ELF handling, other OSes don't care that much.

emaste added a comment.Mar 7 2018, 8:17 PM

ld(1) might accept -z origin, but why do we want it for other OSes? I don't see it used in the NetBSD dynamic ELF loader. DF_ORIGIN is just defined but not used by anything.

It was largely an aside, just pointing out that we could eliminate an OS-specific special case.

FreeBSD seems to be hypercorrect in terms of ELF handling, other OSes don't care that much.

In fact setting -z origin here on (current) versions of FreeBSD serves no purpose, but it also doesn't hurt anything.

In fact setting -z origin here on (current) versions of FreeBSD serves no purpose, but it also doesn't hurt anything.

So another reason to stop accumulating garbage that is not just specific to one OS, but also obsolete.

emaste accepted this revision.Mar 8 2018, 6:20 AM

LGTM for FreeBSD

This revision is now accepted and ready to land.Mar 8 2018, 6:20 AM
mgorny added a comment.Mar 8 2018, 7:11 AM

Thanks for the review. As for the -Wl,-z,origin part, I'd rather leave that alone unless we have a clear indication that either it is not necessary for any of the supported FreeBSD and DragonFly BSD versions, or that it can be passed unconditionally without breaking any of the supported systems (and which linkers accept it).

This revision was automatically updated to reflect the committed changes.
emaste added a comment.Mar 9 2018, 2:23 PM

Thanks for the review. As for the -Wl,-z,origin part, I'd rather leave that alone unless we have a clear indication that either it is not necessary for any of the supported FreeBSD and DragonFly BSD versions, or that it can be passed unconditionally without breaking any of the supported systems (and which linkers accept it).

I checked and it seems FreeBSD 10 (still supported) needs it, 11.x and later does not.
Details are in the commit at https://reviews.freebsd.org/rS282109