Author: nik Date: Tue Jun 11 07:19:09 2019 New Revision: 363068 URL: http://llvm.org/viewvc/llvm-project?rev=363068&view=rev Log: [clang-tidy] Fix invalid read on destruction
...in case the clang tidy plugin is linked into the clang binary. Valgrind's memcheck reports: 8949== Invalid read ==8866== Invalid read of size 4 8866== at 0x164D248B: fetch_sub (atomic_base.h:524) 8866== by 0x164D248B: llvm::ThreadSafeRefCountedBase<clang::ast_matchers::internal::DynMatcherInterface>::Release() const (IntrusiveRefCntPtr.h:98) 8866== by 0x164CE16C: llvm::IntrusiveRefCntPtrInfo<clang::ast_matchers::internal::DynMatcherInterface>::release(clang::ast_matchers::internal::DynMatcherInterface*) (IntrusiveRefCntPtr.h:127) 8866== by 0x164C8D5C: llvm::IntrusiveRefCntPtr<clang::ast_matchers::internal::DynMatcherInterface>::release() (IntrusiveRefCntPtr.h:190) 8866== by 0x164C3B87: llvm::IntrusiveRefCntPtr<clang::ast_matchers::internal::DynMatcherInterface>::~IntrusiveRefCntPtr() (IntrusiveRefCntPtr.h:157) 8866== by 0x164BB4F1: clang::ast_matchers::internal::DynTypedMatcher::~DynTypedMatcher() (ASTMatchersInternal.h:341) 8866== by 0x164BB529: clang::ast_matchers::internal::Matcher<clang::QualType>::~Matcher() (ASTMatchersInternal.h:496) 8866== by 0xD7AE614: __cxa_finalize (cxa_finalize.c:83) 8866== by 0x164B3082: ??? (in /d2/llvm/8/qtc/builds/DebugShared/lib/libclangTidyModernizeModule.so.8) 8866== by 0x4010B72: _dl_fini (dl-fini.c:138) 8866== by 0xD7AE040: __run_exit_handlers (exit.c:108) 8866== by 0xD7AE139: exit (exit.c:139) 8866== by 0xD78CB9D: (below main) (libc-start.c:344) 8866== Address 0x19dd9bc8 is 8 bytes inside a block of size 16 free'd 8866== at 0x4C3123B: operator delete(void*) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) 8866== by 0x1469BB99: clang::ast_matchers::internal::(anonymous namespace)::TrueMatcherImpl::~TrueMatcherImpl() (ASTMatchersInternal.cpp:126) 8866== by 0x1469BBC5: llvm::object_deleter<clang::ast_matchers::internal::(anonymous namespace)::TrueMatcherImpl>::call(void*) (ManagedStatic.h:30) 8866== by 0x9ABFF26: llvm::ManagedStaticBase::destroy() const (ManagedStatic.cpp:72) 8866== by 0x9ABFF94: llvm::llvm_shutdown() (ManagedStatic.cpp:84) 8866== by 0x9A65232: llvm::InitLLVM::~InitLLVM() (InitLLVM.cpp:52) 8866== by 0x14B0C8: main (driver.cpp:323) 8866== Block was alloc'd at 8866== at 0x4C3017F: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) 8866== by 0x1469BB36: llvm::object_creator<clang::ast_matchers::internal::(anonymous namespace)::TrueMatcherImpl>::call() (ManagedStatic.h:24) 8866== by 0x9ABFD99: llvm::ManagedStaticBase::RegisterManagedStatic(void* (*)(), void (*)(void*)) const (ManagedStatic.cpp:42) 8866== by 0x1469B5DF: llvm::ManagedStatic<clang::ast_matchers::internal::(anonymous namespace)::TrueMatcherImpl, llvm::object_creator<clang::ast_matchers::internal::(anonymous namespace)::TrueMatcherImpl>, llvm::object_deleter<clang::ast_matchers::internal::(anonymous namespace)::TrueMatcherImpl> >::operator*() (ManagedStatic.h:67) 8866== by 0x14698F9D: clang::ast_matchers::internal::DynTypedMatcher::trueMatcher(clang::ast_type_traits::ASTNodeKind) (ASTMatchersInternal.cpp:195) 8866== by 0x164C9D3B: _ZNK5clang12ast_matchers8internal11TrueMatchercvNS1_7MatcherIT_EEINS_8QualTypeEEEv (ASTMatchersInternal.h:1247) 8866== by 0x16501458: __static_initialization_and_destruction_0(int, int) (LoopConvertCheck.cpp:48) 8866== by 0x16501976: _GLOBAL__sub_I_LoopConvertCheck.cpp (LoopConvertCheck.cpp:920) 8866== by 0x4010732: call_init (dl-init.c:72) 8866== by 0x4010732: _dl_init (dl-init.c:119) 8866== by 0x40010C9: ??? (in /lib/x86_64-linux-gnu/ld-2.27.so) Differential Revision: https://reviews.llvm.org/D63129 Modified: clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp Modified: clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp?rev=363068&r1=363067&r2=363068&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp Tue Jun 11 07:19:09 2019 @@ -44,18 +44,22 @@ static const char DerefByValueResultName static const char DerefByRefResultName[] = "derefByRefResult"; // shared matchers -static const TypeMatcher AnyType = anything(); +static const TypeMatcher AnyType() { return anything(); } -static const StatementMatcher IntegerComparisonMatcher = - expr(ignoringParenImpCasts( - declRefExpr(to(varDecl(hasType(isInteger())).bind(ConditionVarName))))); - -static const DeclarationMatcher InitToZeroMatcher = - varDecl(hasInitializer(ignoringParenImpCasts(integerLiteral(equals(0))))) - .bind(InitVarName); +static const StatementMatcher IntegerComparisonMatcher() { + return expr(ignoringParenImpCasts( + declRefExpr(to(varDecl(hasType(isInteger())).bind(ConditionVarName))))); +} + +static const DeclarationMatcher InitToZeroMatcher() { + return varDecl( + hasInitializer(ignoringParenImpCasts(integerLiteral(equals(0))))) + .bind(InitVarName); +} -static const StatementMatcher IncrementVarMatcher = - declRefExpr(to(varDecl(hasType(isInteger())).bind(IncrementVarName))); +static const StatementMatcher IncrementVarMatcher() { + return declRefExpr(to(varDecl(hasType(isInteger())).bind(IncrementVarName))); +} /// \brief The matcher for loops over arrays. /// @@ -81,15 +85,15 @@ StatementMatcher makeArrayLoopMatcher() return forStmt( unless(isInTemplateInstantiation()), - hasLoopInit(declStmt(hasSingleDecl(InitToZeroMatcher))), + hasLoopInit(declStmt(hasSingleDecl(InitToZeroMatcher()))), hasCondition(anyOf( binaryOperator(hasOperatorName("<"), - hasLHS(IntegerComparisonMatcher), + hasLHS(IntegerComparisonMatcher()), hasRHS(ArrayBoundMatcher)), binaryOperator(hasOperatorName(">"), hasLHS(ArrayBoundMatcher), - hasRHS(IntegerComparisonMatcher)))), + hasRHS(IntegerComparisonMatcher())))), hasIncrement(unaryOperator(hasOperatorName("++"), - hasUnaryOperand(IncrementVarMatcher)))) + hasUnaryOperand(IncrementVarMatcher())))) .bind(LoopNameArray); } @@ -190,7 +194,7 @@ StatementMatcher makeIteratorLoopMatcher hasIncrement(anyOf( unaryOperator(hasOperatorName("++"), hasUnaryOperand(declRefExpr( - to(varDecl(hasType(pointsTo(AnyType))) + to(varDecl(hasType(pointsTo(AnyType()))) .bind(IncrementVarName))))), cxxOperatorCallExpr( hasOverloadedOperatorName("++"), @@ -278,17 +282,17 @@ StatementMatcher makePseudoArrayLoopMatc unless(isInTemplateInstantiation()), hasLoopInit( anyOf(declStmt(declCountIs(2), - containsDeclaration(0, InitToZeroMatcher), + containsDeclaration(0, InitToZeroMatcher()), containsDeclaration(1, EndDeclMatcher)), - declStmt(hasSingleDecl(InitToZeroMatcher)))), + declStmt(hasSingleDecl(InitToZeroMatcher())))), hasCondition(anyOf( binaryOperator(hasOperatorName("<"), - hasLHS(IntegerComparisonMatcher), + hasLHS(IntegerComparisonMatcher()), hasRHS(IndexBoundMatcher)), binaryOperator(hasOperatorName(">"), hasLHS(IndexBoundMatcher), - hasRHS(IntegerComparisonMatcher)))), + hasRHS(IntegerComparisonMatcher())))), hasIncrement(unaryOperator(hasOperatorName("++"), - hasUnaryOperand(IncrementVarMatcher)))) + hasUnaryOperand(IncrementVarMatcher())))) .bind(LoopNamePseudoArray); } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits