r290352 - [analyzer] Update GTestChecker to tighten API detection
Devin Coughlin via cfe-commits
cfe-commits at lists.llvm.org
Thu Dec 22 09:52:58 PST 2016
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
+}
More information about the cfe-commits
mailing list