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 <v...@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@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@lists.llvm.org >> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits