r290143 - [analyzer] Add checker modeling gtest APIs.
Devin Coughlin via cfe-commits
cfe-commits at lists.llvm.org
Mon Dec 19 14:50:31 PST 2016
Author: dcoughlin
Date: Mon Dec 19 16:50:31 2016
New Revision: 290143
URL: http://llvm.org/viewvc/llvm-project?rev=290143&view=rev
Log:
[analyzer] Add checker modeling gtest APIs.
gtest is a widely-used unit-testing API. It provides macros for unit test
assertions:
ASSERT_TRUE(p != nullptr);
that expand into an if statement that constructs an object representing
the result of the assertion and returns when the assertion is false:
if (AssertionResult gtest_ar_ = AssertionResult(p == nullptr))
;
else
return ...;
Unfortunately, the analyzer does not model the effect of the constructor
precisely because (1) the copy constructor implementation is missing from the
the header (so it can't be inlined) and (2) the boolean-argument constructor
is constructed into a temporary (so the analyzer decides not to inline it since
it doesn't reliably call temporary destructors right now).
This results in false positives because the analyzer does not realize that the
the assertion must hold along the non-return path.
This commit addresses the false positives by explicitly modeling the effects
of the two un-inlined constructors on the AssertionResult state.
I've added a new package, "apiModeling", for these kinds of checkers that
model APIs but don't emit any diagnostics. I envision all the checkers in
this package always being on by default.
This addresses the false positives reported in PR30936.
Differential Revision: https://reviews.llvm.org/D27773
rdar://problem/22705813
Added:
cfe/trunk/lib/StaticAnalyzer/Checkers/GTestChecker.cpp
cfe/trunk/test/Analysis/gtest.cpp
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
cfe/trunk/lib/Driver/Tools.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
cfe/trunk/test/Driver/analyzer-target-enabled-checkers.cpp
Modified: cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td?rev=290143&r1=290142&r2=290143&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td Mon Dec 19 16:50:31 2016
@@ -79,6 +79,13 @@ def LocalizabilityOptIn : Package<"local
def MPI : Package<"mpi">, InPackage<OptIn>;
def LLVM : Package<"llvm">;
+
+// The APIModeling package is for checkers that model APIs and don't perform
+// any diagnostics. These checkers are always turned on; this package is
+// intended for API modeling that is not controlled by the the target triple.
+def APIModeling : Package<"apiModeling">, Hidden;
+def GoogleAPIModeling : Package<"google">, InPackage<APIModeling>;
+
def Debug : Package<"debug">;
def CloneDetectionAlpha : Package<"clone">, InPackage<Alpha>, Hidden;
@@ -643,6 +650,17 @@ def LLVMConventionsChecker : Checker<"Co
HelpText<"Check code for LLVM codebase conventions">,
DescFile<"LLVMConventionsChecker.cpp">;
+
+
+//===----------------------------------------------------------------------===//
+// Checkers modeling Google APIs.
+//===----------------------------------------------------------------------===//
+
+def GTestChecker : Checker<"GTest">,
+ InPackage<GoogleAPIModeling>,
+ HelpText<"Model gtest assertion APIs">,
+ DescFile<"GTestChecker.cpp">;
+
//===----------------------------------------------------------------------===//
// Debugging checkers (for analyzer development).
//===----------------------------------------------------------------------===//
Modified: cfe/trunk/lib/Driver/Tools.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=290143&r1=290142&r2=290143&view=diff
==============================================================================
--- cfe/trunk/lib/Driver/Tools.cpp (original)
+++ cfe/trunk/lib/Driver/Tools.cpp Mon Dec 19 16:50:31 2016
@@ -4263,6 +4263,7 @@ void Clang::ConstructJob(Compilation &C,
// Add default argument set.
if (!Args.hasArg(options::OPT__analyzer_no_default_checks)) {
CmdArgs.push_back("-analyzer-checker=core");
+ CmdArgs.push_back("-analyzer-checker=apiModeling");
if (!IsWindowsMSVC) {
CmdArgs.push_back("-analyzer-checker=unix");
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt?rev=290143&r1=290142&r2=290143&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt Mon Dec 19 16:50:31 2016
@@ -37,6 +37,7 @@ add_clang_library(clangStaticAnalyzerChe
ExprInspectionChecker.cpp
FixedAddressChecker.cpp
GenericTaintChecker.cpp
+ GTestChecker.cpp
IdenticalExprChecker.cpp
IvarInvalidationChecker.cpp
LLVMConventionsChecker.cpp
Added: cfe/trunk/lib/StaticAnalyzer/Checkers/GTestChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/GTestChecker.cpp?rev=290143&view=auto
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/GTestChecker.cpp (added)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/GTestChecker.cpp Mon Dec 19 16:50:31 2016
@@ -0,0 +1,287 @@
+//==- GTestChecker.cpp - Model gtest API --*- C++ -*-==//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This checker models the behavior of un-inlined APIs from the the gtest
+// unit-testing library to avoid false positives when using assertions from
+// that library.
+//
+//===----------------------------------------------------------------------===//
+
+#include "ClangSACheckers.h"
+#include "clang/AST/Expr.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
+#include "llvm/Support/raw_ostream.h"
+
+using namespace clang;
+using namespace ento;
+
+// Modeling of un-inlined AssertionResult constructors
+//
+// The gtest unit testing API provides macros for assertions that expand
+// into an if statement that calls a series of constructors and returns
+// when the "assertion" is false.
+//
+// For example,
+//
+// ASSERT_TRUE(a == b)
+//
+// expands into:
+//
+// switch (0)
+// case 0:
+// default:
+// if (const ::testing::AssertionResult gtest_ar_ =
+// ::testing::AssertionResult((a == b)))
+// ;
+// else
+// return ::testing::internal::AssertHelper(
+// ::testing::TestPartResult::kFatalFailure,
+// "<path to project>",
+// <line number>,
+// ::testing::internal::GetBoolAssertionFailureMessage(
+// gtest_ar_, "a == b", "false", "true")
+// .c_str()) = ::testing::Message();
+//
+// where AssertionResult is defined similarly to
+//
+// class AssertionResult {
+// public:
+// AssertionResult(const AssertionResult& other);
+// explicit AssertionResult(bool success) : success_(success) {}
+// operator bool() const { return success_; }
+// ...
+// private:
+// bool success_;
+// };
+//
+// In order for the analyzer to correctly handle this assertion, it needs to
+// know that the boolean value of the expression "a == b" is stored the
+// 'success_' field of the original AssertionResult temporary and propagated
+// (via the copy constructor) into the 'success_' field of the object stored
+// in 'gtest_ar_'. That boolean value will then be returned from the bool
+// conversion method in the if statement. This guarantees that the assertion
+// holds when the return path is not taken.
+//
+// If the success value is not properly propagated, then the eager case split
+// on evaluating the expression can cause pernicious false positives
+// on the non-return path:
+//
+// ASSERT(ptr != NULL)
+// *ptr = 7; // False positive null pointer dereference here
+//
+// Unfortunately, the bool constructor cannot be inlined (because its
+// implementation is not present in the headers) and the copy constructor is
+// not inlined (because it is constructed into a temporary and the analyzer
+// does not inline these since it does not yet reliably call temporary
+// destructors).
+//
+// This checker compensates for the missing inlining by propgagating the
+// _success value across the bool and copy constructors so the assertion behaves
+// as expected.
+
+namespace {
+class GTestChecker : public Checker<check::PostCall> {
+
+ mutable IdentifierInfo *AssertionResultII;
+ mutable IdentifierInfo *SuccessII;
+
+public:
+ GTestChecker();
+
+ void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
+
+private:
+ void modelAssertionResultBoolConstructor(const CXXConstructorCall *Call,
+ CheckerContext &C) const;
+
+ void modelAssertionResultCopyConstructor(const CXXConstructorCall *Call,
+ CheckerContext &C) const;
+
+ void initIdentifierInfo(ASTContext &Ctx) const;
+
+ SVal
+ getAssertionResultSuccessFieldValue(const CXXRecordDecl *AssertionResultDecl,
+ SVal Instance,
+ ProgramStateRef State) const;
+
+ static ProgramStateRef assumeValuesEqual(SVal Val1, SVal Val2,
+ ProgramStateRef State,
+ CheckerContext &C);
+};
+} // End anonymous namespace.
+
+GTestChecker::GTestChecker() : AssertionResultII(nullptr), SuccessII(nullptr) {}
+
+/// Model a call to an un-inlined AssertionResult(boolean expression).
+/// To do so, constrain the value of the newly-constructed instance's 'success_'
+/// field to be equal to the passed-in boolean value.
+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*)
+
+ 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 (!BooleanArgVal.getAs<Loc>())
+ return;
+ BooleanArgVal = C.getState()->getSVal(BooleanArgVal.castAs<Loc>());
+ }
+
+ ProgramStateRef State = C.getState();
+
+ SVal ThisVal = Call->getCXXThisVal();
+
+ SVal ThisSuccess = getAssertionResultSuccessFieldValue(
+ Call->getDecl()->getParent(), ThisVal, State);
+
+ State = assumeValuesEqual(ThisSuccess, BooleanArgVal, State, C);
+ C.addTransition(State);
+}
+
+/// Model a call to an un-inlined AssertionResult copy constructor:
+///
+/// AssertionResult(const &AssertionResult other)
+///
+/// To do so, constrain the value of the newly-constructed instance's
+/// 'success_' field to be equal to the value of the pass-in instance's
+/// 'success_' field.
+void GTestChecker::modelAssertionResultCopyConstructor(
+ const CXXConstructorCall *Call, CheckerContext &C) const {
+ assert(Call->getNumArgs() > 0);
+
+ // The first parameter of the the copy constructor must be the other
+ // instance to initialize this instances fields from.
+ SVal OtherVal = Call->getArgSVal(0);
+ SVal ThisVal = Call->getCXXThisVal();
+
+ const CXXRecordDecl *AssertResultClassDecl = Call->getDecl()->getParent();
+ ProgramStateRef State = C.getState();
+
+ SVal ThisSuccess = getAssertionResultSuccessFieldValue(AssertResultClassDecl,
+ ThisVal, State);
+ SVal OtherSuccess = getAssertionResultSuccessFieldValue(AssertResultClassDecl,
+ OtherVal, State);
+
+ State = assumeValuesEqual(ThisSuccess, OtherSuccess, State, C);
+ C.addTransition(State);
+}
+
+/// Model calls to AssertionResult constructors that are not inlined.
+void GTestChecker::checkPostCall(const CallEvent &Call,
+ CheckerContext &C) const {
+ /// If the constructor was inlined, there is no need model it.
+ if (C.wasInlined)
+ return;
+
+ initIdentifierInfo(C.getASTContext());
+
+ auto *CtorCall = dyn_cast<CXXConstructorCall>(&Call);
+ if (!CtorCall)
+ return;
+
+ const CXXConstructorDecl *CtorDecl = CtorCall->getDecl();
+ const CXXRecordDecl *CtorParent = CtorDecl->getParent();
+ if (CtorParent->getIdentifier() != AssertionResultII)
+ return;
+
+ if (CtorDecl->getNumParams() == 0)
+ return;
+
+
+ // 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);
+
+ } else if (CtorDecl->isCopyConstructor()) {
+ modelAssertionResultCopyConstructor(CtorCall, C);
+ }
+}
+
+void GTestChecker::initIdentifierInfo(ASTContext &Ctx) const {
+ if (AssertionResultII)
+ return;
+
+ AssertionResultII = &Ctx.Idents.get("AssertionResult");
+ SuccessII = &Ctx.Idents.get("success_");
+}
+
+/// Returns the value stored in the 'success_' field of the passed-in
+/// AssertionResult instance.
+SVal GTestChecker::getAssertionResultSuccessFieldValue(
+ const CXXRecordDecl *AssertionResultDecl, SVal Instance,
+ ProgramStateRef State) const {
+
+ DeclContext::lookup_result Result = AssertionResultDecl->lookup(SuccessII);
+ if (Result.empty())
+ return UnknownVal();
+
+ auto *SuccessField = dyn_cast<FieldDecl>(Result.front());
+ if (!SuccessField)
+ return UnknownVal();
+
+ Optional<Loc> FieldLoc =
+ State->getLValue(SuccessField, Instance).getAs<Loc>();
+ if (!FieldLoc.hasValue())
+ return UnknownVal();
+
+ return State->getSVal(*FieldLoc);
+}
+
+/// Constrain the passed-in state to assume two values are equal.
+ProgramStateRef GTestChecker::assumeValuesEqual(SVal Val1, SVal Val2,
+ ProgramStateRef State,
+ CheckerContext &C) {
+ if (!Val1.getAs<DefinedOrUnknownSVal>() ||
+ !Val2.getAs<DefinedOrUnknownSVal>())
+ return State;
+
+ auto ValuesEqual =
+ C.getSValBuilder().evalEQ(State, Val1.castAs<DefinedOrUnknownSVal>(),
+ Val2.castAs<DefinedOrUnknownSVal>());
+
+ if (!ValuesEqual.getAs<DefinedSVal>())
+ return State;
+
+ State = C.getConstraintManager().assume(
+ State, ValuesEqual.castAs<DefinedSVal>(), true);
+
+ return State;
+}
+
+void ento::registerGTestChecker(CheckerManager &Mgr) {
+ const LangOptions &LangOpts = Mgr.getLangOpts();
+ // gtest is a C++ API so there is no sense running the checker
+ // if not compiling for C++.
+ if (!LangOpts.CPlusPlus)
+ return;
+
+ Mgr.registerChecker<GTestChecker>();
+}
Added: cfe/trunk/test/Analysis/gtest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/gtest.cpp?rev=290143&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/gtest.cpp (added)
+++ cfe/trunk/test/Analysis/gtest.cpp Mon Dec 19 16:50:31 2016
@@ -0,0 +1,142 @@
+//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
+
+// expected-no-diagnostics
+
+namespace std {
+ class string {
+ public:
+ ~string();
+ const char *c_str();
+ };
+}
+
+namespace testing {
+
+class Message { };
+class TestPartResult {
+ public:
+ enum Type {
+ kSuccess,
+ kNonFatalFailure,
+ kFatalFailure
+ };
+};
+
+namespace internal {
+
+class AssertHelper {
+ public:
+ AssertHelper(TestPartResult::Type type, const char* file, int line,
+ const char* message);
+ ~AssertHelper();
+ void operator=(const Message& message) const;
+};
+
+
+template <typename T>
+struct AddReference { typedef T& type; };
+template <typename T>
+struct AddReference<T&> { typedef T& type; };
+template <typename From, typename To>
+class ImplicitlyConvertible {
+ private:
+ static typename AddReference<From>::type MakeFrom();
+ static char Helper(To);
+ static char (&Helper(...))[2];
+ public:
+ static const bool value =
+ sizeof(Helper(ImplicitlyConvertible::MakeFrom())) == 1;
+};
+template <typename From, typename To>
+const bool ImplicitlyConvertible<From, To>::value;
+template<bool> struct EnableIf;
+template<> struct EnableIf<true> { typedef void type; };
+
+} // end internal
+
+
+class AssertionResult {
+public:
+
+ // The implementation for the copy constructor is not exposed in the
+ // interface.
+ AssertionResult(const AssertionResult& other);
+
+#if defined(GTEST_VERSION_1_8_AND_LATER)
+ template <typename T>
+ explicit AssertionResult(
+ const T& success,
+ typename internal::EnableIf<
+ !internal::ImplicitlyConvertible<T, AssertionResult>::value>::type*
+ /*enabler*/ = 0)
+ : success_(success) {}
+#else
+ explicit AssertionResult(bool success) : success_(success) {}
+#endif
+
+ operator bool() const { return success_; }
+
+ // The actual AssertionResult does not have an explicit destructor, but
+ // it does have a non-trivial member veriable, so we add a destructor here
+ // to force temporary cleanups.
+ ~AssertionResult();
+private:
+
+ bool success_;
+};
+
+namespace internal {
+std::string GetBoolAssertionFailureMessage(
+ const AssertionResult& assertion_result,
+ const char* expression_text,
+ const char* actual_predicate_value,
+ const char* expected_predicate_value);
+} // end internal
+
+} // end testing
+
+#define GTEST_MESSAGE_AT_(file, line, message, result_type) \
+ ::testing::internal::AssertHelper(result_type, file, line, message) \
+ = ::testing::Message()
+
+#define GTEST_MESSAGE_(message, result_type) \
+ GTEST_MESSAGE_AT_(__FILE__, __LINE__, message, result_type)
+
+#define GTEST_FATAL_FAILURE_(message) \
+ return GTEST_MESSAGE_(message, ::testing::TestPartResult::kFatalFailure)
+
+#define GTEST_NONFATAL_FAILURE_(message) \
+ GTEST_MESSAGE_(message, ::testing::TestPartResult::kNonFatalFailure)
+
+# define GTEST_AMBIGUOUS_ELSE_BLOCKER_ switch (0) case 0: default:
+
+#define GTEST_TEST_BOOLEAN_(expression, text, actual, expected, fail) \
+ GTEST_AMBIGUOUS_ELSE_BLOCKER_ \
+ if (const ::testing::AssertionResult gtest_ar_ = \
+ ::testing::AssertionResult(expression)) \
+ ; \
+ else \
+ fail(::testing::internal::GetBoolAssertionFailureMessage(\
+ gtest_ar_, text, #actual, #expected).c_str())
+
+#define EXPECT_TRUE(condition) \
+ GTEST_TEST_BOOLEAN_((condition), #condition, false, true, \
+ GTEST_NONFATAL_FAILURE_)
+#define ASSERT_TRUE(condition) \
+ GTEST_TEST_BOOLEAN_((condition), #condition, false, true, \
+ GTEST_FATAL_FAILURE_)
+
+#define ASSERT_FALSE(condition) \
+ GTEST_TEST_BOOLEAN_(!(condition), #condition, true, false, \
+ GTEST_FATAL_FAILURE_)
+
+void testAssertTrue(int *p) {
+ ASSERT_TRUE(p != nullptr);
+ EXPECT_TRUE(1 == *p); // no-warning
+}
+
+void testAssertFalse(int *p) {
+ ASSERT_FALSE(p == nullptr);
+ EXPECT_TRUE(1 == *p); // no-warning
+}
Modified: cfe/trunk/test/Driver/analyzer-target-enabled-checkers.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/analyzer-target-enabled-checkers.cpp?rev=290143&r1=290142&r2=290143&view=diff
==============================================================================
--- cfe/trunk/test/Driver/analyzer-target-enabled-checkers.cpp (original)
+++ cfe/trunk/test/Driver/analyzer-target-enabled-checkers.cpp Mon Dec 19 16:50:31 2016
@@ -4,6 +4,7 @@
// RUN: %clang -### -target x86_64-apple-darwin10 --analyze %s 2>&1 | FileCheck --check-prefix=CHECK-DARWIN %s
// CHECK-DARWIN: "-analyzer-checker=core"
+// CHECK-DARWIN-SAME: "-analyzer-checker=apiModeling"
// CHECK-DARWIN-SAME: "-analyzer-checker=unix"
// CHECK-DARWIN-SAME: "-analyzer-checker=osx"
// CHECK-DARWIN-SAME: "-analyzer-checker=deadcode"
@@ -21,6 +22,7 @@
// RUN: %clang -### -target x86_64-unknown-linux --analyze %s 2>&1 | FileCheck --check-prefix=CHECK-LINUX %s
// CHECK-LINUX: "-analyzer-checker=core"
+// CHECK-LINUX-SAME: "-analyzer-checker=apiModeling"
// CHECK-LINUX-SAME: "-analyzer-checker=unix"
// CHECK-LINUX-NOT: "-analyzer-checker=osx"
// CHECK-LINUX-SAME: "-analyzer-checker=deadcode"
@@ -38,6 +40,7 @@
// RUN: %clang -### -target x86_64-windows --analyze %s 2>&1 | FileCheck --check-prefix=CHECK-WINDOWS %s
// CHECK-WINDOWS: "-analyzer-checker=core"
+// CHECK-WINDOWS-SAME: "-analyzer-checker=apiModeling"
// CHECK-WINDOWS-SAME: "-analyzer-checker=unix.API"
// CHECK-WINDOWS-SAME: "-analyzer-checker=unix.Malloc"
// CHECK-WINDOWS-SAME: "-analyzer-checker=unix.MallocSizeof"
More information about the cfe-commits
mailing list