intel/llvm
1
0
Fork 0
mirror of https://github.com/intel/llvm.git synced 2026-01-14 03:50:17 +08:00
Code Issues Packages Projects Releases Wiki Activity

Enable unique-object-duplication warning for windows (#143537)

Browse Source 
Followup to #125526. This expands the logic of the
unique-object-duplication warning so that it also works for windows
code.

For the most part, the logic is unchanged, merely substituting "has no
import/export annotation" in place of "has hidden visibility". However,
there are some small inconsistencies between the two; namely, visibility
is propagated through nested classes, while import/export annotations
aren't.

This PR:
1. Updates the logic for the warning to account for the differences
between posix and windows
2. Changes the warning message and documentation appropriately
3. Updates the tests to cover windows, and adds new test cases for the
places where behavior differs.

This PR was tested by building chromium (cross compiling linux->windows)
with the changes in place. After accounting for the differences in
semantics, no new warnings were discovered.
This commit is contained in:
Devon Loehr
2025-06-13 10:29:42 -04:00
committed by GitHub
parent 82911f188b
commit 9670e09d0e
5 changed files with 114 additions and 54 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
clang/include/clang/Basic/DiagnosticGroups.td
View File

@@ -806,7 +806,9 @@ def UniqueObjectDuplication : DiagGroup<"unique-object-duplication"> {
Warns when objects which are supposed to be globally unique might get duplicated
when built into a shared library.
If an object with hidden visibility is built into a shared library, each instance
This can occur to objects which are hidden from the dynamic linker, due to
having hidden visibility (on posix) or lacking a dllimport/dllexport attribute
(on windows). If such an object is built into a shared library, each instance
of the library will get its own copy. This can cause very subtle bugs if there was
only supposed to be one copy of the object in question: singletons aren't single,
changes to one object won't affect the others, the object's initializer will run
@@ -815,7 +817,7 @@ once per copy, etc.
Specifically, this warning fires when it detects an object which:
1. Is defined as ``inline`` in a header file (so it might get compiled into multiple libaries), and
2. Has external linkage (otherwise it's supposed to be duplicated), and
3. Has hidden visibility.
3. Has hidden visibility (posix) or lacks a dllimport/dllexport attribute (windows).
As well as one of the following:
1. The object is mutable, or
@@ -825,13 +827,15 @@ The warning can be resolved by removing one of the conditions above. In rough
order of preference, this may be done by:
1. Marking the object ``const`` (if possible)
2. Moving the object's definition to a source file
3. Giving the object non-hidden visibility, e.g. using ``__attribute((visibility("default")))``.
3. Making the object visible using ``__attribute((visibility("default")))``,
``__declspec(dllimport)``, or ``__declspec(dllexport)``.
When annotating an object with ``__declspec(dllimport)`` or ``__declspec(dllexport)``,
take care to ensure that the object is only exported from one dll, and is imported
everywhere else.
Note that for (2), all levels of a pointer variable must be constant;
``const int*`` will trigger the warning because the pointer itself is mutable.
This warning is not yet implemented for Windows, since Windows uses
import/export rules instead of visibility.
}];
}

21
clang/include/clang/Basic/DiagnosticSemaKinds.td
View File

@@ -6267,14 +6267,19 @@ def warn_static_local_in_extern_inline : Warning<
def note_convert_inline_to_static : Note<
"use 'static' to give inline function %0 internal linkage">;
def warn_possible_object_duplication_mutable : Warning<
"%0 may be duplicated when built into a shared library: "
"it is mutable, has hidden visibility, and external linkage">,
InGroup<UniqueObjectDuplication>, DefaultIgnore;
def warn_possible_object_duplication_init : Warning<
"initialization of %0 may run twice when built into a shared library: "
"it has hidden visibility and external linkage">,
InGroup<UniqueObjectDuplication>, DefaultIgnore;
def warn_possible_object_duplication_mutable
: Warning<"%0 may be duplicated when built into a shared library: "
"it is mutable, with external linkage and "
"%select{hidden visibility|no import/export annotation}1">,
InGroup<UniqueObjectDuplication>,
DefaultIgnore;
def warn_possible_object_duplication_init
: Warning<"initialization of %0 may run twice when built into a shared "
"library: "
"it has external linkage and "
"%select{hidden visibility|no import/export annotation}1">,
InGroup<UniqueObjectDuplication>,
DefaultIgnore;
def ext_redefinition_of_typedef : ExtWarn<
"redefinition of typedef %0 is a C11 feature">,

31
clang/lib/Sema/SemaDecl.cpp
View File

@@ -13518,8 +13518,28 @@ bool Sema::GloballyUniqueObjectMightBeAccidentallyDuplicated(
// If the object isn't hidden, the dynamic linker will prevent duplication.
clang::LinkageInfo Lnk = Target->getLinkageAndVisibility();
if (Lnk.getVisibility() != HiddenVisibility)
// 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
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);
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;
}
// If the obj doesn't have external linkage, it's supposed to be duplicated.
if (!isExternalFormalLinkage(Lnk.getLinkage()))
@@ -13550,19 +13570,16 @@ void Sema::DiagnoseUniqueObjectDuplication(const VarDecl *VD) {
// duplicated when built into a shared library, which causes problems if it's
// mutable (since the copies won't be in sync) or its initialization has side
// effects (since it will run once per copy instead of once globally).
// FIXME: Windows uses dllexport/dllimport instead of visibility, and we don't
// handle that yet. Disable the warning on Windows for now.
// Don't diagnose if we're inside a template, because it's not practical to
// fix the warning in most cases.
if (!Context.getTargetInfo().shouldDLLImportComdatSymbols() &&
!VD->isTemplated() &&
if (!VD->isTemplated() &&
GloballyUniqueObjectMightBeAccidentallyDuplicated(VD)) {
QualType Type = VD->getType();
if (looksMutable(Type, VD->getASTContext())) {
Diag(VD->getLocation(), diag::warn_possible_object_duplication_mutable)
<< VD;
<< VD << Context.getTargetInfo().shouldDLLImportComdatSymbols();
}
// To keep false positives low, only warn if we're certain that the
@@ -13575,7 +13592,7 @@ void Sema::DiagnoseUniqueObjectDuplication(const VarDecl *VD) {
/*IncludePossibleEffects=*/false) &&
!isa<CXXNewExpr>(Init->IgnoreParenImpCasts())) {
Diag(Init->getExprLoc(), diag::warn_possible_object_duplication_init)
<< VD;
<< VD << Context.getTargetInfo().shouldDLLImportComdatSymbols();
}
}
}

10
clang/test/SemaCXX/unique_object_duplication.cpp
View File

@@ -1,7 +1,9 @@
// RUN: %clang_cc1 -fsyntax-only -verify=hidden -Wunique-object-duplication -fvisibility=hidden -Wno-unused-value %s
// RUN: %clang_cc1 -fsyntax-only -verify -Wunique-object-duplication -Wno-unused-value %s
// The check is currently disabled on windows in MSVC-like environments. The test should fail because we're not getting the expected warnings.
// XFAIL: target={{.*}}-windows-msvc, {{.*}}-ps{{(4|5)(-.+)?}}
// RUN: %clang_cc1 -fsyntax-only -Wunique-object-duplication -Wno-unused-value \
// RUN: -verify -triple=x86_64-pc-linux-gnu %s
// RUN: %clang_cc1 -fsyntax-only -Wunique-object-duplication -Wno-unused-value \
// RUN: -verify=hidden -triple=x86_64-pc-linux-gnu -fvisibility=hidden %s
// RUN: %clang_cc1 -fsyntax-only -Wunique-object-duplication -Wno-unused-value \
// RUN: -verify=windows -triple=x86_64-windows-msvc -DWINDOWS_TEST -fdeclspec %s
#include "unique_object_duplication.h"

90
clang/test/SemaCXX/unique_object_duplication.h
View File

@@ -3,8 +3,14 @@
* See the warning's documentation for more information.
*/
#ifdef WINDOWS_TEST
#define HIDDEN
// dllimport also suffices for visibility, but those can't have definitions
#define VISIBLE __declspec(dllexport)
#else
#define HIDDEN __attribute__((visibility("hidden")))
#define DEFAULT __attribute__((visibility("default")))
#define VISIBLE __attribute__((visibility("default")))
#endif
// Helper functions
constexpr int init_constexpr(int x) { return x; };
@@ -17,10 +23,11 @@ namespace StaticLocalTest {
inline void has_static_locals_external() {
// Mutable
static int disallowedStatic1 = 0; // hidden-warning {{'disallowedStatic1' may be duplicated when built into a shared library: it is mutable, has hidden visibility, and external linkage}}
static int disallowedStatic1 = 0; // hidden-warning {{'disallowedStatic1' may be duplicated when built into a shared library: it is mutable, with external linkage and hidden visibility}}
// windows-warning@-1 {{'disallowedStatic1' may be duplicated when built into a shared library: it is mutable, with external linkage and no import/export annotation}}
// Initialization might run more than once
static const double disallowedStatic2 = disallowedStatic1++; // hidden-warning {{initialization of 'disallowedStatic2' may run twice when built into a shared library: it has hidden visibility and external linkage}}
static const double disallowedStatic2 = disallowedStatic1++; // hidden-warning {{initialization of 'disallowedStatic2' may run twice when built into a shared library: it has external linkage and hidden visibility}}
// windows-warning@-1 {{initialization of 'disallowedStatic2' may run twice when built into a shared library: it has external linkage and no import/export annotation}}
// OK, because immutable and compile-time-initialized
static constexpr int allowedStatic1 = 0;
static const float allowedStatic2 = 1;
@@ -53,29 +60,33 @@ void has_static_locals_anon() {
static double allowedStatic2 = init_dynamic(2);
static char allowedStatic3 = []() { return allowedStatic1++; }();
static constexpr int allowedStatic4 = init_constexpr(3);
}
}
} // Anonymous namespace
HIDDEN inline void static_local_always_hidden() {
static int disallowedStatic1 = 3; // hidden-warning {{'disallowedStatic1' may be duplicated when built into a shared library: it is mutable, has hidden visibility, and external linkage}}
// expected-warning@-1 {{'disallowedStatic1' may be duplicated when built into a shared library: it is mutable, has hidden visibility, and external linkage}}
static int disallowedStatic1 = 3; // hidden-warning {{'disallowedStatic1' may be duplicated when built into a shared library: it is mutable, with external linkage and hidden visibility}}
// expected-warning@-1 {{'disallowedStatic1' may be duplicated when built into a shared library: it is mutable, with external linkage and hidden visibility}}
// windows-warning@-2 {{'disallowedStatic1' may be duplicated when built into a shared library: it is mutable, with external linkage and no import/export annotation}}
{
static int disallowedStatic2 = 3; // hidden-warning {{'disallowedStatic2' may be duplicated when built into a shared library: it is mutable, has hidden visibility, and external linkage}}
// expected-warning@-1 {{'disallowedStatic2' may be duplicated when built into a shared library: it is mutable, has hidden visibility, and external linkage}}
static int disallowedStatic2 = 3; // hidden-warning {{'disallowedStatic2' may be duplicated when built into a shared library: it is mutable, with external linkage and hidden visibility}}
// expected-warning@-1 {{'disallowedStatic2' may be duplicated when built into a shared library: it is mutable, with external linkage and hidden visibility}}
// windows-warning@-2 {{'disallowedStatic2' may be duplicated when built into a shared library: it is mutable, with external linkage and no import/export annotation}}
}
auto lmb = []() {
static int disallowedStatic3 = 3; // hidden-warning {{'disallowedStatic3' may be duplicated when built into a shared library: it is mutable, has hidden visibility, and external linkage}}
// expected-warning@-1 {{'disallowedStatic3' may be duplicated when built into a shared library: it is mutable, has hidden visibility, and external linkage}}
static int disallowedStatic3 = 3; // hidden-warning {{'disallowedStatic3' may be duplicated when built into a shared library: it is mutable, with external linkage and hidden visibility}}
// expected-warning@-1 {{'disallowedStatic3' may be duplicated when built into a shared library: it is mutable, with external linkage and hidden visibility}}
// windows-warning@-2 {{'disallowedStatic3' may be duplicated when built into a shared library: it is mutable, with external linkage and no import/export annotation}}
};
}
DEFAULT void static_local_never_hidden() {
static int allowedStatic1 = 3;
// Always visible
VISIBLE void static_local_never_hidden() {
static int allowedStatic1 = 3;
{
static int allowedStatic2 = 3;
static int allowedStatic2 = 3;
}
auto lmb = []() {
@@ -96,7 +107,8 @@ inline void has_regular_local() {
inline void has_thread_local() {
// thread_local variables are static by default
thread_local int disallowedThreadLocal = 0; // hidden-warning {{'disallowedThreadLocal' may be duplicated when built into a shared library: it is mutable, has hidden visibility, and external linkage}}
thread_local int disallowedThreadLocal = 0; // hidden-warning {{'disallowedThreadLocal' may be duplicated when built into a shared library: it is mutable, with external linkage and hidden visibility}}
// windows-warning@-1 {{'disallowedThreadLocal' may be duplicated when built into a shared library: it is mutable, with external linkage and no import/export annotation}}
}
// Functions themselves are always immutable, so referencing them is okay
@@ -109,11 +121,13 @@ inline auto& allowedFunctionReference = has_static_locals_external;
******************************************************************************/
namespace GlobalTest {
// Mutable
inline float disallowedGlobal1 = 3.14; // hidden-warning {{'disallowedGlobal1' may be duplicated when built into a shared library: it is mutable, has hidden visibility, and external linkage}}
// Initialization might run more than once
inline const double disallowedGlobal5 = disallowedGlobal1++; // hidden-warning {{initialization of 'disallowedGlobal5' may run twice when built into a shared library: it has hidden visibility and external linkage}}
inline float disallowedGlobal1 = 3.14; // hidden-warning {{'disallowedGlobal1' may be duplicated when built into a shared library: it is mutable, with external linkage and hidden visibility}}
// windows-warning@-1 {{'disallowedGlobal1' may be duplicated when built into a shared library: it is mutable, with external linkage and no import/export annotation}}
// Initialization might run more than once
inline const double disallowedGlobal5 = disallowedGlobal1++; // hidden-warning {{initialization of 'disallowedGlobal5' may run twice when built into a shared library: it has external linkage and hidden visibility}}
// windows-warning@-1 {{initialization of 'disallowedGlobal5' may run twice when built into a shared library: it has external linkage and no import/export annotation}}
// OK because internal linkage, so duplication is intended
static float allowedGlobal1 = 3.14;
const double allowedGlobal2 = init_dynamic(2);
@@ -129,34 +143,52 @@ namespace GlobalTest {
// We don't warn on this because non-inline variables can't (legally) appear
// in more than one TU.
float allowedGlobal9 = 3.14;
// Pointers need to be double-const-qualified
inline float& nonConstReference = disallowedGlobal1; // hidden-warning {{'nonConstReference' may be duplicated when built into a shared library: it is mutable, has hidden visibility, and external linkage}}
inline float& nonConstReference = disallowedGlobal1; // hidden-warning {{'nonConstReference' may be duplicated when built into a shared library: it is mutable, with external linkage and hidden visibility}}
// windows-warning@-1 {{'nonConstReference' may be duplicated when built into a shared library: it is mutable, with external linkage and no import/export annotation}}
const inline int& constReference = allowedGlobal5;
inline int* nonConstPointerToNonConst = nullptr; // hidden-warning {{'nonConstPointerToNonConst' may be duplicated when built into a shared library: it is mutable, has hidden visibility, and external linkage}}
inline int const* nonConstPointerToConst = nullptr; // hidden-warning {{'nonConstPointerToConst' may be duplicated when built into a shared library: it is mutable, has hidden visibility, and external linkage}}
inline int* const constPointerToNonConst = nullptr; // hidden-warning {{'constPointerToNonConst' may be duplicated when built into a shared library: it is mutable, has hidden visibility, and external linkage}}
inline int* nonConstPointerToNonConst = nullptr; // hidden-warning {{'nonConstPointerToNonConst' may be duplicated when built into a shared library: it is mutable, with external linkage and hidden visibility}}
// windows-warning@-1 {{'nonConstPointerToNonConst' may be duplicated when built into a shared library: it is mutable, with external linkage and no import/export annotation}}
inline int const* nonConstPointerToConst = nullptr; // hidden-warning {{'nonConstPointerToConst' may be duplicated when built into a shared library: it is mutable, with external linkage and hidden visibility}}
// windows-warning@-1 {{'nonConstPointerToConst' may be duplicated when built into a shared library: it is mutable, with external linkage and no import/export annotation}}
inline int* const constPointerToNonConst = nullptr; // hidden-warning {{'constPointerToNonConst' may be duplicated when built into a shared library: it is mutable, with external linkage and hidden visibility}}
// windows-warning@-1 {{'constPointerToNonConst' may be duplicated when built into a shared library: it is mutable, with external linkage and no import/export annotation}}
inline int const* const constPointerToConst = nullptr;
// Don't warn on new because it tends to generate false positives
inline int const* const constPointerToConstNew = new int(7);
inline int const * const * const * const nestedConstPointer = nullptr;
inline int const * const ** const * const nestedNonConstPointer = nullptr; // hidden-warning {{'nestedNonConstPointer' may be duplicated when built into a shared library: it is mutable, has hidden visibility, and external linkage}}
inline int const * const ** const * const nestedNonConstPointer = nullptr; // hidden-warning {{'nestedNonConstPointer' may be duplicated when built into a shared library: it is mutable, with external linkage and hidden visibility}}
// windows-warning@-1 {{'nestedNonConstPointer' may be duplicated when built into a shared library: it is mutable, with external linkage and no import/export annotation}}
struct Test {
static inline float disallowedStaticMember1; // hidden-warning {{'disallowedStaticMember1' may be duplicated when built into a shared library: it is mutable, has hidden visibility, and external linkage}}
static inline float disallowedStaticMember1; // hidden-warning {{'disallowedStaticMember1' may be duplicated when built into a shared library: it is mutable, with external linkage and hidden visibility}}
// windows-warning@-1 {{'disallowedStaticMember1' may be duplicated when built into a shared library: it is mutable, with external linkage and no import/export annotation}}
// Defined below, in the header file
static float disallowedStaticMember2;
static float disallowedStaticMember2;
// Defined in the cpp file, so won't get duplicated
static float allowedStaticMember1;
// 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, has hidden visibility, and external linkage}}
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}}
// windows-warning@-1 {{'disallowedStaticMember2' may be duplicated when built into a shared library: it is mutable, with external linkage and no import/export annotation}}
// This is always visible, so nothing inside it will get duplicated
struct VISIBLE NeverHidden {
static inline float allowedStaticMember3;
static float allowedStaticMember4;
};
inline float NeverHidden::allowedStaticMember4 = 3.4;
} // namespace GlobalTest
/******************************************************************************
@@ -165,7 +197,7 @@ namespace GlobalTest {
namespace TemplateTest {
// We never warn inside templates because it's frequently infeasible to actually
// We never warn inside templates because it's usually infeasible to actually
// fix the warning.
template <typename T>