Skip to content

Commit 5ab3114

Browse files
authored
Expand annotation check for -Wunique-object-duplication on Windows. (#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.
1 parent c00c5a3 commit 5ab3114

File tree

2 files changed

+47
-12
lines changed

2 files changed

+47
-12
lines changed

clang/lib/Sema/SemaDecl.cpp

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13531,21 +13531,19 @@ bool Sema::GloballyUniqueObjectMightBeAccidentallyDuplicated(
1353113531

1353213532
// The target is "hidden" (from the dynamic linker) if:
1353313533
// 1. On posix, it has hidden visibility, or
13534-
// 2. On windows, it has no import/export annotation
13534+
// 2. On windows, it has no import/export annotation, and neither does the
13535+
// class which directly contains it.
1353513536
if (Context.getTargetInfo().shouldDLLImportComdatSymbols()) {
1353613537
if (Target->hasAttr<DLLExportAttr>() || Target->hasAttr<DLLImportAttr>())
1353713538
return false;
1353813539

1353913540
// If the variable isn't directly annotated, check to see if it's a member
1354013541
// of an annotated class.
13541-
const VarDecl *VD = dyn_cast<VarDecl>(Target);
13542+
const CXXRecordDecl *Ctx =
13543+
dyn_cast<CXXRecordDecl>(Target->getDeclContext());
13544+
if (Ctx && (Ctx->hasAttr<DLLExportAttr>() || Ctx->hasAttr<DLLImportAttr>()))
13545+
return false;
1354213546

13543-
if (VD && VD->isStaticDataMember()) {
13544-
const CXXRecordDecl *Ctx = dyn_cast<CXXRecordDecl>(VD->getDeclContext());
13545-
if (Ctx &&
13546-
(Ctx->hasAttr<DLLExportAttr>() || Ctx->hasAttr<DLLImportAttr>()))
13547-
return false;
13548-
}
1354913547
} else if (Lnk.getVisibility() != HiddenVisibility) {
1355013548
// Posix case
1355113549
return false;

clang/test/SemaCXX/unique_object_duplication.h

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -173,10 +173,6 @@ namespace GlobalTest {
173173

174174
// Always visible
175175
VISIBLE static inline float allowedStaticMember2 = 0.0;
176-
177-
// Tests here are sparse because the AddrTest case below will define plenty
178-
// more, which aren't problematic to define (because they're immutable), but
179-
// may still cause problems if their address is taken.
180176
};
181177

182178
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;
211207
template int allowedTemplate2<int>;
212208

213209
} // namespace TemplateTest
210+
211+
/******************************************************************************
212+
* Case four: Nested classes
213+
******************************************************************************/
214+
215+
namespace NestedClassTest {
216+
// Unlike visibility, declexport annotations do not propagate down to nested
217+
// classes. Hence on windows, we only avoid duplication of class members if
218+
// their immediate containing class is annotated. On posix, we get avoid
219+
// duplication if any containing class is annotated.
220+
221+
class VISIBLE Outer {
222+
// Visible because their class is exported
223+
inline static int allowedOuterMember = 0;
224+
int* allowedOuterFunction() {
225+
static int allowed = 0;
226+
return &allowed;
227+
}
228+
229+
// No annotation, and visibility is only propagated on posix.
230+
class HiddenOnWindows {
231+
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}}
232+
233+
234+
int* disallowedInnerFunction() {
235+
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}}
236+
return &disallowed;
237+
}
238+
};
239+
240+
class VISIBLE AlwaysVisible {
241+
inline static int allowedInnerMember = 0;
242+
243+
int* allowedInnerFunction() {
244+
static int allowed = 0;
245+
return &allowed;
246+
}
247+
};
248+
};
249+
250+
}

0 commit comments

Comments
 (0)