release/18.x: [runtimes] Prefer -fvisibility-global-new-delete=force-hidden (#84917) #85126
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@ldionne What do you think about merging this PR to the release branch? |
llvmbot
added
libc++
|
@llvm/pr-subscribers-libcxxabi @llvm/pr-subscribers-libcxx Author: None (llvmbot) ChangesBackport 1f973ef Requested by: @ldionne Full diff: https://github.com/llvm/llvm-project/pull/85126.diff 3 Files Affected:
diff --git a/libcxx/src/CMakeLists.txt b/libcxx/src/CMakeLists.txt
index 44a088663463c9..1b80625304a412 100644
--- a/libcxx/src/CMakeLists.txt
+++ b/libcxx/src/CMakeLists.txt
@@ -306,7 +306,10 @@ if (LIBCXX_ENABLE_STATIC)
# then its code shouldn't declare them with hidden visibility. They might
# actually be provided by a shared library at link time.
if (LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS)
- append_flags_if_supported(CXX_STATIC_LIBRARY_FLAGS -fvisibility-global-new-delete-hidden)
+ append_flags_if_supported(CXX_STATIC_LIBRARY_FLAGS -fvisibility-global-new-delete=force-hidden)
+ if (NOT CXX_SUPPORTS_FVISIBILITY_GLOBAL_NEW_DELETE_EQ_FORCE_HIDDEN_FLAG)
+ append_flags_if_supported(CXX_STATIC_LIBRARY_FLAGS -fvisibility-global-new-delete-hidden)
+ endif()
endif()
target_compile_options(cxx_static PRIVATE ${CXX_STATIC_LIBRARY_FLAGS})
# _LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS can be defined in __config_site
diff --git a/libcxxabi/src/CMakeLists.txt b/libcxxabi/src/CMakeLists.txt
index 4198827203fc8b..f4722c3b352d4d 100644
--- a/libcxxabi/src/CMakeLists.txt
+++ b/libcxxabi/src/CMakeLists.txt
@@ -265,7 +265,10 @@ if(LIBCXXABI_HERMETIC_STATIC_LIBRARY)
# then its code shouldn't declare them with hidden visibility. They might
# actually be provided by a shared library at link time.
if (LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS)
- target_add_compile_flags_if_supported(cxxabi_static_objects PRIVATE -fvisibility-global-new-delete-hidden)
+ target_add_compile_flags_if_supported(cxxabi_static_objects PRIVATE -fvisibility-global-new-delete=force-hidden)
+ if (NOT CXX_SUPPORTS_FVISIBILITY_GLOBAL_NEW_DELETE_EQ_FORCE_HIDDEN_FLAG)
+ target_add_compile_flags_if_supported(cxxabi_static_objects PRIVATE -fvisibility-global-new-delete-hidden)
+ endif()
endif()
# _LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS can be defined in libcxx's
# __config_site too. Define it in the same way here, to avoid redefinition
diff --git a/libunwind/src/CMakeLists.txt b/libunwind/src/CMakeLists.txt
index 9c6f5d908b0945..780430ba70ba60 100644
--- a/libunwind/src/CMakeLists.txt
+++ b/libunwind/src/CMakeLists.txt
@@ -201,7 +201,10 @@ set_target_properties(unwind_static_objects
if(LIBUNWIND_HIDE_SYMBOLS)
target_add_compile_flags_if_supported(unwind_static_objects PRIVATE -fvisibility=hidden)
- target_add_compile_flags_if_supported(unwind_static_objects PRIVATE -fvisibility-global-new-delete-hidden)
+ target_add_compile_flags_if_supported(unwind_static_objects PRIVATE -fvisibility-global-new-delete=force-hidden)
+ if (NOT CXX_SUPPORTS_FVISIBILITY_GLOBAL_NEW_DELETE_EQ_FORCE_HIDDEN_FLAG)
+ target_add_compile_flags_if_supported(unwind_static_objects PRIVATE -fvisibility-global-new-delete-hidden)
+ endif()
target_compile_definitions(unwind_static_objects PRIVATE _LIBUNWIND_HIDE_SYMBOLS)
endif()
|
MaskRay
approved these changes
Mar 13, 2024
ldionne
approved these changes
Mar 13, 2024
|
@ldionne Is the test failure in this PR safe to ignore? |
|
@tstellar Yes. I think I'll disable these tests which keep failing in the LLVM 18 release branch so we can remove some of the noise here. |
…4917) 27ce26b added the new option -fvisibility-global-new-delete=, where -fvisibility-global-new-delete=force-hidden is equivalent to the old option -fvisibility-global-new-delete-hidden. At the same time, the old option was deprecated. Test for and use the new option form first; if unsupported, try using the old form. This avoids warnings in the MinGW builds, if built with Clang 18 or newer. (cherry picked from commit 1f973ef)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport 1f973ef
Requested by: @ldionne