mirror of
https://github.com/llvm/llvm-project.git
synced 2025-04-27 14:46:10 +00:00
Revert "Warn when unique objects might be duplicated in shared libraries (#117622)"
There are some buildbot breakages, see the PR. Reverting for now. This reverts commit d5684b8402d2175e80a2792db58e9a8e960a8544.
This commit is contained in:
parent
d03b5de71c
commit
cdeeb390a9
@ -113,10 +113,6 @@ Attribute Changes in Clang
|
||||
Improvements to Clang's diagnostics
|
||||
-----------------------------------
|
||||
|
||||
- The ``-Wunique-object-duplication`` warning has been added to warn about objects
|
||||
which are supposed to only exist once per program, but may get duplicated when
|
||||
built into a shared library.
|
||||
|
||||
Improvements to Clang's time-trace
|
||||
----------------------------------
|
||||
|
||||
|
@ -694,36 +694,6 @@ def SuspiciousMemaccess : DiagGroup<"suspicious-memaccess",
|
||||
NonTrivialMemaccess, MemsetTransposedArgs, SuspiciousBzero]>;
|
||||
def StaticInInline : DiagGroup<"static-in-inline">;
|
||||
def StaticLocalInInline : DiagGroup<"static-local-in-inline">;
|
||||
def UniqueObjectDuplication : DiagGroup<"unique-object-duplication"> {
|
||||
code Documentation = [{
|
||||
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
|
||||
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
|
||||
once per copy, etc.
|
||||
|
||||
Specifically, this warning fires when it detects an object which:
|
||||
1. Appears 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.
|
||||
|
||||
As well as one of the following:
|
||||
1. The object is mutable, or
|
||||
2. The object's initializer definitely has side effects.
|
||||
|
||||
The warning is best resolved by making the object ``const`` (if possible), or by explicitly
|
||||
giving the object non-hidden visibility, e.g. using ``__attribute((visibility("default")))``.
|
||||
Note that all levels of a pointer variable must be constant; ``const int*`` will
|
||||
trigger the warning because the pointer itself is mutable.
|
||||
|
||||
This warning is currently disabled on Windows since it uses import/export rules
|
||||
instead of visibility.
|
||||
}];
|
||||
}
|
||||
|
||||
def GNUStaticFloatInit : DiagGroup<"gnu-static-float-init">;
|
||||
def StaticFloatInit : DiagGroup<"static-float-init", [GNUStaticFloatInit]>;
|
||||
// Allow differentiation between GNU statement expressions in a macro versus
|
||||
|
@ -6167,15 +6167,6 @@ 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<
|
||||
"initializeation of %0 may run twice when built into a shared library: "
|
||||
"it has hidden visibility and external linkage">,
|
||||
InGroup<UniqueObjectDuplication>, DefaultIgnore;
|
||||
|
||||
def ext_redefinition_of_typedef : ExtWarn<
|
||||
"redefinition of typedef %0 is a C11 feature">,
|
||||
InGroup<DiagGroup<"typedef-redefinition"> >;
|
||||
|
@ -3664,12 +3664,6 @@ public:
|
||||
NonTrivialCUnionContext UseContext,
|
||||
unsigned NonTrivialKind);
|
||||
|
||||
/// Certain globally-unique variables might be accidentally duplicated if
|
||||
/// built into multiple shared libraries with hidden visibility. This can
|
||||
/// cause problems if the variable is mutable, its initialization is
|
||||
/// effectful, or its address is taken.
|
||||
bool GloballyUniqueObjectMightBeAccidentallyDuplicated(const VarDecl *Dcl);
|
||||
|
||||
/// AddInitializerToDecl - Adds the initializer Init to the
|
||||
/// declaration dcl. If DirectInit is true, this is C++ direct
|
||||
/// initialization rather than copy initialization.
|
||||
|
@ -13380,62 +13380,6 @@ void Sema::checkNonTrivialCUnion(QualType QT, SourceLocation Loc,
|
||||
.visit(QT, nullptr, false);
|
||||
}
|
||||
|
||||
bool Sema::GloballyUniqueObjectMightBeAccidentallyDuplicated(
|
||||
const VarDecl *Dcl) {
|
||||
if (!getLangOpts().CPlusPlus)
|
||||
return false;
|
||||
|
||||
// We only need to warn if the definition is in a header file, so wait to
|
||||
// diagnose until we've seen the definition.
|
||||
if (!Dcl->isThisDeclarationADefinition())
|
||||
return false;
|
||||
|
||||
// If an object is defined in a source file, its definition can't get
|
||||
// duplicated since it will never appear in more than one TU.
|
||||
if (Dcl->getASTContext().getSourceManager().isInMainFile(Dcl->getLocation()))
|
||||
return false;
|
||||
|
||||
// If the variable we're looking at is a static local, then we actually care
|
||||
// about the properties of the function containing it.
|
||||
const ValueDecl *Target = Dcl;
|
||||
// VarDecls and FunctionDecls have different functions for checking
|
||||
// inline-ness, so we have to do it manually.
|
||||
bool TargetIsInline = Dcl->isInline();
|
||||
|
||||
// Update the Target and TargetIsInline property if necessary
|
||||
if (Dcl->isStaticLocal()) {
|
||||
const DeclContext *Ctx = Dcl->getDeclContext();
|
||||
if (!Ctx)
|
||||
return false;
|
||||
|
||||
const FunctionDecl *FunDcl =
|
||||
dyn_cast_if_present<FunctionDecl>(Ctx->getNonClosureAncestor());
|
||||
if (!FunDcl)
|
||||
return false;
|
||||
|
||||
Target = FunDcl;
|
||||
// IsInlined() checks for the C++ inline property
|
||||
TargetIsInline = FunDcl->isInlined();
|
||||
}
|
||||
|
||||
// Non-inline variables can only legally appear in one TU
|
||||
// FIXME: This also applies to templated variables, but that can rarely lead
|
||||
// to false positives so templates are disabled for now.
|
||||
if (!TargetIsInline)
|
||||
return false;
|
||||
|
||||
// If the object isn't hidden, the dynamic linker will prevent duplication.
|
||||
clang::LinkageInfo Lnk = Target->getLinkageAndVisibility();
|
||||
if (Lnk.getVisibility() != HiddenVisibility)
|
||||
return false;
|
||||
|
||||
// If the obj doesn't have external linkage, it's supposed to be duplicated.
|
||||
if (!isExternalFormalLinkage(Lnk.getLinkage()))
|
||||
return false;
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) {
|
||||
// If there is no declaration, there was an error parsing it. Just ignore
|
||||
// the initializer.
|
||||
@ -14842,51 +14786,6 @@ void Sema::FinalizeDeclaration(Decl *ThisDecl) {
|
||||
if (DC->getRedeclContext()->isFileContext() && VD->isExternallyVisible())
|
||||
AddPushedVisibilityAttribute(VD);
|
||||
|
||||
// If this object has external linkage and hidden visibility, it might be
|
||||
// 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.
|
||||
// FIXME: Checking templates can cause false positives if the template in
|
||||
// question is never instantiated (e.g. only specialized templates are used).
|
||||
if (!Context.getTargetInfo().shouldDLLImportComdatSymbols() &&
|
||||
!VD->isTemplated() &&
|
||||
GloballyUniqueObjectMightBeAccidentallyDuplicated(VD)) {
|
||||
// Check mutability. For pointers, ensure that both the pointer and the
|
||||
// pointee are (recursively) const.
|
||||
QualType Type = VD->getType().getNonReferenceType();
|
||||
if (!Type.isConstant(VD->getASTContext())) {
|
||||
Diag(VD->getLocation(), diag::warn_possible_object_duplication_mutable)
|
||||
<< VD;
|
||||
} else {
|
||||
while (Type->isPointerType()) {
|
||||
Type = Type->getPointeeType();
|
||||
if (Type->isFunctionType())
|
||||
break;
|
||||
if (!Type.isConstant(VD->getASTContext())) {
|
||||
Diag(VD->getLocation(),
|
||||
diag::warn_possible_object_duplication_mutable)
|
||||
<< VD;
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// To keep false positives low, only warn if we're certain that the
|
||||
// initializer has side effects. Don't warn on operator new, since a mutable
|
||||
// pointer will trigger the previous warning, and an immutable pointer
|
||||
// getting duplicated just results in a little extra memory usage.
|
||||
const Expr *Init = VD->getAnyInitializer();
|
||||
if (Init &&
|
||||
Init->HasSideEffects(VD->getASTContext(),
|
||||
/*IncludePossibleEffects=*/false) &&
|
||||
!isa<CXXNewExpr>(Init->IgnoreParenImpCasts())) {
|
||||
Diag(Init->getExprLoc(), diag::warn_possible_object_duplication_init)
|
||||
<< VD;
|
||||
}
|
||||
}
|
||||
|
||||
// FIXME: Warn on unused var template partial specializations.
|
||||
if (VD->isFileVarDecl() && !isa<VarTemplatePartialSpecializationDecl>(VD))
|
||||
MarkUnusedFileScopedDecl(VD);
|
||||
|
@ -1,16 +0,0 @@
|
||||
// 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. The test should fail because we're not getting the expected warnings.
|
||||
// XFAIL: target={{.*}}-windows{{.*}}
|
||||
|
||||
#include "unique_object_duplication.h"
|
||||
|
||||
// Everything in these namespaces here is defined in the cpp file,
|
||||
// so won't get duplicated
|
||||
|
||||
namespace GlobalTest {
|
||||
float Test::allowedStaticMember1 = 2.3;
|
||||
}
|
||||
|
||||
bool disallowed4 = true;
|
||||
constexpr inline bool disallowed5 = true;
|
@ -1,157 +0,0 @@
|
||||
/**
|
||||
* This file contains tests for the -Wunique_object_duplication warning.
|
||||
* See the warning's documentation for more information.
|
||||
*/
|
||||
|
||||
#define HIDDEN __attribute__((visibility("hidden")))
|
||||
#define DEFAULT __attribute__((visibility("default")))
|
||||
|
||||
// Helper functions
|
||||
constexpr int init_constexpr(int x) { return x; };
|
||||
extern double init_dynamic(int);
|
||||
|
||||
/******************************************************************************
|
||||
* Case one: Static local variables in an externally-visible function
|
||||
******************************************************************************/
|
||||
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}}
|
||||
// Initialization might run more than once
|
||||
static const double disallowedStatic2 = disallowedStatic1++; // hidden-warning {{initializeation of 'disallowedStatic2' may run twice when built into a shared library: it has hidden visibility and external linkage}}
|
||||
|
||||
// OK, because immutable and compile-time-initialized
|
||||
static constexpr int allowedStatic1 = 0;
|
||||
static const float allowedStatic2 = 1;
|
||||
static constexpr int allowedStatic3 = init_constexpr(2);
|
||||
static const int allowedStatic4 = init_constexpr(3);
|
||||
}
|
||||
|
||||
// Don't warn for non-inline functions, since they can't (legally) appear
|
||||
// in more than one TU in the first place.
|
||||
void has_static_locals_non_inline() {
|
||||
// Mutable
|
||||
static int allowedStatic1 = 0;
|
||||
// Initialization might run more than once
|
||||
static const double allowedStatic2 = allowedStatic1++;
|
||||
}
|
||||
|
||||
// Everything in this function is OK because the function is TU-local
|
||||
static void has_static_locals_internal() {
|
||||
static int allowedStatic1 = 0;
|
||||
static double allowedStatic2 = init_dynamic(2);
|
||||
static char allowedStatic3 = []() { return allowedStatic1++; }();
|
||||
static constexpr int allowedStatic4 = 0;
|
||||
}
|
||||
|
||||
namespace {
|
||||
|
||||
// Everything in this function is OK because the function is also TU-local
|
||||
void has_static_locals_anon() {
|
||||
static int allowedStatic1 = 0;
|
||||
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 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}}
|
||||
}
|
||||
|
||||
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}}
|
||||
};
|
||||
}
|
||||
|
||||
DEFAULT void static_local_never_hidden() {
|
||||
static int allowedStatic1 = 3;
|
||||
|
||||
{
|
||||
static int allowedStatic2 = 3;
|
||||
}
|
||||
|
||||
auto lmb = []() {
|
||||
static int allowedStatic3 = 3;
|
||||
};
|
||||
}
|
||||
|
||||
// Don't warn on this because it's not in a function
|
||||
const int setByLambda = ([]() { static int x = 3; return x++; })();
|
||||
|
||||
inline void has_extern_local() {
|
||||
extern int allowedAddressExtern; // Not a definition
|
||||
}
|
||||
|
||||
inline void has_regular_local() {
|
||||
int allowedAddressLocal = 0;
|
||||
}
|
||||
|
||||
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}}
|
||||
}
|
||||
|
||||
} // namespace StaticLocalTest
|
||||
|
||||
/******************************************************************************
|
||||
* Case two: Globals with external linkage
|
||||
******************************************************************************/
|
||||
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 {{initializeation of 'disallowedGlobal5' may run twice when built into a shared library: it has hidden visibility and external linkage}}
|
||||
|
||||
// OK because internal linkage, so duplication is intended
|
||||
static float allowedGlobal1 = 3.14;
|
||||
const double allowedGlobal2 = init_dynamic(2);
|
||||
static const char allowedGlobal3 = []() { return disallowedGlobal1++; }();
|
||||
static inline double allowedGlobal4 = init_dynamic(2);
|
||||
|
||||
// OK, because immutable and compile-time-initialized
|
||||
constexpr int allowedGlobal5 = 0;
|
||||
const float allowedGlobal6 = 1;
|
||||
constexpr int allowedGlobal7 = init_constexpr(2);
|
||||
const int allowedGlobal8 = init_constexpr(3);
|
||||
|
||||
// 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}}
|
||||
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 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}}
|
||||
|
||||
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}}
|
||||
// Defined below, in the header file
|
||||
static float disallowedStaticMember2;
|
||||
// Defined in the cpp file, so won't get duplicated
|
||||
static float allowedStaticMember1;
|
||||
|
||||
// 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}}
|
||||
} // namespace GlobalTest
|
Loading…
x
Reference in New Issue
Block a user