r354795 - Make static counters in ASTContext non-static.
Alexander Kornienko via cfe-commits
cfe-commits at lists.llvm.org
Mon Feb 25 14:31:13 PST 2019
Done. r354827 should be better.
On Mon, Feb 25, 2019 at 11:18 PM Alexander Kornienko <alexfh at 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 <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/85b8b923/attachment-0001.html>
More information about the cfe-commits
mailing list