llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-codegen
Author: Akash Manna (akash-manna-sky)
<details>
<summary>Changes</summary>
Fixes #<!-- -->207963
[clang] Inconsistent attributes for `dllexport __cxxabiv1::
__fundamental_type_info with -fvisibility=hidden
The issue is `__declspec(dllexport)` and `hidden` visibility cannot be used
together. LLVM rejects `globals` that have both attributes.
For normal globals, `CodeGenModule::setGlobalVisibility` already handles this
by giving `dllexport` priority over the visibility implied by
`-fvisibility=hidden`. RTTI follows a different code path, though.
`ItaniumRTTIBuilder::BuildTypeInfo` applies the visibility and DLL storage
class separately to the generated `_ZTI*` and `_ZTS*` globals, so the conflict
was not resolved.
As a result, marking `__cxxabiv1::__fundamental_type_info` as `dllexport` while
compiling with `-fvisibility=hidden` could generate RTTI globals with both
hidden visibility and `dllexport`, causing LLVM to fail verification with
Broken module found. The same problem can occur for `dllexport`-ed polymorphic
classes when targeting Windows with the Itanium ABI.
Resolve the conflict directly in `BuildTypeInfo`, where both attributes are
applied. When `dllexport` is present, only hidden visibility is changed to
default; protected visibility is left unchanged because protected with
`dllexport` is valid LLVM IR. Explicit `visibility("hidden")` on a
`dllexport`-ed class continues to produce the existing diagnostic. I tested the
changes locally, and all tests are passed.
---
Full diff: https://github.com/llvm/llvm-project/pull/215859.diff
2 Files Affected:
- (modified) clang/lib/CodeGen/ItaniumCXXABI.cpp (+14-2)
- (added) clang/test/CodeGenCXX/dllexport-rtti-visibility.cpp (+50)
``````````diff
diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp
b/clang/lib/CodeGen/ItaniumCXXABI.cpp
index c17813140b10f..41913e1891f39 100644
--- a/clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -4237,6 +4237,18 @@ llvm::Constant
*ItaniumRTTIBuilder::BuildTypeInfo(QualType Ty) {
return BuildTypeInfo(Ty, Linkage, llvmVisibility, DLLStorageClass);
}
+/// Returns the visibility to use for an RTTI global. A dllexported global must
+/// not be hidden, so dllexport takes precedence over the visibility implied by
+/// -fvisibility=hidden, as CodeGenModule::setGlobalVisibility does elsewhere.
+static llvm::GlobalValue::VisibilityTypes
+getRTTIVisibility(llvm::GlobalValue::VisibilityTypes Visibility,
+ llvm::GlobalValue::DLLStorageClassTypes DLLStorageClass) {
+ if (DLLStorageClass == llvm::GlobalValue::DLLExportStorageClass &&
+ Visibility == llvm::GlobalValue::HiddenVisibility)
+ return llvm::GlobalValue::DefaultVisibility;
+ return Visibility;
+}
+
llvm::Constant *ItaniumRTTIBuilder::BuildTypeInfo(
QualType Ty,
llvm::GlobalVariable::LinkageTypes Linkage,
@@ -4419,10 +4431,10 @@ llvm::Constant *ItaniumRTTIBuilder::BuildTypeInfo(
// All of this is to say that it's important that both the type_info
// object and the type_info name be uniqued when weakly emitted.
- TypeName->setVisibility(Visibility);
+ TypeName->setVisibility(getRTTIVisibility(Visibility, DLLStorageClass));
CGM.setDSOLocal(TypeName);
- GV->setVisibility(Visibility);
+ GV->setVisibility(getRTTIVisibility(Visibility, GVDLLStorageClass));
CGM.setDSOLocal(GV);
TypeName->setDLLStorageClass(DLLStorageClass);
diff --git a/clang/test/CodeGenCXX/dllexport-rtti-visibility.cpp
b/clang/test/CodeGenCXX/dllexport-rtti-visibility.cpp
new file mode 100644
index 0000000000000..0081a8f8cac3b
--- /dev/null
+++ b/clang/test/CodeGenCXX/dllexport-rtti-visibility.cpp
@@ -0,0 +1,50 @@
+/// dllexport expresses non-hidden intention and takes precedence over the
+/// visibility implied by -fvisibility=hidden. Check that this holds for RTTI.
+
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-windows-gnu -fdeclspec
-fvisibility=hidden -o - %s | FileCheck %s --check-prefixes=CHECK,GNU
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-windows-itanium -fdeclspec
-fvisibility=hidden -o - %s | FileCheck %s --check-prefixes=CHECK,ITANIUM
+
+// GNU-DAG: @_ZTI5plain = linkonce_odr hidden constant
+// GNU-DAG: @_ZTS5plain = linkonce_odr hidden constant
+// ITANIUM-DAG: @_ZTI5plain = hidden constant
+// ITANIUM-DAG: @_ZTS5plain = hidden constant
+struct plain {
+ virtual ~plain();
+};
+plain::~plain() {}
+
+/// RTTI is only dllexported for Windows Itanium.
+// GNU-DAG: @_ZTI8exported = linkonce_odr hidden constant
+// GNU-DAG: @_ZTS8exported = linkonce_odr hidden constant
+// ITANIUM-DAG: @_ZTI8exported = dso_local dllexport constant
+// ITANIUM-DAG: @_ZTS8exported = dso_local dllexport constant
+struct __declspec(dllexport) exported {
+ virtual ~exported();
+};
+exported::~exported() {}
+
+/// Defining __cxxabiv1::__fundamental_type_info makes Clang implicitly emit
the
+/// RTTI descriptors for the fundamental types, with the class' storage class.
+// GNU-DAG: @_ZTIN10__cxxabiv123__fundamental_type_infoE = linkonce_odr hidden
constant
+// GNU-DAG: @_ZTSN10__cxxabiv123__fundamental_type_infoE = linkonce_odr hidden
constant
+// ITANIUM-DAG: @_ZTIN10__cxxabiv123__fundamental_type_infoE = dso_local
dllexport constant
+// ITANIUM-DAG: @_ZTSN10__cxxabiv123__fundamental_type_infoE = dso_local
dllexport constant
+
+// CHECK-DAG: @_ZTIv = dso_local dllexport constant
+// CHECK-DAG: @_ZTSv = dso_local dllexport constant
+// CHECK-DAG: @_ZTIPv = dso_local dllexport constant
+// CHECK-DAG: @_ZTSPv = dso_local dllexport constant
+// CHECK-DAG: @_ZTIPKv = dso_local dllexport constant
+// CHECK-DAG: @_ZTSPKv = dso_local dllexport constant
+// CHECK-DAG: @_ZTIi = dso_local dllexport constant
+// CHECK-DAG: @_ZTSi = dso_local dllexport constant
+// CHECK-DAG: @_ZTIPi = dso_local dllexport constant
+// CHECK-DAG: @_ZTSPi = dso_local dllexport constant
+// CHECK-DAG: @_ZTIPKi = dso_local dllexport constant
+// CHECK-DAG: @_ZTSPKi = dso_local dllexport constant
+namespace __cxxabiv1 {
+struct __declspec(dllexport) __fundamental_type_info {
+ virtual ~__fundamental_type_info();
+};
+__fundamental_type_info::~__fundamental_type_info() {}
+} // namespace __cxxabiv1
``````````
</details>
https://github.com/llvm/llvm-project/pull/215859
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
