https://github.com/DKLoehr created
https://github.com/llvm/llvm-project/pull/145944
Since dllexport/dllimport annotations don't propagate the same way as
visibility, the unique object duplication warning needs to check both the
object in question and its containing class. Previously, we restricted this
check to static data members, but it applies to all objects inside a class,
including functions. Not checking functions leads to false positives, so remove
that restriction.
>From 8ac463dbf1cf906ef0156e5e60403399b3b93a68 Mon Sep 17 00:00:00 2001
From: Devon Loehr <dlo...@google.com>
Date: Thu, 26 Jun 2025 17:50:27 +0000
Subject: [PATCH] Check containing class for everything, not just variables
---
clang/lib/Sema/SemaDecl.cpp | 14 +++---
.../test/SemaCXX/unique_object_duplication.h | 45 +++++++++++++++++--
2 files changed, 47 insertions(+), 12 deletions(-)
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index a60a558155e69..246e0395435bb 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -13524,21 +13524,19 @@ bool
Sema::GloballyUniqueObjectMightBeAccidentallyDuplicated(
// The target is "hidden" (from the dynamic linker) if:
// 1. On posix, it has hidden visibility, or
- // 2. On windows, it has no import/export annotation
+ // 2. On windows, it has no import/export annotation, and neither does the
+ // class which directly contains it.
if (Context.getTargetInfo().shouldDLLImportComdatSymbols()) {
if (Target->hasAttr<DLLExportAttr>() || Target->hasAttr<DLLImportAttr>())
return false;
// If the variable isn't directly annotated, check to see if it's a member
// of an annotated class.
- const VarDecl *VD = dyn_cast<VarDecl>(Target);
+ const CXXRecordDecl *Ctx =
+ dyn_cast<CXXRecordDecl>(Target->getDeclContext());
+ if (Ctx && (Ctx->hasAttr<DLLExportAttr>() ||
Ctx->hasAttr<DLLImportAttr>()))
+ return false;
- if (VD && VD->isStaticDataMember()) {
- const CXXRecordDecl *Ctx = dyn_cast<CXXRecordDecl>(VD->getDeclContext());
- if (Ctx &&
- (Ctx->hasAttr<DLLExportAttr>() || Ctx->hasAttr<DLLImportAttr>()))
- return false;
- }
} else if (Lnk.getVisibility() != HiddenVisibility) {
// Posix case
return false;
diff --git a/clang/test/SemaCXX/unique_object_duplication.h
b/clang/test/SemaCXX/unique_object_duplication.h
index bd0ee6bd14d64..cac2e3378ae70 100644
--- a/clang/test/SemaCXX/unique_object_duplication.h
+++ b/clang/test/SemaCXX/unique_object_duplication.h
@@ -173,10 +173,6 @@ namespace GlobalTest {
// Always visible
VISIBLE static inline float allowedStaticMember2 = 0.0;
-
- // Tests here are sparse because the AddrTest case below will define plenty
- // more, which aren't problematic to define (because they're immutable),
but
- // may still cause problems if their address is taken.
};
inline float Test::disallowedStaticMember2 = 2.3; // hidden-warning
{{'disallowedStaticMember2' may be duplicated when built into a shared library:
it is mutable, with external linkage and hidden visibility}}
@@ -211,3 +207,44 @@ inline int allowedTemplate2 = 0;
template int allowedTemplate2<int>;
} // namespace TemplateTest
+
+/******************************************************************************
+ * Case four: Nested classes
+
******************************************************************************/
+
+namespace NestedClassTest {
+// Unlike visibility, declexport annotations do not propagate down to nested
+// classes. Hence on windows, we only get warnings for class members if their
+// immediate class is annotated. On posix, we get warnings if any containing
+// class is annotated.
+
+class VISIBLE Outer {
+ // Visible because their class is exported
+ inline static int allowedOuterMember = 0;
+ int* allowedOuterFunction() {
+ static int allowed = 0;
+ return &allowed;
+ }
+
+ // No annotation, and visibility is only propagated on posix.
+ class HiddenOnWindows {
+ inline static int disallowedInnerMember = 0; // windows-warning
{{'disallowedInnerMember' may be duplicated when built into a shared library:
it is mutable, with external linkage and no import/export annotation}}
+
+
+ int* disallowedInnerFunction() {
+ static int disallowed = 0; // windows-warning {{'disallowed' may be
duplicated when built into a shared library: it is mutable, with external
linkage and no import/export annotation}}
+ return &disallowed;
+ }
+ };
+
+ class VISIBLE AlwaysVisible {
+ inline static int allowedInnerMember = 0;
+
+ int* allowedInnerFunction() {
+ static int allowed = 0;
+ return &allowed;
+ }
+ };
+};
+
+}
\ No newline at end of file
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
