Done. r354827 should be better. On Mon, Feb 25, 2019 at 11:18 PM Alexander Kornienko <ale...@google.com> wrote:
> 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
