dcoughlin updated the summary for this revision.
dcoughlin updated this revision to Diff 81651.
dcoughlin marked an inline comment as done.
dcoughlin added a comment.
Update to address Aleksei's comments.
https://reviews.llvm.org/D27773
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/Driver/Tools.cpp
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/GTestChecker.cpp
test/Analysis/gtest.cpp
test/Driver/analyzer-target-enabled-checkers.cpp
Index: test/Driver/analyzer-target-enabled-checkers.cpp
===================================================================
--- test/Driver/analyzer-target-enabled-checkers.cpp
+++ test/Driver/analyzer-target-enabled-checkers.cpp
@@ -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"
Index: test/Analysis/gtest.cpp
===================================================================
--- /dev/null
+++ test/Analysis/gtest.cpp
@@ -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
+}
Index: lib/StaticAnalyzer/Checkers/GTestChecker.cpp
===================================================================
--- /dev/null
+++ lib/StaticAnalyzer/Checkers/GTestChecker.cpp
@@ -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>();
+}
Index: lib/StaticAnalyzer/Checkers/CMakeLists.txt
===================================================================
--- lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -37,6 +37,7 @@
ExprInspectionChecker.cpp
FixedAddressChecker.cpp
GenericTaintChecker.cpp
+ GTestChecker.cpp
IdenticalExprChecker.cpp
IvarInvalidationChecker.cpp
LLVMConventionsChecker.cpp
Index: lib/Driver/Tools.cpp
===================================================================
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -4262,6 +4262,7 @@
// 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");
Index: include/clang/StaticAnalyzer/Checkers/Checkers.td
===================================================================
--- include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -79,6 +79,13 @@
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 @@
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).
//===----------------------------------------------------------------------===//
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
