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

[libcxx, libcxxabi, libunwind] Prefer -fvisibility-global-new-delete=force-hidden #84917

Merged
merged 1 commit into from
Mar 13, 2024

Conversation

mstorsjo
Copy link
Member

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.

…force-hidden

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.
@mstorsjo mstorsjo added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Mar 12, 2024
@mstorsjo mstorsjo requested review from a team as code owners March 12, 2024 13:57
@llvmbot llvmbot added libc++abi libc++abi C++ Runtime Library. Not libc++. libunwind labels Mar 12, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 12, 2024

@llvm/pr-subscribers-libunwind
@llvm/pr-subscribers-libcxxabi

@llvm/pr-subscribers-libcxx

Author: Martin Storsjö (mstorsjo)

Changes

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.


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

3 Files Affected:

  • (modified) libcxx/src/CMakeLists.txt (+4-1)
  • (modified) libcxxabi/src/CMakeLists.txt (+4-1)
  • (modified) libunwind/src/CMakeLists.txt (+4-1)
diff --git a/libcxx/src/CMakeLists.txt b/libcxx/src/CMakeLists.txt
index 07ffc8bfdaae3d..1110a79ddcacd5 100644
--- a/libcxx/src/CMakeLists.txt
+++ b/libcxx/src/CMakeLists.txt
@@ -301,7 +301,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 0af4dc1448e91a..c8cc93de50777b 100644
--- a/libcxxabi/src/CMakeLists.txt
+++ b/libcxxabi/src/CMakeLists.txt
@@ -268,7 +268,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()
 

@ldionne ldionne added this to the LLVM 18.X Release milestone Mar 13, 2024
@ldionne ldionne merged commit 1f973ef into llvm:main Mar 13, 2024
55 checks passed
@ldionne
Copy link
Member

ldionne commented Mar 13, 2024

/cherry-pick 1f973ef

llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Mar 13, 2024
…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)
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 13, 2024

/pull-request #85126

@mstorsjo mstorsjo deleted the visibility-global-new-delete branch March 13, 2024 20:15
llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Mar 15, 2024
…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)
@pointhex pointhex mentioned this pull request May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++abi libc++abi C++ Runtime Library. Not libc++. libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. libunwind
Projects
Development

Successfully merging this pull request may close these issues.

4 participants