Author: dcoughlin Date: Thu Dec 22 11:52:57 2016 New Revision: 290352 URL: http://llvm.org/viewvc/llvm-project?rev=290352&view=rev Log: [analyzer] Update GTestChecker to tighten API detection
Update the GTestChecker to tighten up the API detection and make it cleaner in response to post-commit feedback. Also add tests for when temporary destructors are enabled to make sure we get the expected behavior when inlining constructors for temporaries. Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/GTestChecker.cpp cfe/trunk/test/Analysis/gtest.cpp Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/GTestChecker.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/GTestChecker.cpp?rev=290352&r1=290351&r2=290352&view=diff ============================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/GTestChecker.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Checkers/GTestChecker.cpp Thu Dec 22 11:52:57 2016 @@ -7,7 +7,7 @@ // //===----------------------------------------------------------------------===// // -// This checker models the behavior of un-inlined APIs from the the gtest +// This checker models the behavior of un-inlined APIs from the gtest // unit-testing library to avoid false positives when using assertions from // that library. // @@ -85,7 +85,7 @@ using namespace ento; // does not inline these since it does not yet reliably call temporary // destructors). // -// This checker compensates for the missing inlining by propgagating the +// This checker compensates for the missing inlining by propagating the // _success value across the bool and copy constructors so the assertion behaves // as expected. @@ -102,7 +102,7 @@ public: private: void modelAssertionResultBoolConstructor(const CXXConstructorCall *Call, - CheckerContext &C) const; + bool IsRef, CheckerContext &C) const; void modelAssertionResultCopyConstructor(const CXXConstructorCall *Call, CheckerContext &C) const; @@ -122,35 +122,25 @@ private: GTestChecker::GTestChecker() : AssertionResultII(nullptr), SuccessII(nullptr) {} -/// Model a call to an un-inlined AssertionResult(boolean expression). +/// Model a call to an un-inlined AssertionResult(bool) or +/// AssertionResult(bool &, ...). /// To do so, constrain the value of the newly-constructed instance's 'success_' /// field to be equal to the passed-in boolean value. +/// +/// \param IsRef Whether the boolean parameter is a reference or not. void GTestChecker::modelAssertionResultBoolConstructor( - const CXXConstructorCall *Call, CheckerContext &C) const { - assert(Call->getNumArgs() > 0); - - // Depending on the version of gtest the constructor can be either of: - // - // v1.7 and earlier: - // AssertionResult(bool success) - // - // v1.8 and greater: - // template <typename T> - // AssertionResult(const T& success, - // typename internal::EnableIf< - // !internal::ImplicitlyConvertible<T, - // AssertionResult>::value>::type*) + const CXXConstructorCall *Call, bool IsRef, CheckerContext &C) const { + assert(Call->getNumArgs() >= 1 && Call->getNumArgs() <= 2); + ProgramStateRef State = C.getState(); SVal BooleanArgVal = Call->getArgSVal(0); - if (Call->getDecl()->getParamDecl(0)->getType()->getAs<ReferenceType>()) { - // We have v1.8+, so load the value from the reference. + if (IsRef) { + // The argument is a reference, so load from it to get the boolean value. if (!BooleanArgVal.getAs<Loc>()) return; BooleanArgVal = C.getState()->getSVal(BooleanArgVal.castAs<Loc>()); } - ProgramStateRef State = C.getState(); - SVal ThisVal = Call->getCXXThisVal(); SVal ThisSuccess = getAssertionResultSuccessFieldValue( @@ -169,7 +159,7 @@ void GTestChecker::modelAssertionResultB /// 'success_' field. void GTestChecker::modelAssertionResultCopyConstructor( const CXXConstructorCall *Call, CheckerContext &C) const { - assert(Call->getNumArgs() > 0); + assert(Call->getNumArgs() == 1); // The first parameter of the the copy constructor must be the other // instance to initialize this instances fields from. @@ -206,22 +196,44 @@ void GTestChecker::checkPostCall(const C if (CtorParent->getIdentifier() != AssertionResultII) return; - if (CtorDecl->getNumParams() == 0) - return; - + unsigned ParamCount = CtorDecl->getNumParams(); - // Call the appropriate modeling method based on the type of the first - // constructor parameter. - const ParmVarDecl *ParamDecl = CtorDecl->getParamDecl(0); - QualType ParamType = ParamDecl->getType(); - if (CtorDecl->getNumParams() <= 2 && - ParamType.getNonReferenceType()->getCanonicalTypeUnqualified() == - C.getASTContext().BoolTy) { - // The first parameter is either a boolean or reference to a boolean - modelAssertionResultBoolConstructor(CtorCall, C); + // Call the appropriate modeling method based the parameters and their + // types. - } else if (CtorDecl->isCopyConstructor()) { + // We have AssertionResult(const &AssertionResult) + if (CtorDecl->isCopyConstructor() && ParamCount == 1) { modelAssertionResultCopyConstructor(CtorCall, C); + return; + } + + // There are two possible boolean constructors, depending on which + // version of gtest is being used: + // + // v1.7 and earlier: + // AssertionResult(bool success) + // + // v1.8 and greater: + // template <typename T> + // AssertionResult(const T& success, + // typename internal::EnableIf< + // !internal::ImplicitlyConvertible<T, + // AssertionResult>::value>::type*) + // + CanQualType BoolTy = C.getASTContext().BoolTy; + if (ParamCount == 1 && CtorDecl->getParamDecl(0)->getType() == BoolTy) { + // We have AssertionResult(bool) + modelAssertionResultBoolConstructor(CtorCall, /*IsRef=*/false, C); + return; + } + if (ParamCount == 2){ + auto *RefTy = CtorDecl->getParamDecl(0)->getType()->getAs<ReferenceType>(); + if (RefTy && + RefTy->getPointeeType()->getCanonicalTypeUnqualified() == BoolTy) { + // We have AssertionResult(bool &, ...) + modelAssertionResultBoolConstructor(CtorCall, /*IsRef=*/true, C); + return; + } } } Modified: cfe/trunk/test/Analysis/gtest.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/gtest.cpp?rev=290352&r1=290351&r2=290352&view=diff ============================================================================== --- cfe/trunk/test/Analysis/gtest.cpp (original) +++ cfe/trunk/test/Analysis/gtest.cpp Thu Dec 22 11:52:57 2016 @@ -1,7 +1,9 @@ -//RUN: %clang_cc1 -cc1 -std=c++11 -analyze -analyzer-checker=core,apiModeling.google.GTest -analyzer-eagerly-assume %s -verify -//RUN: %clang_cc1 -cc1 -std=c++11 -analyze -analyzer-checker=core,apiModeling.google.GTest -analyzer-eagerly-assume -DGTEST_VERSION_1_8_AND_LATER=1 %s -verify +//RUN: %clang_cc1 -cc1 -std=c++11 -analyze -analyzer-checker=core,apiModeling.google.GTest,debug.ExprInspection -analyzer-eagerly-assume %s -verify +//RUN: %clang_cc1 -cc1 -std=c++11 -analyze -analyzer-checker=core,apiModeling.google.GTest,debug.ExprInspection -analyzer-eagerly-assume -DGTEST_VERSION_1_8_AND_LATER=1 %s -verify +//RUN: %clang_cc1 -cc1 -std=c++11 -analyze -analyzer-checker=core,apiModeling.google.GTest,debug.ExprInspection -analyzer-eagerly-assume -analyzer-config cfg-temporary-dtors=true %s -verify -// expected-no-diagnostics +void clang_analyzer_eval(int); +void clang_analyzer_warnIfReached(); namespace std { class string { @@ -140,3 +142,12 @@ void testAssertFalse(int *p) { ASSERT_FALSE(p == nullptr); EXPECT_TRUE(1 == *p); // no-warning } + +void testConstrainState(int p) { + ASSERT_TRUE(p == 7); + + clang_analyzer_eval(p == 7); // expected-warning {{TRUE}} + + ASSERT_TRUE(false); + clang_analyzer_warnIfReached(); // no-warning +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits