Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CMake] Add a linker test for -Bsymbolic-functions to AddLLVM #79539

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

brad0
Copy link
Contributor

@brad0 brad0 commented Jan 26, 2024

Add a linker test for -Bsymbolic-functions to AddLLVM and remove
the illumos hardcoded bits for its handling. OpenBSD also has a
local patch for linking with the old BFD linker on mips64 and
sparc64.

@llvmbot llvmbot added cmake Build system in general and CMake in particular clang Clang issues not falling into any other category labels Jan 26, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 26, 2024

@llvm/pr-subscribers-clang

Author: Brad Smith (brad0)

Changes

Add a linker test for -Bsymbolic-functions to AddLLVM and remove
the illumos hardcoded bits for its handling. OpenBSD also has a
local patch for linking with the old BFD linker on mips64 and
sparc64.


Full diff: https://github.com/llvm/llvm-project/pull/79539.diff

3 Files Affected:

  • (modified) clang/tools/clang-shlib/CMakeLists.txt (+1-1)
  • (modified) llvm/cmake/modules/AddLLVM.cmake (+6-7)
  • (modified) llvm/tools/llvm-shlib/CMakeLists.txt (+1-1)
diff --git a/clang/tools/clang-shlib/CMakeLists.txt b/clang/tools/clang-shlib/CMakeLists.txt
index 298d3a9d18fec8c..6afd3efebe53a0b 100644
--- a/clang/tools/clang-shlib/CMakeLists.txt
+++ b/clang/tools/clang-shlib/CMakeLists.txt
@@ -50,7 +50,7 @@ add_clang_library(clang-cpp
                   ${_DEPS})
 # Optimize function calls for default visibility definitions to avoid PLT and
 # reduce dynamic relocations.
-if (NOT APPLE AND NOT MINGW AND NOT LLVM_LINKER_IS_SOLARISLD_ILLUMOS)
+if (NOT APPLE AND LINKER_SUPPORTS_B_SYMBOLIC_FUNCTIONS)
   target_link_options(clang-cpp PRIVATE LINKER:-Bsymbolic-functions)
 endif()
 if (MINGW OR CYGWIN)
diff --git a/llvm/cmake/modules/AddLLVM.cmake b/llvm/cmake/modules/AddLLVM.cmake
index 5e9896185528246..a030489b6196023 100644
--- a/llvm/cmake/modules/AddLLVM.cmake
+++ b/llvm/cmake/modules/AddLLVM.cmake
@@ -241,12 +241,6 @@ if (NOT DEFINED LLVM_LINKER_DETECTED AND NOT WIN32)
       set(LLVM_LINKER_DETECTED YES CACHE INTERNAL "")
       set(LLVM_LINKER_IS_GNULD YES CACHE INTERNAL "")
       message(STATUS "Linker detection: GNU ld")
-    elseif("${stderr}" MATCHES "(illumos)" OR
-           "${stdout}" MATCHES "(illumos)")
-      set(LLVM_LINKER_DETECTED YES CACHE INTERNAL "")
-      set(LLVM_LINKER_IS_SOLARISLD YES CACHE INTERNAL "")
-      set(LLVM_LINKER_IS_SOLARISLD_ILLUMOS YES CACHE INTERNAL "")
-      message(STATUS "Linker detection: Solaris ld (illumos)")
     elseif("${stderr}" MATCHES "Solaris Link Editors" OR
            "${stdout}" MATCHES "Solaris Link Editors")
       set(LLVM_LINKER_DETECTED YES CACHE INTERNAL "")
@@ -260,6 +254,7 @@ if (NOT DEFINED LLVM_LINKER_DETECTED AND NOT WIN32)
 endif()
 
 function(add_link_opts target_name)
+  include(LLVMCheckLinkerFlag)
   get_llvm_distribution(${target_name} in_distribution in_distribution_var)
   if(NOT in_distribution)
     # Don't LTO optimize targets that aren't part of any distribution.
@@ -291,7 +286,6 @@ function(add_link_opts target_name)
       elseif(${CMAKE_SYSTEM_NAME} MATCHES "SunOS" AND LLVM_LINKER_IS_SOLARISLD)
         # Support for ld -z discard-unused=sections was only added in
         # Solaris 11.4.  GNU ld ignores it, but warns every time.
-        include(LLVMCheckLinkerFlag)
         llvm_check_linker_flag(CXX "-Wl,-z,discard-unused=sections" LINKER_SUPPORTS_Z_DISCARD_UNUSED)
         if (LINKER_SUPPORTS_Z_DISCARD_UNUSED)
           set_property(TARGET ${target_name} APPEND_STRING PROPERTY
@@ -314,6 +308,11 @@ function(add_link_opts target_name)
     set_property(TARGET ${target_name} APPEND_STRING PROPERTY
                  LINK_FLAGS " -Wl,-brtl")
   endif()
+
+  if (NOT MINGW)
+    llvm_check_linker_flag(CXX "-Wl,-Bsymbolic-functions"
+                           LINKER_SUPPORTS_B_SYMBOLIC_FUNCTIONS)
+  endif()
 endfunction(add_link_opts)
 
 # Set each output directory according to ${CMAKE_CONFIGURATION_TYPES}.
diff --git a/llvm/tools/llvm-shlib/CMakeLists.txt b/llvm/tools/llvm-shlib/CMakeLists.txt
index a47a0ec84c625c6..3e08dd5459a3f1a 100644
--- a/llvm/tools/llvm-shlib/CMakeLists.txt
+++ b/llvm/tools/llvm-shlib/CMakeLists.txt
@@ -49,7 +49,7 @@ if(LLVM_BUILD_LLVM_DYLIB)
       # Solaris ld does not accept global: *; so there is no way to version *all* global symbols
       set(LIB_NAMES -Wl,--version-script,${LLVM_LIBRARY_DIR}/tools/llvm-shlib/simple_version_script.map ${LIB_NAMES})
     endif()
-    if (NOT MINGW AND NOT LLVM_LINKER_IS_SOLARISLD_ILLUMOS)
+    if (LINKER_SUPPORTS_B_SYMBOLIC_FUNCTIONS)
       # Optimize function calls for default visibility definitions to avoid PLT and
       # reduce dynamic relocations.
       # Note: for -fno-pic default, the address of a function may be different from

Add a linker test for -Bsymbolic-functions to AddLLVM and remove
the illumos hardcoded bits for its handling. OpenBSD also has a
local patch for linking with the old BFD linker on mips64 and
sparc64.
@MaskRay
Copy link
Member

MaskRay commented Jan 30, 2024

OpenBSD also has a local patch for linking with the old BFD linker on mips64 and sparc64.

What's the issue? GNU ld has had -Bsymbolic-functions since 2007 and for quite a few releases LLVM has been using -Bsymbolic-functions.

@brad0
Copy link
Contributor Author

brad0 commented Feb 19, 2024

What's the issue? GNU ld has had -Bsymbolic-functions since 2007 and for quite a few releases LLVM has been using -Bsymbolic-functions.

This is binutils 2.17. We've had patches for awhile in our tree too specifically for the exceptions not using LLD.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang Clang issues not falling into any other category cmake Build system in general and CMake in particular
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants