From 3906da9a67fcde6e85c14239f7bacd8aaa0e332e Mon Sep 17 00:00:00 2001 From: Devin Coughlin Date: Thu, 22 Dec 2016 17:52:57 +0000 Subject: [PATCH] [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. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@290352 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/StaticAnalyzer/Checkers/GTestChecker.cpp | 84 +++++++++++--------- test/Analysis/gtest.cpp | 17 +++- 2 files changed, 62 insertions(+), 39 deletions(-) diff --git a/lib/StaticAnalyzer/Checkers/GTestChecker.cpp b/lib/StaticAnalyzer/Checkers/GTestChecker.cpp index eb7c2874b9..f0be41b293 100644 --- a/lib/StaticAnalyzer/Checkers/GTestChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/GTestChecker.cpp @@ -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 - // AssertionResult(const T& success, - // typename internal::EnableIf< - // !internal::ImplicitlyConvertible::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()) { - // 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()) return; BooleanArgVal = C.getState()->getSVal(BooleanArgVal.castAs()); } - ProgramStateRef State = C.getState(); - SVal ThisVal = Call->getCXXThisVal(); SVal ThisSuccess = getAssertionResultSuccessFieldValue( @@ -169,7 +159,7 @@ void GTestChecker::modelAssertionResultBoolConstructor( /// '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 CallEvent &Call, 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 + // AssertionResult(const T& success, + // typename internal::EnableIf< + // !internal::ImplicitlyConvertible::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(); + if (RefTy && + RefTy->getPointeeType()->getCanonicalTypeUnqualified() == BoolTy) { + // We have AssertionResult(bool &, ...) + modelAssertionResultBoolConstructor(CtorCall, /*IsRef=*/true, C); + return; + } } } diff --git a/test/Analysis/gtest.cpp b/test/Analysis/gtest.cpp index 33043b714b..f33569598e 100644 --- a/test/Analysis/gtest.cpp +++ b/test/Analysis/gtest.cpp @@ -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 +} -- 2.40.0