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

[mgorny]

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.

[mgorny]

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

[emaste]

LGTM from the FreeBSD perspective

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

[krytarowski]

In D43751#1027234, @emaste wrote:

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]

In D43751#1027263, @krytarowski wrote:

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]

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]

In D43751#1030947, @krytarowski wrote:

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.

[krytarowski]

In D43751#1031012, @emaste wrote:

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]

LGTM for FreeBSD

[mgorny]

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).

[emaste]

In D43751#1031350, @mgorny wrote:

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

cmake/modules/AddLLVM.cmake

function(llvm_setup_rpath name)
   if (APPLE)
     set(_install_name_dir INSTALL_NAME_DIR "@rpath")
     set(_install_rpath "@loader_path/../lib" ${extra_libdir})
   elseif(UNIX)
     set(_install_rpath "\$ORIGIN/../lib${LLVM_LIBDIR_SUFFIX}" ${extra_libdir})
     if(${CMAKE_SYSTEM_NAME} MATCHES "(FreeBSD|DragonFly)")
       set_property(TARGET ${name} APPEND_STRING PROPERTY
                    LINK_FLAGS " -Wl,-z,origin ")
-    elseif(${CMAKE_SYSTEM_NAME} STREQUAL "Linux" AND NOT LLVM_LINKER_IS_GOLD)
+    endif()
+    if(LLVM_LINKER_IS_GNULD)
       # $ORIGIN is not interpreted at link time by ld.bfd
       set_property(TARGET ${name} APPEND_STRING PROPERTY
                    LINK_FLAGS " -Wl,-rpath-link,${LLVM_LIBRARY_OUTPUT_INTDIR} ")
     endif()
   else()
     return()
   endif()