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; 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; + def Debug : Package<"debug">; def CloneDetectionAlpha : Package<"clone">, InPackage, Hidden; @@ -643,6 +650,17 @@ HelpText<"Check code for LLVM codebase conventions">, DescFile<"LLVMConventionsChecker.cpp">; + + +//===----------------------------------------------------------------------===// +// Checkers modeling Google APIs. +//===----------------------------------------------------------------------===// + +def GTestChecker : Checker<"GTest">, + InPackage, + HelpText<"Model gtest assertion APIs">, + DescFile<"GTestChecker.cpp">; + //===----------------------------------------------------------------------===// // Debugging checkers (for analyzer development). //===----------------------------------------------------------------------===// Index: lib/Driver/Tools.cpp =================================================================== --- lib/Driver/Tools.cpp +++ lib/Driver/Tools.cpp @@ -4232,6 +4232,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: 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/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 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, +// "", +// , +// ::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 { + + 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 + // AssertionResult(const T& success, + // typename internal::EnableIf< + // !internal::ImplicitlyConvertible::value>::type*) + + SVal BooleanArgVal = Call->getArgSVal(0); + if (Call->getDecl()->getParamDecl(0)->getType()->getAs()) { + // We have v1.8+, so load the value from the reference. + if (!BooleanArgVal.getAs()) + return; + BooleanArgVal = C.getState()->getSVal(BooleanArgVal.castAs()); + } + + 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(&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(Result.front()); + if (!SuccessField) + return UnknownVal(); + + Optional FieldLoc = + State->getLValue(SuccessField, Instance).getAs(); + 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() || + !Val2.getAs()) + return State; + + auto ValuesEqual = + C.getSValBuilder().evalEQ(State, Val1.castAs(), + Val2.castAs()); + + if (!ValuesEqual.getAs()) + return State; + + State = C.getConstraintManager().assume( + State, ValuesEqual.castAs(), 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(); +} 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 +struct AddReference { typedef T& type; }; +template +struct AddReference { typedef T& type; }; +template +class ImplicitlyConvertible { + private: + static typename AddReference::type MakeFrom(); + static char Helper(To); + static char (&Helper(...))[2]; + public: + static const bool value = + sizeof(Helper(ImplicitlyConvertible::MakeFrom())) == 1; +}; +template +const bool ImplicitlyConvertible::value; +template struct EnableIf; +template<> struct EnableIf { 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 + explicit AssertionResult( + const T& success, + typename internal::EnableIf< + !internal::ImplicitlyConvertible::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: 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"