[clang] [Clang] Add explicit visibility symbol macros (PR #108276)

Thomas Fransham via cfe-commits cfe-commits at lists.llvm.org
Tue Oct 8 22:13:43 PDT 2024


https://github.com/fsfod updated https://github.com/llvm/llvm-project/pull/108276

>From fff6064a63ddf85857ea5036333866317a156ffc Mon Sep 17 00:00:00 2001
From: Thomas Fransham <tfransham at gmail.com>
Date: Tue, 10 Sep 2024 02:22:18 +0100
Subject: [PATCH 1/7] [Clang] Add explicit visibility symbol macros and update
 CMake to control them

We need a different set of visibility macros to LLVM's since on windows you need a
different attribute to import and export a symbol unlike other platforms and many
translation units will be importing LLVM symbols and export some of Clangs.
Updated Clang CMake to define a macro to enable the symbol visibility macros.
---
 clang/cmake/modules/AddClang.cmake     |  4 ++
 clang/include/clang/Support/Compiler.h | 69 ++++++++++++++++++++++++++
 clang/tools/libclang/CMakeLists.txt    |  2 +-
 3 files changed, 74 insertions(+), 1 deletion(-)
 create mode 100644 clang/include/clang/Support/Compiler.h

diff --git a/clang/cmake/modules/AddClang.cmake b/clang/cmake/modules/AddClang.cmake
index 5327b5d2f08928..dcdea3eeee78fb 100644
--- a/clang/cmake/modules/AddClang.cmake
+++ b/clang/cmake/modules/AddClang.cmake
@@ -108,6 +108,10 @@ macro(add_clang_library name)
   endif()
   llvm_add_library(${name} ${LIBTYPE} ${ARG_UNPARSED_ARGUMENTS} ${srcs})
 
+  if(NOT ARG_SHARED AND NOT ARG_STATIC)
+    target_compile_definitions("obj.${name}" PRIVATE CLANG_EXPORTS)
+  endif()
+
   set(libs ${name})
   if(ARG_SHARED AND ARG_STATIC)
     list(APPEND libs ${name}_static)
diff --git a/clang/include/clang/Support/Compiler.h b/clang/include/clang/Support/Compiler.h
new file mode 100644
index 00000000000000..4a584e190dc16e
--- /dev/null
+++ b/clang/include/clang/Support/Compiler.h
@@ -0,0 +1,69 @@
+//===-- clang/Support/Compiler.h - Compiler abstraction support -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines explicit visibility macros used to export symbols from
+// clang-cpp
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef CLANG_SUPPORT_COMPILER_H
+#define CLANG_SUPPORT_COMPILER_H
+
+#include "llvm/Support/Compiler.h"
+
+/// CLANG_ABI is the main export/visibility macro to mark something as
+/// explicitly exported when clang is built as a shared library with everything
+/// else that is unannotated will have internal visibility.
+///
+/// CLANG_EXPORT_TEMPLATE is used on explicit template instantiations in source
+/// files that were declared extern in a header. This macro is only set as a
+/// compiler export attribute on windows, on other platforms it does nothing.
+///
+/// CLANG_TEMPLATE_ABI is for annotating extern template declarations in headers
+/// for both functions and classes. On windows its turned in to dllimport for
+/// library consumers, for other platforms its a default visibility attribute.
+#ifndef CLANG_ABI_GENERATING_ANNOTATIONS
+// Marker to add to classes or functions in public headers that should not have
+// export macros added to them by the clang tool
+#define CLANG_ABI_NOT_EXPORTED
+#if defined(LLVM_BUILD_LLVM_DYLIB) || defined(LLVM_BUILD_SHARED_LIBS)
+// Some libraries like those for tablegen are linked in to tools that used
+// in the build so can't depend on the llvm shared library. If export macros
+// were left enabled when building these we would get duplicate or
+// missing symbol linker errors on windows.
+#if defined(CLANG_BUILD_STATIC)
+#define CLANG_ABI
+#define CLANG_TEMPLATE_ABI
+#define CLANG_EXPORT_TEMPLATE
+#elif defined(_WIN32) && !defined(__MINGW32__)
+#if defined(CLANG_EXPORTS)
+#define CLANG_ABI __declspec(dllexport)
+#define CLANG_TEMPLATE_ABI
+#define CLANG_EXPORT_TEMPLATE __declspec(dllexport)
+#else
+#define CLANG_ABI __declspec(dllimport)
+#define CLANG_TEMPLATE_ABI __declspec(dllimport)
+#define CLANG_EXPORT_TEMPLATE
+#endif
+#elif defined(__ELF__) || defined(__MINGW32__) || defined(_AIX)
+#define CLANG_ABI LLVM_ATTRIBUTE_VISIBILITY_DEFAULT
+#define CLANG_TEMPLATE_ABI LLVM_ATTRIBUTE_VISIBILITY_DEFAULT
+#define CLANG_EXPORT_TEMPLATE
+#elif defined(__MACH__) || defined(__WASM__)
+#define CLANG_ABI LLVM_ATTRIBUTE_VISIBILITY_DEFAULT
+#define CLANG_TEMPLATE_ABI
+#define CLANG_EXPORT_TEMPLATE
+#endif
+#else
+#define CLANG_ABI
+#define CLANG_TEMPLATE_ABI
+#define CLANG_EXPORT_TEMPLATE
+#endif
+#endif
+
+#endif
\ No newline at end of file
diff --git a/clang/tools/libclang/CMakeLists.txt b/clang/tools/libclang/CMakeLists.txt
index 968b46acb784cb..8c6a07f9e52a8c 100644
--- a/clang/tools/libclang/CMakeLists.txt
+++ b/clang/tools/libclang/CMakeLists.txt
@@ -166,7 +166,7 @@ if(ENABLE_SHARED)
     set_target_properties(libclang
       PROPERTIES
       VERSION ${LIBCLANG_LIBRARY_VERSION}
-      DEFINE_SYMBOL _CINDEX_LIB_)
+      DEFINE_SYMBOL _CINDEX_LIB_  DEFINE_SYMBOL CLANG_EXPORTS)
   elseif(APPLE)
     set(LIBCLANG_LINK_FLAGS " -Wl,-compatibility_version -Wl,1")
     set(LIBCLANG_LINK_FLAGS "${LIBCLANG_LINK_FLAGS} -Wl,-current_version -Wl,${LLVM_VERSION_MAJOR}.${LLVM_VERSION_MINOR}.${LLVM_VERSION_PATCH}")

>From 7ff61ef49c19c2a04c6b7a1d2d190e36e857b0b6 Mon Sep 17 00:00:00 2001
From: Thomas Fransham <tfransham at gmail.com>
Date: Wed, 11 Sep 2024 19:54:27 +0100
Subject: [PATCH 2/7] Add a option to add_clang_tool CMake macro to force
 static linking to clang libraries

---
 clang/cmake/modules/AddClang.cmake            | 21 ++++++++++++++-----
 clang/tools/amdgpu-arch/CMakeLists.txt        |  3 ++-
 .../tools/clang-linker-wrapper/CMakeLists.txt |  2 +-
 clang/tools/nvptx-arch/CMakeLists.txt         |  3 ++-
 4 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/clang/cmake/modules/AddClang.cmake b/clang/cmake/modules/AddClang.cmake
index dcdea3eeee78fb..ee6397a7c543bd 100644
--- a/clang/cmake/modules/AddClang.cmake
+++ b/clang/cmake/modules/AddClang.cmake
@@ -155,20 +155,27 @@ macro(add_clang_executable name)
 endmacro(add_clang_executable)
 
 macro(add_clang_tool name)
-  cmake_parse_arguments(ARG "DEPENDS;GENERATE_DRIVER" "" "" ${ARGN})
+  cmake_parse_arguments(ARG "DEPENDS;GENERATE_DRIVER;DISABLE_CLANG_LINK_DYLIB" "" "" ${ARGN})
   if (NOT CLANG_BUILD_TOOLS)
     set(EXCLUDE_FROM_ALL ON)
   endif()
+
+  set(args_list ${ARGN})
+
+  if(ARG_DISABLE_CLANG_LINK_DYLIB)
+    # Remove this so the llvm argument parsing doesn't get confused
+    list(REMOVE_ITEM args_list DISABLE_CLANG_LINK_DYLIB)
+  endif()
+
   if(ARG_GENERATE_DRIVER
      AND LLVM_TOOL_LLVM_DRIVER_BUILD
      AND (NOT LLVM_DISTRIBUTION_COMPONENTS OR ${name} IN_LIST LLVM_DISTRIBUTION_COMPONENTS)
     )
-    set(get_obj_args ${ARGN})
-    list(FILTER get_obj_args EXCLUDE REGEX "^SUPPORT_PLUGINS$")
-    generate_llvm_objects(${name} ${get_obj_args})
+    list(FILTER args_list EXCLUDE REGEX "^SUPPORT_PLUGINS$")
+    generate_llvm_objects(${name} ${args_list})
     add_custom_target(${name} DEPENDS llvm-driver clang-resource-headers)
   else()
-    add_clang_executable(${name} ${ARGN})
+    add_clang_executable(${name} ${args_list})
     add_dependencies(${name} clang-resource-headers)
 
     if (CLANG_BUILD_TOOLS)
@@ -187,6 +194,10 @@ macro(add_clang_tool name)
     endif()
   endif()
   set_target_properties(${name} PROPERTIES XCODE_GENERATE_SCHEME ON)
+
+  if(ARG_DISABLE_CLANG_LINK_DYLIB)
+    target_compile_definitions(${name} PRIVATE CLANG_BUILD_STATIC)
+  endif()
 endmacro()
 
 macro(add_clang_symlink name dest)
diff --git a/clang/tools/amdgpu-arch/CMakeLists.txt b/clang/tools/amdgpu-arch/CMakeLists.txt
index 1657c701251308..6b7bed99544cee 100644
--- a/clang/tools/amdgpu-arch/CMakeLists.txt
+++ b/clang/tools/amdgpu-arch/CMakeLists.txt
@@ -8,6 +8,7 @@
 
 set(LLVM_LINK_COMPONENTS Support)
 
-add_clang_tool(amdgpu-arch AMDGPUArch.cpp AMDGPUArchByHSA.cpp AMDGPUArchByHIP.cpp)
+add_clang_tool(amdgpu-arch DISABLE_CLANG_LINK_DYLIB
+AMDGPUArch.cpp AMDGPUArchByHSA.cpp AMDGPUArchByHIP.cpp)
 
 target_link_libraries(amdgpu-arch PRIVATE clangBasic)
diff --git a/clang/tools/clang-linker-wrapper/CMakeLists.txt b/clang/tools/clang-linker-wrapper/CMakeLists.txt
index bf37d8031025ed..61926a5b8a6b58 100644
--- a/clang/tools/clang-linker-wrapper/CMakeLists.txt
+++ b/clang/tools/clang-linker-wrapper/CMakeLists.txt
@@ -26,7 +26,7 @@ if(NOT CLANG_BUILT_STANDALONE)
   set(tablegen_deps intrinsics_gen LinkerWrapperOpts)
 endif()
 
-add_clang_tool(clang-linker-wrapper
+add_clang_tool(clang-linker-wrapper DISABLE_CLANG_LINK_DYLIB
   ClangLinkerWrapper.cpp
 
   DEPENDS
diff --git a/clang/tools/nvptx-arch/CMakeLists.txt b/clang/tools/nvptx-arch/CMakeLists.txt
index 8f756be2c86d04..0fd9951b631b22 100644
--- a/clang/tools/nvptx-arch/CMakeLists.txt
+++ b/clang/tools/nvptx-arch/CMakeLists.txt
@@ -7,6 +7,7 @@
 # //===--------------------------------------------------------------------===//
 
 set(LLVM_LINK_COMPONENTS Support)
-add_clang_tool(nvptx-arch NVPTXArch.cpp)
+add_clang_tool(nvptx-arch DISABLE_CLANG_LINK_DYLIB
+NVPTXArch.cpp)
 
 target_link_libraries(nvptx-arch PRIVATE clangBasic)

>From 8c1b0e1e35d503384f453da1de451a8bf3a5c051 Mon Sep 17 00:00:00 2001
From: Thomas Fransham <tfransham at gmail.com>
Date: Fri, 13 Sep 2024 17:58:05 +0100
Subject: [PATCH 3/7] Fix defining the macro in cmake to control c++ symbol
 visibility for libclang on windows

---
 clang/tools/libclang/CMakeLists.txt | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/clang/tools/libclang/CMakeLists.txt b/clang/tools/libclang/CMakeLists.txt
index 8c6a07f9e52a8c..00a1223c0831a7 100644
--- a/clang/tools/libclang/CMakeLists.txt
+++ b/clang/tools/libclang/CMakeLists.txt
@@ -166,7 +166,11 @@ if(ENABLE_SHARED)
     set_target_properties(libclang
       PROPERTIES
       VERSION ${LIBCLANG_LIBRARY_VERSION}
-      DEFINE_SYMBOL _CINDEX_LIB_  DEFINE_SYMBOL CLANG_EXPORTS)
+      DEFINE_SYMBOL _CINDEX_LIB_)
+      # Avoid declaring clang c++ symbols that are statically linked into libclang as dllimport'ed.
+      # If llvm/libclang-cpp dll is also being built for windows clang c++ symbols will still be
+      # implicitly be exported from libclang.
+      target_compile_definitions(libclang PRIVATE CLANG_BUILD_STATIC)
   elseif(APPLE)
     set(LIBCLANG_LINK_FLAGS " -Wl,-compatibility_version -Wl,1")
     set(LIBCLANG_LINK_FLAGS "${LIBCLANG_LINK_FLAGS} -Wl,-current_version -Wl,${LLVM_VERSION_MAJOR}.${LLVM_VERSION_MINOR}.${LLVM_VERSION_PATCH}")

>From 7b61be7bc49a13e489a0d145195acfb77d6cbebe Mon Sep 17 00:00:00 2001
From: Thomas Fransham <tfransham at gmail.com>
Date: Fri, 20 Sep 2024 18:30:06 +0100
Subject: [PATCH 4/7] Fix missing newline at end of file

---
 clang/include/clang/Support/Compiler.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/include/clang/Support/Compiler.h b/clang/include/clang/Support/Compiler.h
index 4a584e190dc16e..1e1eb8ac408219 100644
--- a/clang/include/clang/Support/Compiler.h
+++ b/clang/include/clang/Support/Compiler.h
@@ -66,4 +66,4 @@
 #endif
 #endif
 
-#endif
\ No newline at end of file
+#endif

>From a10cc2340ea0134698024be27aa5b5aa0b8a604a Mon Sep 17 00:00:00 2001
From: Thomas Fransham <tfransham at gmail.com>
Date: Mon, 23 Sep 2024 21:49:44 +0100
Subject: [PATCH 5/7] Try to fully control the mode of clang visibility macros
 from CMake

---
 clang/cmake/modules/AddClang.cmake     | 11 +++++++++--
 clang/include/clang/Support/Compiler.h |  2 --
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/clang/cmake/modules/AddClang.cmake b/clang/cmake/modules/AddClang.cmake
index ee6397a7c543bd..1f8239cfde7db0 100644
--- a/clang/cmake/modules/AddClang.cmake
+++ b/clang/cmake/modules/AddClang.cmake
@@ -108,8 +108,15 @@ macro(add_clang_library name)
   endif()
   llvm_add_library(${name} ${LIBTYPE} ${ARG_UNPARSED_ARGUMENTS} ${srcs})
 
-  if(NOT ARG_SHARED AND NOT ARG_STATIC)
-    target_compile_definitions("obj.${name}" PRIVATE CLANG_EXPORTS)
+  if(MSVC AND NOT CLANG_LINK_CLANG_DYLIB)
+    # Make sure all consumers also turn off visibility macros so there not trying to dllimport symbols.
+    target_compile_definitions(${name} PUBLIC CLANG_BUILD_STATIC)
+    if(TARGET "obj.${name}")
+      target_compile_definitions("obj.${name}" PUBLIC CLANG_BUILD_STATIC)
+    endif()
+  elseif(NOT ARG_SHARED AND NOT ARG_STATIC)
+    # Clang component libraries linked in to clang-cpp are declared without SHARED or STATIC
+    target_compile_definitions("obj.${name}" PUBLIC CLANG_EXPORTS)
   endif()
 
   set(libs ${name})
diff --git a/clang/include/clang/Support/Compiler.h b/clang/include/clang/Support/Compiler.h
index 1e1eb8ac408219..a2a8ca1ac962b3 100644
--- a/clang/include/clang/Support/Compiler.h
+++ b/clang/include/clang/Support/Compiler.h
@@ -31,7 +31,6 @@
 // Marker to add to classes or functions in public headers that should not have
 // export macros added to them by the clang tool
 #define CLANG_ABI_NOT_EXPORTED
-#if defined(LLVM_BUILD_LLVM_DYLIB) || defined(LLVM_BUILD_SHARED_LIBS)
 // Some libraries like those for tablegen are linked in to tools that used
 // in the build so can't depend on the llvm shared library. If export macros
 // were left enabled when building these we would get duplicate or
@@ -64,6 +63,5 @@
 #define CLANG_TEMPLATE_ABI
 #define CLANG_EXPORT_TEMPLATE
 #endif
-#endif
 
 #endif

>From 8fc43259f06fdd149838bb93bdb5b779d47381c9 Mon Sep 17 00:00:00 2001
From: Thomas Fransham <tfransham at gmail.com>
Date: Thu, 26 Sep 2024 06:39:45 +0100
Subject: [PATCH 6/7] Just leave ARG_DISABLE_CLANG_LINK_DYLIB for another PR
 since theres no use of the macros yet

---
 clang/cmake/modules/AddClang.cmake            | 21 +++++--------------
 clang/tools/amdgpu-arch/CMakeLists.txt        |  3 +--
 .../tools/clang-linker-wrapper/CMakeLists.txt |  2 +-
 clang/tools/nvptx-arch/CMakeLists.txt         |  3 +--
 4 files changed, 8 insertions(+), 21 deletions(-)

diff --git a/clang/cmake/modules/AddClang.cmake b/clang/cmake/modules/AddClang.cmake
index 1f8239cfde7db0..091aec98e93ca3 100644
--- a/clang/cmake/modules/AddClang.cmake
+++ b/clang/cmake/modules/AddClang.cmake
@@ -162,27 +162,20 @@ macro(add_clang_executable name)
 endmacro(add_clang_executable)
 
 macro(add_clang_tool name)
-  cmake_parse_arguments(ARG "DEPENDS;GENERATE_DRIVER;DISABLE_CLANG_LINK_DYLIB" "" "" ${ARGN})
+  cmake_parse_arguments(ARG "DEPENDS;GENERATE_DRIVER" "" "" ${ARGN})
   if (NOT CLANG_BUILD_TOOLS)
     set(EXCLUDE_FROM_ALL ON)
   endif()
-
-  set(args_list ${ARGN})
-
-  if(ARG_DISABLE_CLANG_LINK_DYLIB)
-    # Remove this so the llvm argument parsing doesn't get confused
-    list(REMOVE_ITEM args_list DISABLE_CLANG_LINK_DYLIB)
-  endif()
-
   if(ARG_GENERATE_DRIVER
      AND LLVM_TOOL_LLVM_DRIVER_BUILD
      AND (NOT LLVM_DISTRIBUTION_COMPONENTS OR ${name} IN_LIST LLVM_DISTRIBUTION_COMPONENTS)
     )
-    list(FILTER args_list EXCLUDE REGEX "^SUPPORT_PLUGINS$")
-    generate_llvm_objects(${name} ${args_list})
+    set(get_obj_args ${ARGN})
+    list(FILTER get_obj_args EXCLUDE REGEX "^SUPPORT_PLUGINS$")
+    generate_llvm_objects(${name} ${get_obj_args})
     add_custom_target(${name} DEPENDS llvm-driver clang-resource-headers)
   else()
-    add_clang_executable(${name} ${args_list})
+    add_clang_executable(${name} ${ARGN})
     add_dependencies(${name} clang-resource-headers)
 
     if (CLANG_BUILD_TOOLS)
@@ -201,10 +194,6 @@ macro(add_clang_tool name)
     endif()
   endif()
   set_target_properties(${name} PROPERTIES XCODE_GENERATE_SCHEME ON)
-
-  if(ARG_DISABLE_CLANG_LINK_DYLIB)
-    target_compile_definitions(${name} PRIVATE CLANG_BUILD_STATIC)
-  endif()
 endmacro()
 
 macro(add_clang_symlink name dest)
diff --git a/clang/tools/amdgpu-arch/CMakeLists.txt b/clang/tools/amdgpu-arch/CMakeLists.txt
index 6b7bed99544cee..1657c701251308 100644
--- a/clang/tools/amdgpu-arch/CMakeLists.txt
+++ b/clang/tools/amdgpu-arch/CMakeLists.txt
@@ -8,7 +8,6 @@
 
 set(LLVM_LINK_COMPONENTS Support)
 
-add_clang_tool(amdgpu-arch DISABLE_CLANG_LINK_DYLIB
-AMDGPUArch.cpp AMDGPUArchByHSA.cpp AMDGPUArchByHIP.cpp)
+add_clang_tool(amdgpu-arch AMDGPUArch.cpp AMDGPUArchByHSA.cpp AMDGPUArchByHIP.cpp)
 
 target_link_libraries(amdgpu-arch PRIVATE clangBasic)
diff --git a/clang/tools/clang-linker-wrapper/CMakeLists.txt b/clang/tools/clang-linker-wrapper/CMakeLists.txt
index 61926a5b8a6b58..bf37d8031025ed 100644
--- a/clang/tools/clang-linker-wrapper/CMakeLists.txt
+++ b/clang/tools/clang-linker-wrapper/CMakeLists.txt
@@ -26,7 +26,7 @@ if(NOT CLANG_BUILT_STANDALONE)
   set(tablegen_deps intrinsics_gen LinkerWrapperOpts)
 endif()
 
-add_clang_tool(clang-linker-wrapper DISABLE_CLANG_LINK_DYLIB
+add_clang_tool(clang-linker-wrapper
   ClangLinkerWrapper.cpp
 
   DEPENDS
diff --git a/clang/tools/nvptx-arch/CMakeLists.txt b/clang/tools/nvptx-arch/CMakeLists.txt
index 0fd9951b631b22..8f756be2c86d04 100644
--- a/clang/tools/nvptx-arch/CMakeLists.txt
+++ b/clang/tools/nvptx-arch/CMakeLists.txt
@@ -7,7 +7,6 @@
 # //===--------------------------------------------------------------------===//
 
 set(LLVM_LINK_COMPONENTS Support)
-add_clang_tool(nvptx-arch DISABLE_CLANG_LINK_DYLIB
-NVPTXArch.cpp)
+add_clang_tool(nvptx-arch NVPTXArch.cpp)
 
 target_link_libraries(nvptx-arch PRIVATE clangBasic)

>From a464d83f3ce1eec8bfdc7165609806d9ebdc12c3 Mon Sep 17 00:00:00 2001
From: Thomas Fransham <fsfod11 at gmail.com>
Date: Wed, 9 Oct 2024 06:13:33 +0100
Subject: [PATCH 7/7] suggested comment changes

Co-authored-by: Aaron Ballman <aaron at aaronballman.com>
---
 clang/include/clang/Support/Compiler.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/include/clang/Support/Compiler.h b/clang/include/clang/Support/Compiler.h
index a2a8ca1ac962b3..da820eeb520d51 100644
--- a/clang/include/clang/Support/Compiler.h
+++ b/clang/include/clang/Support/Compiler.h
@@ -18,7 +18,7 @@
 
 /// CLANG_ABI is the main export/visibility macro to mark something as
 /// explicitly exported when clang is built as a shared library with everything
-/// else that is unannotated will have internal visibility.
+/// else that is unannotated having internal visibility.
 ///
 /// CLANG_EXPORT_TEMPLATE is used on explicit template instantiations in source
 /// files that were declared extern in a header. This macro is only set as a



More information about the cfe-commits mailing list