[Clang] Apply exclude_from_explicit_instantiation to dllimport/dllexport

[kikairoya]

Attaching __declspec(dllexport/dllimport) to explicit instantiation declaration made its whole member instantiated even if they were __attribute__((__exclude_from_explicit_instantation__)).
Such members should not be instantiated nor be exported to avoid symbol leakage or duplication.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Tomohiro Kashiwada (kikairoya)

Changes

Attaching __declspec(dllexport/dllimport) to explicit instantiation declaration made its whole member instantiated even if they were __attribute__((__exclude_from_explicit_instantation__)).
Such members should not be instantiated nor be exported to avoid symbol leakage or duplication.

---

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

3 Files Affected:

• (modified) clang/lib/Sema/SemaDeclCXX.cpp (+12-2)
• (added) clang/test/CodeGenCXX/attr-exclude_from_explicit_instantiation.exclude_from_dllexport.cpp (+59)
• (added) clang/test/CodeGenCXX/attr-exclude_from_explicit_instantiation.exclude_from_dllimport.cpp (+58)

diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index aa36a79142e52..fe732a1328fab 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -6551,6 +6551,8 @@ void Sema::checkClassLevelDLLAttribute(CXXRecordDecl *Class) {
     return;
   }
 
+  TemplateSpecializationKind TSK = Class->getTemplateSpecializationKind();
+
   if (Context.getTargetInfo().shouldDLLImportComdatSymbols() &&
       !ClassAttr->isInherited()) {
     // Diagnose dll attributes on members of class with dll attribute.
@@ -6561,6 +6563,11 @@ void Sema::checkClassLevelDLLAttribute(CXXRecordDecl *Class) {
       if (!MemberAttr || MemberAttr->isInherited() || Member->isInvalidDecl())
         continue;
 
+      if ((TSK == TSK_ExplicitInstantiationDeclaration ||
+           TSK == TSK_ExplicitInstantiationDefinition) &&
+          Member->hasAttr<ExcludeFromExplicitInstantiationAttr>())
+        continue;
+
       Diag(MemberAttr->getLocation(),
              diag::err_attribute_dll_member_of_dll_class)
           << MemberAttr << ClassAttr;
@@ -6583,8 +6590,6 @@ void Sema::checkClassLevelDLLAttribute(CXXRecordDecl *Class) {
       !ClassExported &&
       cast<DLLImportAttr>(ClassAttr)->wasPropagatedToBaseTemplate();
 
-  TemplateSpecializationKind TSK = Class->getTemplateSpecializationKind();
-
   // Ignore explicit dllexport on explicit class template instantiation
   // declarations, except in MinGW mode.
   if (ClassExported && !ClassAttr->isInherited() &&
@@ -6601,6 +6606,11 @@ void Sema::checkClassLevelDLLAttribute(CXXRecordDecl *Class) {
   // seem to be true in practice?
 
   for (Decl *Member : Class->decls()) {
+    if ((TSK == TSK_ExplicitInstantiationDeclaration ||
+         TSK == TSK_ExplicitInstantiationDefinition) &&
+        Member->hasAttr<ExcludeFromExplicitInstantiationAttr>())
+      continue;
+
     VarDecl *VD = dyn_cast<VarDecl>(Member);
     CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(Member);
 
diff --git a/clang/test/CodeGenCXX/attr-exclude_from_explicit_instantiation.exclude_from_dllexport.cpp b/clang/test/CodeGenCXX/attr-exclude_from_explicit_instantiation.exclude_from_dllexport.cpp
new file mode 100644
index 0000000000000..0a94bd4b1f6b6
--- /dev/null
+++ b/clang/test/CodeGenCXX/attr-exclude_from_explicit_instantiation.exclude_from_dllexport.cpp
@@ -0,0 +1,59 @@
+// RUN: %clang_cc1 -triple x86_64-win32 -fms-extensions -emit-llvm -o - %s | FileCheck %s --check-prefixes=MSC --implicit-check-not=to_be_
+// RUN: %clang_cc1 -triple x86_64-mingw                 -emit-llvm -o - %s | FileCheck %s --check-prefixes=GNU --implicit-check-not=to_be_
+// RUN: %clang_cc1 -triple x86_64-cygwin                -emit-llvm -o - %s | FileCheck %s --check-prefixes=GNU --implicit-check-not=to_be_
+
+// Test that __declspec(dllexport) doesn't instantiate entities marked with
+// the exclude_from_explicit_instantiation attribute unless marked as dllexport explicitly.
+
+#define EXCLUDE_FROM_EXPLICIT_INSTANTIATION __attribute__((exclude_from_explicit_instantiation))
+
+template <class T>
+struct C {
+  // This will be instantiated explicitly as an exported function because it
+  // inherits dllexport from the class instantiation.
+  void to_be_exported() noexcept;
+
+  // This will be instantiated implicitly as an exported function because it is
+  // marked as dllexport explicitly.
+  EXCLUDE_FROM_EXPLICIT_INSTANTIATION __declspec(dllexport) void to_be_exported_explicitly() noexcept;
+
+  // This will be instantiated implicitly but won't be exported.
+  EXCLUDE_FROM_EXPLICIT_INSTANTIATION void not_to_be_exported() noexcept;
+
+  // This won't be instantiated.
+  EXCLUDE_FROM_EXPLICIT_INSTANTIATION void not_to_be_instantiated() noexcept;
+};
+
+template <class T> void C<T>::to_be_exported() noexcept {}
+template <class T> void C<T>::to_be_exported_explicitly() noexcept {}
+template <class T> void C<T>::not_to_be_exported() noexcept {}
+template <class T> void C<T>::not_to_be_instantiated() noexcept {}
+
+// MSC: $"?to_be_exported@?$C@H@@QEAAXXZ" = comdat any
+// MSC: $"?to_be_exported_explicitly@?$C@H@@QEAAXXZ" = comdat any
+// MSC: $"?not_to_be_exported@?$C@H@@QEAAXXZ" = comdat any
+// GNU: $_ZN1CIiE14to_be_exportedEv = comdat any
+// GNU: $_ZN1CIiE25to_be_exported_explicitlyEv = comdat any
+// GNU: $_ZN1CIiE18not_to_be_exportedEv = comdat any
+
+// MSC: define weak_odr dso_local dllexport{{.*}} void @"?to_be_exported@?$C@H@@QEAAXXZ"
+// GNU: define weak_odr dso_local dllexport{{.*}} void @_ZN1CIiE14to_be_exportedEv
+template struct __declspec(dllexport) C<int>;
+
+void use() {
+  C<int> c;
+
+  // MSC: call void @"?to_be_exported_explicitly@?$C@H@@QEAAXXZ"
+  // GNU: call void @_ZN1CIiE25to_be_exported_explicitlyEv
+  c.to_be_exported_explicitly(); // implicitly instantiated here
+
+  // MSC: call void @"?not_to_be_exported@?$C@H@@QEAAXXZ"
+  // GNU: call void @_ZN1CIiE18not_to_be_exportedEv
+  c.not_to_be_exported(); // implicitly instantiated here
+};
+
+// MSC: define weak_odr dso_local dllexport{{.*}} void @"?to_be_exported_explicitly@?$C@H@@QEAAXXZ"
+// GNU: define weak_odr dso_local dllexport{{.*}} void @_ZN1CIiE25to_be_exported_explicitlyEv
+
+// MSC: define linkonce_odr dso_local void @"?not_to_be_exported@?$C@H@@QEAAXXZ"
+// GNU: define linkonce_odr dso_local void @_ZN1CIiE18not_to_be_exportedEv
diff --git a/clang/test/CodeGenCXX/attr-exclude_from_explicit_instantiation.exclude_from_dllimport.cpp b/clang/test/CodeGenCXX/attr-exclude_from_explicit_instantiation.exclude_from_dllimport.cpp
new file mode 100644
index 0000000000000..b070259c2f048
--- /dev/null
+++ b/clang/test/CodeGenCXX/attr-exclude_from_explicit_instantiation.exclude_from_dllimport.cpp
@@ -0,0 +1,58 @@
+// RUN: %clang_cc1 -triple x86_64-win32 -fms-extensions -emit-llvm -o - %s | FileCheck %s --check-prefixes=MSC --implicit-check-not=to_be_
+// RUN: %clang_cc1 -triple x86_64-mingw                 -emit-llvm -o - %s | FileCheck %s --check-prefixes=GNU --implicit-check-not=to_be_
+// RUN: %clang_cc1 -triple x86_64-cygwin                -emit-llvm -o - %s | FileCheck %s --check-prefixes=GNU --implicit-check-not=to_be_
+
+// Test that __declspec(dllimport) doesn't instantiate entities marked with
+// the exclude_from_explicit_instantiation attribute unless marked as dllimport explicitly.
+
+#define EXCLUDE_FROM_EXPLICIT_INSTANTIATION __attribute__((exclude_from_explicit_instantiation))
+
+template <class T>
+struct C {
+  // This will be instantiated explicitly as an imported function because it
+  // inherits dllimport from the class instantiation.
+  void to_be_imported() noexcept;
+
+  // This will be instantiated implicitly as an imported function because it is
+  // marked as dllimport explicitly.
+  EXCLUDE_FROM_EXPLICIT_INSTANTIATION __declspec(dllimport) void to_be_imported_explicitly() noexcept;
+
+  // This will be instantiated implicitly but won't be imported.
+  EXCLUDE_FROM_EXPLICIT_INSTANTIATION void not_to_be_imported() noexcept;
+
+  // This won't be instantiated.
+  EXCLUDE_FROM_EXPLICIT_INSTANTIATION void not_to_be_instantiated() noexcept;
+};
+
+template <class T> void C<T>::to_be_imported() noexcept {}
+template <class T> void C<T>::not_to_be_imported() noexcept {}
+template <class T> void C<T>::not_to_be_instantiated() noexcept {}
+
+// MSC: $"?not_to_be_imported@?$C@H@@QEAAXXZ" = comdat any
+// GNU: $_ZN1CIiE18not_to_be_importedEv = comdat any
+extern template struct __declspec(dllimport) C<int>;
+
+void use() {
+  C<int> c;
+
+  // MSC: call void @"?to_be_imported@?$C@H@@QEAAXXZ"
+  // GNU: call void @_ZN1CIiE14to_be_importedEv
+  c.to_be_imported();
+
+  // MSC: call void @"?to_be_imported_explicitly@?$C@H@@QEAAXXZ"
+  // GNU: call void @_ZN1CIiE25to_be_imported_explicitlyEv
+  c.to_be_imported_explicitly(); // implicitly instantiated here
+
+  // MSC: call void @"?not_to_be_imported@?$C@H@@QEAAXXZ"
+  // GNU: call void @_ZN1CIiE18not_to_be_importedEv
+  c.not_to_be_imported(); // implicitly instantiated here
+};
+
+// MSC: declare dllimport void @"?to_be_imported@?$C@H@@QEAAXXZ"
+// GNU: declare dllimport void @_ZN1CIiE14to_be_importedEv
+
+// MSC: declare dllimport void @"?to_be_imported_explicitly@?$C@H@@QEAAXXZ"
+// GNU: declare dllimport void @_ZN1CIiE25to_be_imported_explicitlyEv
+
+// MSC: define linkonce_odr dso_local void @"?not_to_be_imported@?$C@H@@QEAAXXZ"
+// GNU: define linkonce_odr dso_local void @_ZN1CIiE18not_to_be_importedEv

[kikairoya]

@mstorsjo This fixes #135910 (comment)

[mstorsjo]

Thanks!

If I remember correctly, there are a number of preexisting references to the problem that exclude_from_explicit_instantiation doesn't do what it would need to do on Windows; I think such references may exist in libcxx (in the code tree maybe?) and/or as other issues here on github. It would be a nice extra if we could find some of those references, to get more stakeholders aware of this change here (and possibly know which references to update).

I'm not very familiar with the code here so I'm not sure if I can do an authoritative review - but it practically looks reasonable. Looping in @zmodem who either can do a more confident review, or find more reviewers.

[mstorsjo]

@mstorsjo This fixes #135910 (comment)

That's great! Does this, together with #168170, address the overall problem in #135910, or are there many other issues remaining in order to fix that (other than reverting my old clang change)?

[kikairoya]

That's great! Does this, together with #168170, address the overall problem in #135910, or are there many other issues remaining in order to fix that (other than reverting my old clang change)?

Neither of this nor #168170 is directly related to #135910 itself.
These are prerequisites for libc++'s ABI checking #135910 (comment) , and another patch and checking functionality itself are still required to upstream.

For #135910 , I already have a "functional" local patch (with exporting nested classes), but I feel that I should try to find more edge cases.

[kikairoya]

If I remember correctly, there are a number of preexisting references to the problem that exclude_from_explicit_instantiation doesn't do what it would need to do on Windows; I think such references may exist in libcxx (in the code tree maybe?) and/or as other issues here on github. It would be a nice extra if we could find some of those references, to get more stakeholders aware of this change here (and possibly know which references to update).

Ugh, I missed #66909. I need to add more fixes to address it.