From d598242a9e938153a106b421c9b0f69139e192f3 Mon Sep 17 00:00:00 2001 From: Thomas Fransham Date: Tue, 10 Sep 2024 02:22:18 +0100 Subject: [PATCH 1/9] [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 952bb31df835b61b346965d5bf525788ab1eb7ad Mon Sep 17 00:00:00 2001 From: Thomas Fransham Date: Wed, 11 Sep 2024 19:54:27 +0100 Subject: [PATCH 2/9] 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 7f849a0d0c96450198229efd8f96a9aef30c78aa Mon Sep 17 00:00:00 2001 From: Thomas Fransham Date: Fri, 13 Sep 2024 17:58:05 +0100 Subject: [PATCH 3/9] 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 66aaa9f5d5f9cc148b0985ea254738fba8b89c8f Mon Sep 17 00:00:00 2001 From: Thomas Fransham Date: Fri, 20 Sep 2024 18:30:06 +0100 Subject: [PATCH 4/9] 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 7e8cea686ab4dc98087846ea5944effa00b35de2 Mon Sep 17 00:00:00 2001 From: Thomas Fransham Date: Mon, 23 Sep 2024 21:49:44 +0100 Subject: [PATCH 5/9] 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 c0152f0e8ecbe1732b14984374c1be7b15279267 Mon Sep 17 00:00:00 2001 From: Thomas Fransham Date: Thu, 26 Sep 2024 06:39:45 +0100 Subject: [PATCH 6/9] 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 cccb2875a64d5061a1cf126f009bf9f55db6dcc6 Mon Sep 17 00:00:00 2001 From: Thomas Fransham Date: Wed, 9 Oct 2024 06:13:33 +0100 Subject: [PATCH 7/9] suggested comment changes Co-authored-by: Aaron Ballman --- 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 From 962d215b3f02ef15f02e41ee0b48c7e49f967021 Mon Sep 17 00:00:00 2001 From: Thomas Fransham Date: Wed, 9 Oct 2024 16:13:58 +0100 Subject: [PATCH 8/9] Remove unneeded empty macro visibility definitions These shouldn't be set when the clang tool is generating visibly macros --- clang/include/clang/Support/Compiler.h | 4 ---- 1 file changed, 4 deletions(-) diff --git a/clang/include/clang/Support/Compiler.h b/clang/include/clang/Support/Compiler.h index da820eeb520d51..920ca4dbe0a49d 100644 --- a/clang/include/clang/Support/Compiler.h +++ b/clang/include/clang/Support/Compiler.h @@ -58,10 +58,6 @@ #define CLANG_TEMPLATE_ABI #define CLANG_EXPORT_TEMPLATE #endif -#else -#define CLANG_ABI -#define CLANG_TEMPLATE_ABI -#define CLANG_EXPORT_TEMPLATE #endif #endif From cce1848816bdf95b40d2cb660693a779448578c9 Mon Sep 17 00:00:00 2001 From: Thomas Fransham Date: Fri, 11 Oct 2024 23:49:06 +0100 Subject: [PATCH 9/9] Change comment to say hidden instead of internal visibility --- 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 920ca4dbe0a49d..c35815e106d2e2 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 having internal visibility. +/// else that is unannotated having hidden 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