Skip to content

Get the linker version and pass the it to compiler-rt tests on Darwin. #86220

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
Mar 22, 2024

Conversation

usama54321
Copy link
Member

The HOST_LINK_VERSION is a hardcoded string in Darwin clang that detects the linker version at configure time. The driver uses this information to build the correct set of arguments for the linker. This patch detects the linker version again during compiler-rt configuration and passes it to the tests. This allows a clang built on a machine with a new linker to run compiler-rt tests on a machine with an old linker.

rdar://125198603

@llvmbot llvmbot added cmake Build system in general and CMake in particular clang Clang issues not falling into any other category compiler-rt labels Mar 21, 2024
@llvmbot
Copy link
Member

llvmbot commented Mar 21, 2024

@llvm/pr-subscribers-clang

Author: Usama Hameed (usama54321)

Changes

The HOST_LINK_VERSION is a hardcoded string in Darwin clang that detects the linker version at configure time. The driver uses this information to build the correct set of arguments for the linker. This patch detects the linker version again during compiler-rt configuration and passes it to the tests. This allows a clang built on a machine with a new linker to run compiler-rt tests on a machine with an old linker.

rdar://125198603


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

5 Files Affected:

  • (modified) clang/CMakeLists.txt (+2-14)
  • (added) cmake/Modules/GetDarwinLinkerVersion.cmake (+19)
  • (modified) compiler-rt/CMakeLists.txt (+10)
  • (modified) compiler-rt/test/lit.common.cfg.py (+4)
  • (modified) compiler-rt/test/lit.common.configured.in (+1)
diff --git a/clang/CMakeLists.txt b/clang/CMakeLists.txt
index 761dab8c28c134..bcafbc37fbe1fb 100644
--- a/clang/CMakeLists.txt
+++ b/clang/CMakeLists.txt
@@ -15,6 +15,7 @@ endif()
 
 # Must go below project(..)
 include(GNUInstallDirs)
+include(GetDarwinLinkerVersion)
 
 if(CLANG_BUILT_STANDALONE)
   set(CMAKE_CXX_STANDARD 17 CACHE STRING "C++ standard to conform to")
@@ -346,20 +347,7 @@ endif ()
 # Determine HOST_LINK_VERSION on Darwin.
 set(HOST_LINK_VERSION)
 if (APPLE AND NOT CMAKE_LINKER MATCHES ".*lld.*")
-  set(LD_V_OUTPUT)
-  execute_process(
-    COMMAND sh -c "${CMAKE_LINKER} -v 2>&1 | head -1"
-    RESULT_VARIABLE HAD_ERROR
-    OUTPUT_VARIABLE LD_V_OUTPUT
-  )
-  if (HAD_ERROR)
-    message(FATAL_ERROR "${CMAKE_LINKER} failed with status ${HAD_ERROR}")
-  endif()
-  if ("${LD_V_OUTPUT}" MATCHES ".*ld64-([0-9.]+).*")
-    string(REGEX REPLACE ".*ld64-([0-9.]+).*" "\\1" HOST_LINK_VERSION ${LD_V_OUTPUT})
-  elseif ("${LD_V_OUTPUT}" MATCHES "[^0-9]*([0-9.]+).*")
-    string(REGEX REPLACE "[^0-9]*([0-9.]+).*" "\\1" HOST_LINK_VERSION ${LD_V_OUTPUT})
-  endif()
+  get_darwin_linker_version()
   message(STATUS "Host linker version: ${HOST_LINK_VERSION}")
 endif()
 
diff --git a/cmake/Modules/GetDarwinLinkerVersion.cmake b/cmake/Modules/GetDarwinLinkerVersion.cmake
new file mode 100644
index 00000000000000..b6a10fdc173677
--- /dev/null
+++ b/cmake/Modules/GetDarwinLinkerVersion.cmake
@@ -0,0 +1,19 @@
+# Get the linker version on Darwin
+function(get_darwin_linker_version)
+  set(LINK_VERSION)
+  set(LD_V_OUTPUT)
+  execute_process(
+    COMMAND sh -c "${CMAKE_LINKER} -v 2>&1 | head -1"
+    RESULT_VARIABLE HAD_ERROR
+    OUTPUT_VARIABLE LD_V_OUTPUT
+    )
+  if (HAD_ERROR)
+    message(FATAL_ERROR "${CMAKE_LINKER} failed with status ${HAD_ERROR}")
+  endif()
+  if ("${LD_V_OUTPUT}" MATCHES ".*ld64-([0-9.]+).*")
+    string(REGEX REPLACE ".*ld64-([0-9.]+).*" "\\1" LINK_VERSION ${LD_V_OUTPUT})
+  elseif ("${LD_V_OUTPUT}" MATCHES "[^0-9]*([0-9.]+).*")
+    string(REGEX REPLACE "[^0-9]*([0-9.]+).*" "\\1" LINK_VERSION ${LD_V_OUTPUT})
+  endif()
+  set(HOST_LINK_VERSION ${LINK_VERSION} PARENT_SCOPE)
+endfunction()
diff --git a/compiler-rt/CMakeLists.txt b/compiler-rt/CMakeLists.txt
index d562a6206c00e7..9fb15403b933dc 100644
--- a/compiler-rt/CMakeLists.txt
+++ b/compiler-rt/CMakeLists.txt
@@ -36,6 +36,7 @@ include(SetPlatformToolchainTools)
 include(base-config-ix)
 include(CompilerRTUtils)
 include(CMakeDependentOption)
+include(GetDarwinLinkerVersion)
 
 option(COMPILER_RT_BUILD_BUILTINS "Build builtins" ON)
 mark_as_advanced(COMPILER_RT_BUILD_BUILTINS)
@@ -444,6 +445,15 @@ else()
   set(SANITIZER_USE_SYMBOLS FALSE)
 endif()
 
+# Get the linker version while configuring compiler-rt and explicitly pass it
+# in cflags during testing. This fixes the compiler/linker version mismatch
+# issue when running a clang built with a newer Xcode in an older Xcode
+set(HOST_LINK_VERSION)
+if (APPLE AND NOT CMAKE_LINKER MATCHES ".*lld.*")
+  get_darwin_linker_version()
+  message(STATUS "Host linker version: ${HOST_LINK_VERSION}")
+endif()
+
 # Build sanitizer runtimes with debug info.
 if(MSVC)
   # Use /Z7 instead of /Zi for the asan runtime. This avoids the LNK4099
diff --git a/compiler-rt/test/lit.common.cfg.py b/compiler-rt/test/lit.common.cfg.py
index bd9b926c1505b1..cc59a794e11352 100644
--- a/compiler-rt/test/lit.common.cfg.py
+++ b/compiler-rt/test/lit.common.cfg.py
@@ -882,6 +882,10 @@ def is_windows_lto_supported():
 elif config.use_lld and (not config.has_lld):
     config.unsupported = True
 
+if config.host_os == "Darwin":
+    if getattr(config, "host_link_version", None):
+        extra_cflags += ["-mlinker-version=" + config.host_link_version]
+
 # Append any extra flags passed in lit_config
 append_target_cflags = lit_config.params.get("append_target_cflags", None)
 if append_target_cflags:
diff --git a/compiler-rt/test/lit.common.configured.in b/compiler-rt/test/lit.common.configured.in
index db5d7c598b7310..28c0f02a03976c 100644
--- a/compiler-rt/test/lit.common.configured.in
+++ b/compiler-rt/test/lit.common.configured.in
@@ -51,6 +51,7 @@ set_default("expensive_checks", @LLVM_ENABLE_EXPENSIVE_CHECKS_PYBOOL@)
 set_default("test_standalone_build_libs", @COMPILER_RT_TEST_STANDALONE_BUILD_LIBS_PYBOOL@)
 set_default("has_compiler_rt_libatomic", @COMPILER_RT_BUILD_STANDALONE_LIBATOMIC_PYBOOL@)
 set_default("aarch64_sme", @COMPILER_RT_HAS_AARCH64_SME_PYBOOL@)
+set_default("host_link_version", "@HOST_LINK_VERSION@")
 # True iff the test suite supports ignoring the test compiler's runtime library path
 # and using `config.compiler_rt_libdir` instead. This only matters when the runtime
 # library paths differ.

Copy link
Contributor

@thetruestblue thetruestblue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for looking into this.

@wrotki
Copy link
Contributor

wrotki commented Mar 22, 2024

Yes, it makes sense

Copy link
Collaborator

@yln yln left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, approved with a few nits!

@@ -15,6 +15,7 @@ endif()

# Must go below project(..)
include(GNUInstallDirs)
include(GetDarwinLinkerVersion)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for extracting this into a separate file! 👍

# Get the linker version while configuring compiler-rt and explicitly pass it
# in cflags during testing. This fixes the compiler/linker version mismatch
# issue when running a clang built with a newer Xcode in an older Xcode
set(HOST_LINK_VERSION)
Copy link
Collaborator

@yln yln Mar 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • How about DARWIN_LINKER_VERSION?

@@ -51,6 +51,7 @@ set_default("expensive_checks", @LLVM_ENABLE_EXPENSIVE_CHECKS_PYBOOL@)
set_default("test_standalone_build_libs", @COMPILER_RT_TEST_STANDALONE_BUILD_LIBS_PYBOOL@)
set_default("has_compiler_rt_libatomic", @COMPILER_RT_BUILD_STANDALONE_LIBATOMIC_PYBOOL@)
set_default("aarch64_sme", @COMPILER_RT_HAS_AARCH64_SME_PYBOOL@)
set_default("host_link_version", "@HOST_LINK_VERSION@")
Copy link
Collaborator

@yln yln Mar 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Many of the other vars here have a COMPILER_RT prefix. Should we do the same or does it imply some additional semantics ("user option" in CMAKE script)?
  • set_default("darwin_linker_version", "@COMPILER_RT_DARWIN_LINKER_VERSION@")

elseif ("${LD_V_OUTPUT}" MATCHES "[^0-9]*([0-9.]+).*")
string(REGEX REPLACE "[^0-9]*([0-9.]+).*" "\\1" HOST_LINK_VERSION ${LD_V_OUTPUT})
endif()
get_darwin_linker_version()
Copy link
Collaborator

@yln yln Mar 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Would it be possible to "declare" where the value is stored, i.e., a CMAKE output parameter? So the relationship between HOST_LINK_VERSION and get_darwin_linker_version() is explicit (and other variable names can be used)?
Suggested change
get_darwin_linker_version()
get_darwin_linker_version(HOST_LINK_VERSION)

The HOST_LINK_VERSION is a hardcoded string in Darwin clang that detects
the linker version at configure time. The driver uses this information
to build the correct set of arguments for the linker. This patch detects
the linker version again during compiler-rt configuration and passes it
to the tests. This allows a clang built on a machine with a new linker
to run compiler-rt tests on a machine with an old linker.

rdar://125198603
@usama54321 usama54321 force-pushed the linker-version-compiler-rt-tests branch from 3148a5e to 0dab57b Compare March 22, 2024 22:16
Copy link

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link

✅ With the latest revision this PR passed the Python code formatter.

@usama54321 usama54321 merged commit 3bc71c2 into llvm:main Mar 22, 2024
usama54321 added a commit to usama54321/apple-llvm-project that referenced this pull request Apr 4, 2024
llvm#86220)

The HOST_LINK_VERSION is a hardcoded string in Darwin clang that detects
the linker version at configure time. The driver uses this information
to build the correct set of arguments for the linker. This patch detects
the linker version again during compiler-rt configuration and passes it
to the tests. This allows a clang built on a machine with a new linker
to run compiler-rt tests on a machine with an old linker.

rdar://125198603
usama54321 added a commit to swiftlang/llvm-project that referenced this pull request Apr 5, 2024
llvm#86220)

The HOST_LINK_VERSION is a hardcoded string in Darwin clang that detects
the linker version at configure time. The driver uses this information
to build the correct set of arguments for the linker. This patch detects
the linker version again during compiler-rt configuration and passes it
to the tests. This allows a clang built on a machine with a new linker
to run compiler-rt tests on a machine with an old linker.

rdar://125198603
usama54321 added a commit to usama54321/apple-llvm-project that referenced this pull request Apr 30, 2024
llvm#86220)

The HOST_LINK_VERSION is a hardcoded string in Darwin clang that detects
the linker version at configure time. The driver uses this information
to build the correct set of arguments for the linker. This patch detects
the linker version again during compiler-rt configuration and passes it
to the tests. This allows a clang built on a machine with a new linker
to run compiler-rt tests on a machine with an old linker.

rdar://125198603
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 compiler-rt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants