Skip to content

[CMake] Add support for building on illumos #74930

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

Merged
merged 1 commit into from
Jan 9, 2024

Conversation

brad0
Copy link
Contributor

@brad0 brad0 commented Dec 9, 2023

illumos has an older version of the Solaris linker that does not
support the GNU version script compat nor version scripts and does
not support -Bsymbolic-functions. Treat illumos linker separately.

The libclang/CMakeLists part lifted from NetBSD's pkgsrc.

Build tested on Solaris 11.4 and OpenIndiana 2023.10.

/usr/bin/ld --version

ld: Software Generation Utilities - Solaris Link Editors: 5.11-1.3260

ld: Software Generation Utilities - Solaris Link Editors: 5.11-1.1790 (illumos)

@llvmbot llvmbot added cmake Build system in general and CMake in particular clang Clang issues not falling into any other category labels Dec 9, 2023
@llvmbot
Copy link
Member

llvmbot commented Dec 9, 2023

@llvm/pr-subscribers-clang

Author: Brad Smith (brad0)

Changes

illumos has an older version of the Solaris linker that does not
support the GNU version script compat nor version scripts and does
not support -Bsymbolic-functions. Treat illumos linker separately.

The libclang/CMakeLists part lifted from NetBSD's pkgsrc.

Build tested on Solaris 11.4 and OpenIndiana.

/usr/bin/ld --version

ld: Software Generation Utilities - Solaris Link Editors: 5.11-1.3260
ld: Software Generation Utilities - Solaris Link Editors: 5.11-1.1790 (illumos)


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

4 Files Affected:

  • (modified) clang/tools/clang-shlib/CMakeLists.txt (+1-1)
  • (modified) clang/tools/libclang/CMakeLists.txt (+15-4)
  • (modified) llvm/cmake/modules/AddLLVM.cmake (+10-3)
  • (modified) llvm/tools/llvm-shlib/CMakeLists.txt (+2-2)
diff --git a/clang/tools/clang-shlib/CMakeLists.txt b/clang/tools/clang-shlib/CMakeLists.txt
index aa7fcd1efed45b..298d3a9d18fec8 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)
+if (NOT APPLE AND NOT MINGW AND NOT LLVM_LINKER_IS_SOLARISLD_ILLUMOS)
   target_link_options(clang-cpp PRIVATE LINKER:-Bsymbolic-functions)
 endif()
 if (MINGW OR CYGWIN)
diff --git a/clang/tools/libclang/CMakeLists.txt b/clang/tools/libclang/CMakeLists.txt
index 4f23065a247274..1cfc46eb1a52f6 100644
--- a/clang/tools/libclang/CMakeLists.txt
+++ b/clang/tools/libclang/CMakeLists.txt
@@ -185,11 +185,22 @@ if(ENABLE_SHARED)
     endif()
   endif()
   if (USE_VERSION_SCRIPT)
-    target_link_options(libclang PRIVATE "-Wl,--version-script,${CMAKE_CURRENT_SOURCE_DIR}/libclang.map")
-    # The Solaris 11.4 linker supports a subset of GNU ld version scripts,
-    # but requires a special option to enable it.
     if (${CMAKE_SYSTEM_NAME} MATCHES "SunOS")
-      target_link_options(libclang PRIVATE "-Wl,-z,gnu-version-script-compat")
+      include(CheckLinkerFlag)
+      # The Solaris 11.4 linker supports a subset of GNU ld version scripts,
+      # but requires a special option to enable it.
+      llvm_check_linker_flag(CXX "-Wl,-z,gnu-version-script-compat"
+                             LINKER_SUPPORTS_Z_GNU_VERSION_SCRIPT_COMPAT)
+      # Older Solaris (and illumos) linker does not support GNU ld version scripts
+      # and does not support GNU version script compat.
+      if (LINKER_SUPPORTS_Z_GNU_VERSION_SCRIPT_COMPAT)
+        target_link_options(libclang PRIVATE "-Wl,--version-script,${CMAKE_CURRENT_SOURCE_DIR}/libclang.map")
+        target_link_options(libclang PRIVATE "-Wl,-z,gnu-version-script-compat")
+      else()
+        target_link_options(libclang PRIVATE "-Wl,-M,${CMAKE_CURRENT_SOURCE_DIR}/libclang.map")
+      endif()
+    else()
+      target_link_options(libclang PRIVATE "-Wl,--version-script,${CMAKE_CURRENT_SOURCE_DIR}/libclang.map")
     endif()
     # Ensure that libclang.so gets rebuilt when the linker script changes.
     set_property(SOURCE ARCMigrate.cpp APPEND PROPERTY
diff --git a/llvm/cmake/modules/AddLLVM.cmake b/llvm/cmake/modules/AddLLVM.cmake
index c9bca30c8f33d1..ca146ad699ff6c 100644
--- a/llvm/cmake/modules/AddLLVM.cmake
+++ b/llvm/cmake/modules/AddLLVM.cmake
@@ -115,7 +115,7 @@ function(add_llvm_symbol_exports target_name export_file)
       DEPENDS ${export_file}
       VERBATIM
       COMMENT "Creating export file for ${target_name}")
-    if (${LLVM_LINKER_IS_SOLARISLD})
+    if ((${LLVM_LINKER_IS_SOLARISLD}) OR (${LLVM_LINKER_IS_SOLARISLD_ILLUMOS}))
       set_property(TARGET ${target_name} APPEND_STRING PROPERTY
                    LINK_FLAGS "  -Wl,-M,\"${CMAKE_CURRENT_BINARY_DIR}/${native_export_file}\"")
     else()
@@ -241,6 +241,11 @@ 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_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 "")
@@ -282,11 +287,13 @@ function(add_link_opts target_name)
         # ld64's implementation of -dead_strip breaks tools that use plugins.
         set_property(TARGET ${target_name} APPEND_STRING PROPERTY
                      LINK_FLAGS " -Wl,-dead_strip")
-      elseif(${CMAKE_SYSTEM_NAME} MATCHES "SunOS" AND LLVM_LINKER_IS_SOLARISLD)
+      elseif(${CMAKE_SYSTEM_NAME} MATCHES "SunOS" AND (LLVM_LINKER_IS_SOLARISLD OR
+             LLVM_LINKER_IS_SOLARISLD_ILLUMOS))
         # 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)
+        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
                        LINK_FLAGS " -Wl,-z,discard-unused=sections")
diff --git a/llvm/tools/llvm-shlib/CMakeLists.txt b/llvm/tools/llvm-shlib/CMakeLists.txt
index 64d6f631ffadd5..119247582da28b 100644
--- a/llvm/tools/llvm-shlib/CMakeLists.txt
+++ b/llvm/tools/llvm-shlib/CMakeLists.txt
@@ -45,11 +45,11 @@ if(LLVM_BUILD_LLVM_DYLIB)
 
     # GNU ld doesn't resolve symbols in the version script.
     set(LIB_NAMES -Wl,--whole-archive ${LIB_NAMES} -Wl,--no-whole-archive)
-    if (NOT LLVM_LINKER_IS_SOLARISLD AND NOT MINGW)
+    if (NOT LLVM_LINKER_IS_SOLARISLD AND NOT LLVM_LINKER_IS_SOLARISLD_ILLUMOS AND NOT MINGW)
       # 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)
+    if (NOT MINGW AND NOT LLVM_LINKER_IS_SOLARISLD_ILLUMOS)
       # 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

@brad0 brad0 force-pushed the llvm_solaris_illumos branch from 5b41ec8 to 58f07d5 Compare December 9, 2023 12:34
@brad0 brad0 requested review from rorth and MaskRay December 9, 2023 12:36
@MaskRay
Copy link
Member

MaskRay commented Dec 11, 2023

I hope that someone using illumos can test, but if it is hard to find one I can approve this.

@brad0
Copy link
Contributor Author

brad0 commented Jan 4, 2024

Not so far. I'd like to get this in.

@MaskRay
Copy link
Member

MaskRay commented Jan 6, 2024

illumos has an older version of the Solaris linker that does not support the GNU version script compat nor version scripts and does not support -Bsymbolic-functions. Treat illumos linker separately.

Are they aware of the issue and is there a tracking feature request/issue? It seems that this patch adds code to hard code the implementation properties which may become stale.

@brad0 brad0 force-pushed the llvm_solaris_illumos branch from 58f07d5 to bc209c8 Compare January 6, 2024 08:18
@brad0 brad0 requested a review from MaskRay January 7, 2024 06:42
@MaskRay
Copy link
Member

MaskRay commented Jan 7, 2024

[CMake] may be a good tag.

@brad0
Copy link
Contributor Author

brad0 commented Jan 9, 2024

Are they aware of the issue and is there a tracking feature request/issue? It seems that this patch adds code to hard code the implementation properties which may become stale.

Searching via their bug tracker I can't find anything open or closed. But I have a hard time believing they are not aware of this.

We run into -Bsymbolic-functions on OpenBSD enough as it is with our BFD 2.17 linker.

Not sure about how frequently we run into version scripts in the wild.

@brad0 brad0 changed the title [llvm] Add support for building on illumos [CMake] Add support for building on illumos Jan 9, 2024
illumos has an older version of the Solaris linker that does not
support the GNU version script compat nor version scripts and does
not support -Bsymbolic-functions. Treat illumos linker separately.

The libclang/CMakeLists part lifted from NetBSD's pkgsrc.
@brad0 brad0 force-pushed the llvm_solaris_illumos branch from bc209c8 to b630092 Compare January 9, 2024 04:16
@brad0 brad0 merged commit 49c35f6 into llvm:main Jan 9, 2024
@brad0 brad0 deleted the llvm_solaris_illumos branch January 9, 2024 04:28
@rorth
Copy link
Collaborator

rorth commented Jan 18, 2024

I wonder what the current flurry of patdhes to support Illumos is about. I'd originally filed Issue #53919 to make clear what's needed from the Illumos project to keep it as an LLVM target, given that they haven't contributed either bug reports, code or testing in a long time. Without that, keeping Illumos somehow in a gray zone between supported and unsupported, this doubles my workload for Solaris patches:

  • Whenever I add or remove a feature, I need to investigate what needs to be done on the Illumos side.
  • Depending on the outcome, I need to add separate Illumos support code whenever it and Solaris differ.
  • And ultimately, I need to test all my patches not only on Solaris, but also no Illumos.

This could only be avoided if the Illumos guys (or someone else) reliably perform those tasks instead. However, I don't see any contributions from them, so the situation hasn't changed since the Issue was filed.

It also means that cleanup that could be done on the Solaris side (I'd like to rip out [sanitizer_common] Support Solaris < 11.4 in GetStaticTlsBoundary as soon as Solaris 11.3 support has been ripped out from GCC; 11.4 doesn't need it, but Illumos would AFAIK).

I'm perfectly happy to work with them to develop code that works on both variants (Illumos and Solaris), but I'm opposed to have untested/barely tested code in that creates massive work for me.

justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
illumos has an older version of the Solaris linker that does not
support the GNU version script compat nor version scripts and does
not support -Bsymbolic-functions. Treat illumos linker separately.

The libclang/CMakeLists part lifted from NetBSD's pkgsrc.

Build tested on Solaris 11.4 and OpenIndiana 2023.10.

/usr/bin/ld --version

ld: Software Generation Utilities - Solaris Link Editors: 5.11-1.3260

ld: Software Generation Utilities - Solaris Link Editors: 5.11-1.1790 (illumos)
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 platform:solaris
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants