r354795 - Make static counters in ASTContext non-static.
Alexander Kornienko via cfe-commits
cfe-commits at lists.llvm.org
Mon Feb 25 14:18:13 PST 2019
Thank you for taking care of this! That's a bug indeed. Will recommit with
a fix.
On Mon, Feb 25, 2019 at 9:25 PM Vlad Tsyrklevich <vlad at tsyrklevich.net>
wrote:
> I've reverted this commit in r354812, it was causing MSan failures like
> http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/29886/steps/check-clang%20msan/logs/stdio Though
> these error reports don't clearly implicate this change, from your change
> it seems like the failure is occurring because you changed static
> (zero-initialized) variables to member (uninitialized) variables that were
> then never initialized before being used. The build recovered after the
> revert landed in
> http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/29894
>
> On Mon, Feb 25, 2019 at 8:07 AM Alexander Kornienko via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
>
>> Author: alexfh
>> Date: Mon Feb 25 08:08:46 2019
>> New Revision: 354795
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=354795&view=rev
>> Log:
>> Make static counters in ASTContext non-static.
>>
>> Summary:
>> Fixes a data race and makes it possible to run clang-based tools in
>> multithreaded environment with TSan.
>>
>> Reviewers: ilya-biryukov, riccibruno
>>
>> Reviewed By: riccibruno
>>
>> Subscribers: riccibruno, jfb, cfe-commits
>>
>> Tags: #clang
>>
>> Differential Revision: https://reviews.llvm.org/D58612
>>
>> Modified:
>> cfe/trunk/include/clang/AST/ASTContext.h
>> cfe/trunk/lib/AST/ASTContext.cpp
>> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>>
>> Modified: cfe/trunk/include/clang/AST/ASTContext.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTContext.h?rev=354795&r1=354794&r2=354795&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/include/clang/AST/ASTContext.h (original)
>> +++ cfe/trunk/include/clang/AST/ASTContext.h Mon Feb 25 08:08:46 2019
>> @@ -2809,46 +2809,46 @@ public:
>>
>> //===--------------------------------------------------------------------===//
>>
>> /// The number of implicitly-declared default constructors.
>> - static unsigned NumImplicitDefaultConstructors;
>> + unsigned NumImplicitDefaultConstructors;
>>
>> /// The number of implicitly-declared default constructors for
>> /// which declarations were built.
>> - static unsigned NumImplicitDefaultConstructorsDeclared;
>> + unsigned NumImplicitDefaultConstructorsDeclared;
>>
>> /// The number of implicitly-declared copy constructors.
>> - static unsigned NumImplicitCopyConstructors;
>> + unsigned NumImplicitCopyConstructors;
>>
>> /// The number of implicitly-declared copy constructors for
>> /// which declarations were built.
>> - static unsigned NumImplicitCopyConstructorsDeclared;
>> + unsigned NumImplicitCopyConstructorsDeclared;
>>
>> /// The number of implicitly-declared move constructors.
>> - static unsigned NumImplicitMoveConstructors;
>> + unsigned NumImplicitMoveConstructors;
>>
>> /// The number of implicitly-declared move constructors for
>> /// which declarations were built.
>> - static unsigned NumImplicitMoveConstructorsDeclared;
>> + unsigned NumImplicitMoveConstructorsDeclared;
>>
>> /// The number of implicitly-declared copy assignment operators.
>> - static unsigned NumImplicitCopyAssignmentOperators;
>> + unsigned NumImplicitCopyAssignmentOperators;
>>
>> /// The number of implicitly-declared copy assignment operators for
>> /// which declarations were built.
>> - static unsigned NumImplicitCopyAssignmentOperatorsDeclared;
>> + unsigned NumImplicitCopyAssignmentOperatorsDeclared;
>>
>> /// The number of implicitly-declared move assignment operators.
>> - static unsigned NumImplicitMoveAssignmentOperators;
>> + unsigned NumImplicitMoveAssignmentOperators;
>>
>> /// The number of implicitly-declared move assignment operators for
>> /// which declarations were built.
>> - static unsigned NumImplicitMoveAssignmentOperatorsDeclared;
>> + unsigned NumImplicitMoveAssignmentOperatorsDeclared;
>>
>> /// The number of implicitly-declared destructors.
>> - static unsigned NumImplicitDestructors;
>> + unsigned NumImplicitDestructors;
>>
>> /// The number of implicitly-declared destructors for which
>> /// declarations were built.
>> - static unsigned NumImplicitDestructorsDeclared;
>> + unsigned NumImplicitDestructorsDeclared;
>>
>> public:
>> /// Initialize built-in types.
>>
>> Modified: cfe/trunk/lib/AST/ASTContext.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=354795&r1=354794&r2=354795&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/AST/ASTContext.cpp (original)
>> +++ cfe/trunk/lib/AST/ASTContext.cpp Mon Feb 25 08:08:46 2019
>> @@ -94,19 +94,6 @@
>>
>> using namespace clang;
>>
>> -unsigned ASTContext::NumImplicitDefaultConstructors;
>> -unsigned ASTContext::NumImplicitDefaultConstructorsDeclared;
>> -unsigned ASTContext::NumImplicitCopyConstructors;
>> -unsigned ASTContext::NumImplicitCopyConstructorsDeclared;
>> -unsigned ASTContext::NumImplicitMoveConstructors;
>> -unsigned ASTContext::NumImplicitMoveConstructorsDeclared;
>> -unsigned ASTContext::NumImplicitCopyAssignmentOperators;
>> -unsigned ASTContext::NumImplicitCopyAssignmentOperatorsDeclared;
>> -unsigned ASTContext::NumImplicitMoveAssignmentOperators;
>> -unsigned ASTContext::NumImplicitMoveAssignmentOperatorsDeclared;
>> -unsigned ASTContext::NumImplicitDestructors;
>> -unsigned ASTContext::NumImplicitDestructorsDeclared;
>> -
>> enum FloatingRank {
>> Float16Rank, HalfRank, FloatRank, DoubleRank, LongDoubleRank,
>> Float128Rank
>> };
>>
>> Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=354795&r1=354794&r2=354795&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Mon Feb 25 08:08:46 2019
>> @@ -7971,14 +7971,14 @@ void Sema::ActOnFinishCXXMemberSpecifica
>> /// definition of the class is complete.
>> void Sema::AddImplicitlyDeclaredMembersToClass(CXXRecordDecl *ClassDecl)
>> {
>> if (ClassDecl->needsImplicitDefaultConstructor()) {
>> - ++ASTContext::NumImplicitDefaultConstructors;
>> + ++getASTContext().NumImplicitDefaultConstructors;
>>
>> if (ClassDecl->hasInheritedConstructor())
>> DeclareImplicitDefaultConstructor(ClassDecl);
>> }
>>
>> if (ClassDecl->needsImplicitCopyConstructor()) {
>> - ++ASTContext::NumImplicitCopyConstructors;
>> + ++getASTContext().NumImplicitCopyConstructors;
>>
>> // If the properties or semantics of the copy constructor couldn't be
>> // determined while the class was being declared, force a declaration
>> @@ -8000,7 +8000,7 @@ void Sema::AddImplicitlyDeclaredMembersT
>> }
>>
>> if (getLangOpts().CPlusPlus11 &&
>> ClassDecl->needsImplicitMoveConstructor()) {
>> - ++ASTContext::NumImplicitMoveConstructors;
>> + ++getASTContext().NumImplicitMoveConstructors;
>>
>> if (ClassDecl->needsOverloadResolutionForMoveConstructor() ||
>> ClassDecl->hasInheritedConstructor())
>> @@ -8008,7 +8008,7 @@ void Sema::AddImplicitlyDeclaredMembersT
>> }
>>
>> if (ClassDecl->needsImplicitCopyAssignment()) {
>> - ++ASTContext::NumImplicitCopyAssignmentOperators;
>> + ++getASTContext().NumImplicitCopyAssignmentOperators;
>>
>> // If we have a dynamic class, then the copy assignment operator may
>> be
>> // virtual, so we have to declare it immediately. This ensures that,
>> e.g.,
>> @@ -8021,7 +8021,7 @@ void Sema::AddImplicitlyDeclaredMembersT
>> }
>>
>> if (getLangOpts().CPlusPlus11 &&
>> ClassDecl->needsImplicitMoveAssignment()) {
>> - ++ASTContext::NumImplicitMoveAssignmentOperators;
>> + ++getASTContext().NumImplicitMoveAssignmentOperators;
>>
>> // Likewise for the move assignment operator.
>> if (ClassDecl->isDynamicClass() ||
>> @@ -8031,7 +8031,7 @@ void Sema::AddImplicitlyDeclaredMembersT
>> }
>>
>> if (ClassDecl->needsImplicitDestructor()) {
>> - ++ASTContext::NumImplicitDestructors;
>> + ++getASTContext().NumImplicitDestructors;
>>
>> // If we have a dynamic class, then the destructor may be virtual,
>> so we
>> // have to declare the destructor immediately. This ensures that,
>> e.g., it
>> @@ -11013,7 +11013,7 @@ CXXConstructorDecl *Sema::DeclareImplici
>> DefaultCon->setTrivial(ClassDecl->hasTrivialDefaultConstructor());
>>
>> // Note that we have declared this constructor.
>> - ++ASTContext::NumImplicitDefaultConstructorsDeclared;
>> + ++getASTContext().NumImplicitDefaultConstructorsDeclared;
>>
>> Scope *S = getScopeForContext(ClassDecl);
>> CheckImplicitSpecialMemberDeclaration(S, DefaultCon);
>> @@ -11286,7 +11286,7 @@ CXXDestructorDecl *Sema::DeclareImplicit
>>
>> ClassDecl->hasTrivialDestructorForCall());
>>
>> // Note that we have declared this destructor.
>> - ++ASTContext::NumImplicitDestructorsDeclared;
>> + ++getASTContext().NumImplicitDestructorsDeclared;
>>
>> Scope *S = getScopeForContext(ClassDecl);
>> CheckImplicitSpecialMemberDeclaration(S, Destructor);
>> @@ -11896,7 +11896,7 @@ CXXMethodDecl *Sema::DeclareImplicitCopy
>> : ClassDecl->hasTrivialCopyAssignment());
>>
>> // Note that we have added this copy-assignment operator.
>> - ++ASTContext::NumImplicitCopyAssignmentOperatorsDeclared;
>> + ++getASTContext().NumImplicitCopyAssignmentOperatorsDeclared;
>>
>> Scope *S = getScopeForContext(ClassDecl);
>> CheckImplicitSpecialMemberDeclaration(S, CopyAssignment);
>> @@ -12219,7 +12219,7 @@ CXXMethodDecl *Sema::DeclareImplicitMove
>> : ClassDecl->hasTrivialMoveAssignment());
>>
>> // Note that we have added this copy-assignment operator.
>> - ++ASTContext::NumImplicitMoveAssignmentOperatorsDeclared;
>> + ++getASTContext().NumImplicitMoveAssignmentOperatorsDeclared;
>>
>> Scope *S = getScopeForContext(ClassDecl);
>> CheckImplicitSpecialMemberDeclaration(S, MoveAssignment);
>> @@ -12602,7 +12602,7 @@ CXXConstructorDecl *Sema::DeclareImplici
>> : ClassDecl->hasTrivialCopyConstructorForCall()));
>>
>> // Note that we have declared this constructor.
>> - ++ASTContext::NumImplicitCopyConstructorsDeclared;
>> + ++getASTContext().NumImplicitCopyConstructorsDeclared;
>>
>> Scope *S = getScopeForContext(ClassDecl);
>> CheckImplicitSpecialMemberDeclaration(S, CopyConstructor);
>> @@ -12732,7 +12732,7 @@ CXXConstructorDecl *Sema::DeclareImplici
>> : ClassDecl->hasTrivialMoveConstructorForCall()));
>>
>> // Note that we have declared this constructor.
>> - ++ASTContext::NumImplicitMoveConstructorsDeclared;
>> + ++getASTContext().NumImplicitMoveConstructorsDeclared;
>>
>> Scope *S = getScopeForContext(ClassDecl);
>> CheckImplicitSpecialMemberDeclaration(S, MoveConstructor);
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190225/a6be9582/attachment-0001.html>
More information about the cfe-commits
mailing list