diff options
author | Devin Coughlin <dcoughlin@apple.com> | 2016-12-19 22:50:31 +0000 |
---|---|---|
committer | Devin Coughlin <dcoughlin@apple.com> | 2016-12-19 22:50:31 +0000 |
commit | 8beac28564e9769efa9f473bafe6a5ff8d0e88b3 (patch) | |
tree | b83b5e52b01bb2bc9eafde7cbefbc0052c15fa26 /clang/lib | |
parent | fb7dd86fd667fa24dfb2bc2bb323d976058816e8 (diff) | |
download | bcm5719-llvm-8beac28564e9769efa9f473bafe6a5ff8d0e88b3.tar.gz bcm5719-llvm-8beac28564e9769efa9f473bafe6a5ff8d0e88b3.zip |
[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
llvm-svn: 290143
Diffstat (limited to 'clang/lib')
-rw-r--r-- | clang/lib/Driver/Tools.cpp | 1 | ||||
-rw-r--r-- | clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt | 1 | ||||
-rw-r--r-- | clang/lib/StaticAnalyzer/Checkers/GTestChecker.cpp | 287 |
3 files changed, 289 insertions, 0 deletions
diff --git a/clang/lib/Driver/Tools.cpp b/clang/lib/Driver/Tools.cpp index 251187a25c1..f4cab2a8882 100644 --- a/clang/lib/Driver/Tools.cpp +++ b/clang/lib/Driver/Tools.cpp @@ -4263,6 +4263,7 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA, // 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"); diff --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt index 1038a9685db..41415f0376c 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -37,6 +37,7 @@ add_clang_library(clangStaticAnalyzerCheckers ExprInspectionChecker.cpp FixedAddressChecker.cpp GenericTaintChecker.cpp + GTestChecker.cpp IdenticalExprChecker.cpp IvarInvalidationChecker.cpp LLVMConventionsChecker.cpp diff --git a/clang/lib/StaticAnalyzer/Checkers/GTestChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/GTestChecker.cpp new file mode 100644 index 00000000000..eb7c2874b94 --- /dev/null +++ b/clang/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>(); +} |