https://github.com/abrachet created 
https://github.com/llvm/llvm-project/pull/67067

There was an issue with relative vtables when two TU's which define the same 
vtable object are built with different default visibilities. Some TU's are 
built with -fvisibility=hidden in the code base, grep for 
CMAKE_CXX_VISIBILITY_PRESET to find them. Our whole toolchain, is statically 
linked, and built with -fPIE anyway, so the cost of overriding local 
CMAKE_CXX_VISIBILITY_PRESET properties is not high. I've counted that adding 
this flag increases our llvm binary by 13 relocations. Frankly I'm not sure 
where those are even coming from.

It would be preferable to use hidden visibility, but that breaks liblld. This 
can be solved by setting LLDB_EXPORT_ALL_SYMBOLS. After that some ORC tests 
fail which do symbolic lookup in the tests. It seems that setting 
CMAKE_CXX_VISIBILITY_PRESET=hidden will not be worth the maintenance burden. 
Setting it to default works to unblock using relative vtables, so we can just 
go with that.

>From 3d54dc44c1cf0eb5a115b93737070ad8142dcf5b Mon Sep 17 00:00:00 2001
From: Alex Brachet <abrac...@google.com>
Date: Wed, 20 Sep 2023 17:55:44 -0400
Subject: [PATCH] [Fuchsia] Build with -fvisibility=default

There was an issue with relative vtables when two TU's which define
the same vtable object are built with different default visibilities.
Some TU's are built with -fvisibility=hidden in the code base, grep
for CMAKE_CXX_VISIBILITY_PRESET to find them. Our whole toolchain,
is statically linked, and built with -fPIE anyway, so the cost of
overriding local CMAKE_CXX_VISIBILITY_PRESET properties is not high.
I've counted that adding this flag increases our llvm binary by 13
relocations. Frankly I'm not sure where those are even coming from.

It would be preferable to use hidden visibility, but that breaks
liblld. This can be solved by setting LLDB_EXPORT_ALL_SYMBOLS.
After that some ORC tests fail which do symbolic lookup in the
tests. It seems that setting CMAKE_CXX_VISIBILITY_PRESET=hidden
will not be worth the maintenance burden. Setting it to default
works to unblock using relative vtables, so we can just go with
that.
---
 clang/cmake/caches/Fuchsia-stage2.cmake | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/clang/cmake/caches/Fuchsia-stage2.cmake 
b/clang/cmake/caches/Fuchsia-stage2.cmake
index bc4d9f1462b1814..a3b7a4cb1c59886 100644
--- a/clang/cmake/caches/Fuchsia-stage2.cmake
+++ b/clang/cmake/caches/Fuchsia-stage2.cmake
@@ -28,6 +28,7 @@ set(LLVM_STATIC_LINK_CXX_STDLIB ON CACHE BOOL "")
 set(LLVM_USE_RELATIVE_PATHS_IN_FILES ON CACHE BOOL "")
 set(LLDB_ENABLE_CURSES OFF CACHE BOOL "")
 set(LLDB_ENABLE_LIBEDIT OFF CACHE BOOL "")
+# set(LLDB_EXPORT_ALL_SYMBOLS ON CACHE BOOL "")
 
 if(WIN32)
   set(LLVM_USE_CRT_RELEASE "MT" CACHE STRING "")
@@ -52,6 +53,7 @@ set(ENABLE_LINKER_BUILD_ID ON CACHE BOOL "")
 set(ENABLE_X86_RELAX_RELOCATIONS ON CACHE BOOL "")
 
 set(CMAKE_BUILD_TYPE Release CACHE STRING "")
+set(CMAKE_CXX_VISIBILITY_PRESET default CACHE STRING "")
 if (APPLE)
   set(CMAKE_OSX_DEPLOYMENT_TARGET "10.13" CACHE STRING "")
 elseif(WIN32)

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to