diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 1870d1271c556..a501b901862b6 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -3669,6 +3669,7 @@ class Sema final : public SemaBase { /// cause problems if the variable is mutable, its initialization is /// effectful, or its address is taken. bool GloballyUniqueObjectMightBeAccidentallyDuplicated(const VarDecl *Dcl); + void DiagnoseUniqueObjectDuplication(const VarDecl *Dcl); /// AddInitializerToDecl - Adds the initializer Init to the /// declaration dcl. If DirectInit is true, this is C++ direct diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 74e0fcec2d911..4ed80327291ff 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -13399,8 +13399,11 @@ bool Sema::GloballyUniqueObjectMightBeAccidentallyDuplicated( // 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. + // inline-ness, and whether they were originally templated, so we have to + // call the appropriate functions manually. bool TargetIsInline = Dcl->isInline(); + bool TargetWasTemplated = + Dcl->getTemplateSpecializationKind() != TSK_Undeclared; // Update the Target and TargetIsInline property if necessary if (Dcl->isStaticLocal()) { @@ -13416,12 +13419,13 @@ bool Sema::GloballyUniqueObjectMightBeAccidentallyDuplicated( Target = FunDcl; // IsInlined() checks for the C++ inline property TargetIsInline = FunDcl->isInlined(); + TargetWasTemplated = + FunDcl->getTemplateSpecializationKind() != TSK_Undeclared; } - // 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) + // Non-inline functions/variables can only legally appear in one TU, + // unless they were part of a template. + if (!TargetIsInline && !TargetWasTemplated) return false; // If the object isn't hidden, the dynamic linker will prevent duplication. @@ -13436,6 +13440,55 @@ bool Sema::GloballyUniqueObjectMightBeAccidentallyDuplicated( return true; } +void Sema::DiagnoseUniqueObjectDuplication(const VarDecl *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. + + // Don't diagnose if we're inside a template; + // we'll diagnose during instantiation instead. + 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(Init->IgnoreParenImpCasts())) { + Diag(Init->getExprLoc(), diag::warn_possible_object_duplication_init) + << VD; + } + } +} + void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) { // If there is no declaration, there was an error parsing it. Just ignore // the initializer. @@ -14655,6 +14708,8 @@ void Sema::CheckCompleteVariableDeclaration(VarDecl *var) { return; } + DiagnoseUniqueObjectDuplication(var); + // Require the destructor. if (!type->isDependentType()) if (const RecordType *recordType = baseType->getAs()) @@ -14842,51 +14897,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(Init->IgnoreParenImpCasts())) { - Diag(Init->getExprLoc(), diag::warn_possible_object_duplication_init) - << VD; - } - } - // FIXME: Warn on unused var template partial specializations. if (VD->isFileVarDecl() && !isa(VD)) MarkUnusedFileScopedDecl(VD); diff --git a/clang/test/SemaCXX/unique_object_duplication.h b/clang/test/SemaCXX/unique_object_duplication.h index 5b2002c31be7c..a59a8f91da8b8 100644 --- a/clang/test/SemaCXX/unique_object_duplication.h +++ b/clang/test/SemaCXX/unique_object_duplication.h @@ -154,4 +154,89 @@ namespace GlobalTest { }; 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 \ No newline at end of file +} // namespace GlobalTest + +/****************************************************************************** + * Case three: Inside templates + ******************************************************************************/ + +namespace TemplateTest { + +template +int disallowedTemplate1 = 0; // hidden-warning {{'disallowedTemplate1' may be duplicated when built into a shared library: it is mutable, has hidden visibility, and external linkage}} + +template int disallowedTemplate1; // hidden-note {{in instantiation of}} + + +// Should work for implicit instantiation as well +template +int disallowedTemplate2 = 0; // hidden-warning {{'disallowedTemplate2' may be duplicated when built into a shared library: it is mutable, has hidden visibility, and external linkage}} + +int implicit_instantiate() { + return disallowedTemplate2; // hidden-note {{in instantiation of}} +} + + +// Ensure we only get warnings for templates that are actually instantiated +template +int maybeAllowedTemplate = 0; // Not instantiated, so no warning here + +template +int maybeAllowedTemplate = 1; // hidden-warning {{'maybeAllowedTemplate' may be duplicated when built into a shared library: it is mutable, has hidden visibility, and external linkage}} + +template <> +int maybeAllowedTemplate = 2; // hidden-warning {{'maybeAllowedTemplate' may be duplicated when built into a shared library: it is mutable, has hidden visibility, and external linkage}} + +template int maybeAllowedTemplate; // hidden-note {{in instantiation of}} + + + +// Should work the same for static class members +template +struct S { + static int staticMember; +}; + +template +int S::staticMember = 0; // Never instantiated + +// T* specialization +template +struct S { + static int staticMember; +}; + +template +int S::staticMember = 1; // hidden-warning {{'staticMember' may be duplicated when built into a shared library: it is mutable, has hidden visibility, and external linkage}} + +template class S; // hidden-note {{in instantiation of}} + +// T& specialization, implicitly instantiated +template +struct S { + static int staticMember; +}; + +template +int S::staticMember = 2; // hidden-warning {{'staticMember' may be duplicated when built into a shared library: it is mutable, has hidden visibility, and external linkage}} + +int implicit_instantiate2() { + return S::staticMember; // hidden-note {{in instantiation of}} +} + + +// Should work for static locals as well +template +int* wrapper() { + static int staticLocal; // hidden-warning {{'staticLocal' may be duplicated when built into a shared library: it is mutable, has hidden visibility, and external linkage}} + return &staticLocal; +} + +template <> +int* wrapper() { + static int staticLocal; // hidden-warning {{'staticLocal' may be duplicated when built into a shared library: it is mutable, has hidden visibility, and external linkage}} + return &staticLocal; +} + +auto dummy = wrapper(); // hidden-note {{in instantiation of}} +} // namespace TemplateTest \ No newline at end of file