Skip to content

Commit

Permalink
[analyzer] ReturnValueChecker: Model the guaranteed boolean return va…
Browse files Browse the repository at this point in the history
…lue of function calls

Summary: It models the known LLVM methods paired with their class.

Reviewers: NoQ, xazax.hun, ravikandhadai, baloghadamsoftware, Szelethus

Reviewed By: NoQ

Subscribers: dschuff, aheejin, mgorny, szepet, rnkovacs, a.sidorin,
             mikhail.ramalho, donat.nagy, dkrupp, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D63915

llvm-svn: 365103
  • Loading branch information
Charusso committed Jul 4, 2019
1 parent 312f1d7 commit 57835bc
Show file tree
Hide file tree
Showing 6 changed files with 285 additions and 6 deletions.
4 changes: 4 additions & 0 deletions clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
Expand Up @@ -274,6 +274,10 @@ def NullableReturnedFromNonnullChecker : Checker<"NullableReturnedFromNonnull">,

let ParentPackage = APIModeling in {

def ReturnValueChecker : Checker<"ReturnValue">,
HelpText<"Model the guaranteed boolean return value of function calls">,
Documentation<NotDocumented>;

def StdCLibraryFunctionsChecker : Checker<"StdCLibraryFunctions">,
HelpText<"Improve modeling of the C standard library functions">,
Documentation<NotDocumented>;
Expand Down
Expand Up @@ -219,24 +219,34 @@ class CheckerContext {
Eng.getBugReporter().emitReport(std::move(R));
}


/// Produce a program point tag that displays an additional path note
/// to the user. This is a lightweight alternative to the
/// BugReporterVisitor mechanism: instead of visiting the bug report
/// node-by-node to restore the sequence of events that led to discovering
/// a bug, you can add notes as you add your transitions.
const NoteTag *getNoteTag(NoteTag::Callback &&Cb) {
return Eng.getNoteTags().makeNoteTag(std::move(Cb));
///
/// @param Cb Callback with 'BugReporterContext &, BugReport &' parameters.
/// @param IsPrunable Whether the note is prunable. It allows BugReporter
/// to omit the note from the report if it would make the displayed
/// bug path significantly shorter.
const NoteTag *getNoteTag(NoteTag::Callback &&Cb, bool IsPrunable = false) {
return Eng.getNoteTags().makeNoteTag(std::move(Cb), IsPrunable);
}

/// A shorthand version of getNoteTag that doesn't require you to accept
/// the BugReporterContext arguments when you don't need it.
const NoteTag *getNoteTag(std::function<std::string(BugReport &)> &&Cb) {
///
/// @param Cb Callback only with 'BugReport &' parameter.
/// @param IsPrunable Whether the note is prunable. It allows BugReporter
/// to omit the note from the report if it would make the displayed
/// bug path significantly shorter.
const NoteTag *getNoteTag(std::function<std::string(BugReport &)> &&Cb,
bool IsPrunable = false) {
return getNoteTag(
[Cb](BugReporterContext &, BugReport &BR) { return Cb(BR); });
[Cb](BugReporterContext &, BugReport &BR) { return Cb(BR); },
IsPrunable);
}


/// Returns the word that should be used to refer to the declaration
/// in the report.
StringRef getDeclDescription(const Decl *D);
Expand Down
1 change: 1 addition & 0 deletions clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
Expand Up @@ -83,6 +83,7 @@ add_clang_library(clangStaticAnalyzerCheckers
RetainCountChecker/RetainCountDiagnostics.cpp
ReturnPointerRangeChecker.cpp
ReturnUndefChecker.cpp
ReturnValueChecker.cpp
RunLoopAutoreleaseLeakChecker.cpp
SimpleStreamChecker.cpp
SmartPtrModeling.cpp
Expand Down
170 changes: 170 additions & 0 deletions clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp
@@ -0,0 +1,170 @@
//===- ReturnValueChecker - Applies guaranteed return values ----*- C++ -*-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
//
// This defines ReturnValueChecker, which checks for calls with guaranteed
// boolean return value. It ensures the return value of each function call.
//
//===----------------------------------------------------------------------===//

#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/SmallVector.h"

using namespace clang;
using namespace ento;

namespace {
class ReturnValueChecker : public Checker<check::PostCall, check::EndFunction> {
public:
// It sets the predefined invariant ('CDM') if the current call not break it.
void checkPostCall(const CallEvent &Call, CheckerContext &C) const;

// It reports whether a predefined invariant ('CDM') is broken.
void checkEndFunction(const ReturnStmt *RS, CheckerContext &C) const;

private:
// The pairs are in the following form: {{{class, call}}, return value}
const CallDescriptionMap<bool> CDM = {
// These are known in the LLVM project: 'Error()'
{{{"ARMAsmParser", "Error"}}, true},
{{{"HexagonAsmParser", "Error"}}, true},
{{{"LLLexer", "Error"}}, true},
{{{"LLParser", "Error"}}, true},
{{{"MCAsmParser", "Error"}}, true},
{{{"MCAsmParserExtension", "Error"}}, true},
{{{"TGParser", "Error"}}, true},
{{{"X86AsmParser", "Error"}}, true},
// 'TokError()'
{{{"LLParser", "TokError"}}, true},
{{{"MCAsmParser", "TokError"}}, true},
{{{"MCAsmParserExtension", "TokError"}}, true},
{{{"TGParser", "TokError"}}, true},
// 'error()'
{{{"MIParser", "error"}}, true},
{{{"WasmAsmParser", "error"}}, true},
{{{"WebAssemblyAsmParser", "error"}}, true},
// Other
{{{"AsmParser", "printError"}}, true}};
};
} // namespace

static std::string getName(const CallEvent &Call) {
std::string Name = "";
if (const auto *MD = dyn_cast<CXXMethodDecl>(Call.getDecl()))
if (const CXXRecordDecl *RD = MD->getParent())
Name += RD->getNameAsString() + "::";

Name += Call.getCalleeIdentifier()->getName();
return Name;
}

// The predefinitions ('CDM') could break due to the ever growing code base.
// Check for the expected invariants and see whether they apply.
static Optional<bool> isInvariantBreak(bool ExpectedValue, SVal ReturnV,
CheckerContext &C) {
auto ReturnDV = ReturnV.getAs<DefinedOrUnknownSVal>();
if (!ReturnDV)
return None;

if (ExpectedValue)
return C.getState()->isNull(*ReturnDV).isConstrainedTrue();

return C.getState()->isNull(*ReturnDV).isConstrainedFalse();
}

void ReturnValueChecker::checkPostCall(const CallEvent &Call,
CheckerContext &C) const {
const bool *RawExpectedValue = CDM.lookup(Call);
if (!RawExpectedValue)
return;

SVal ReturnV = Call.getReturnValue();
bool ExpectedValue = *RawExpectedValue;
Optional<bool> IsInvariantBreak = isInvariantBreak(ExpectedValue, ReturnV, C);
if (!IsInvariantBreak)
return;

// If the invariant is broken it is reported by 'checkEndFunction()'.
if (*IsInvariantBreak)
return;

std::string Name = getName(Call);
const NoteTag *CallTag = C.getNoteTag(
[Name, ExpectedValue](BugReport &) -> std::string {
SmallString<128> Msg;
llvm::raw_svector_ostream Out(Msg);

Out << '\'' << Name << "' returns "
<< (ExpectedValue ? "true" : "false");
return Out.str();
},
/*IsPrunable=*/true);

ProgramStateRef State = C.getState();
State = State->assume(ReturnV.castAs<DefinedOrUnknownSVal>(), ExpectedValue);
C.addTransition(State, CallTag);
}

void ReturnValueChecker::checkEndFunction(const ReturnStmt *RS,
CheckerContext &C) const {
if (!RS || !RS->getRetValue())
return;

// We cannot get the caller in the top-frame.
const StackFrameContext *SFC = C.getStackFrame();
if (C.getStackFrame()->inTopFrame())
return;

ProgramStateRef State = C.getState();
CallEventManager &CMgr = C.getStateManager().getCallEventManager();
CallEventRef<> Call = CMgr.getCaller(SFC, State);
if (!Call)
return;

const bool *RawExpectedValue = CDM.lookup(*Call);
if (!RawExpectedValue)
return;

SVal ReturnV = State->getSVal(RS->getRetValue(), C.getLocationContext());
bool ExpectedValue = *RawExpectedValue;
Optional<bool> IsInvariantBreak = isInvariantBreak(ExpectedValue, ReturnV, C);
if (!IsInvariantBreak)
return;

// If the invariant is appropriate it is reported by 'checkPostCall()'.
if (!*IsInvariantBreak)
return;

std::string Name = getName(*Call);
const NoteTag *CallTag = C.getNoteTag(
[Name, ExpectedValue](BugReport &BR) -> std::string {
SmallString<128> Msg;
llvm::raw_svector_ostream Out(Msg);

// The following is swapped because the invariant is broken.
Out << '\'' << Name << "' returns "
<< (ExpectedValue ? "false" : "true");

return Out.str();
},
/*IsPrunable=*/false);

C.addTransition(State, CallTag);
}

void ento::registerReturnValueChecker(CheckerManager &Mgr) {
Mgr.registerChecker<ReturnValueChecker>();
}

bool ento::shouldRegisterReturnValueChecker(const LangOptions &LO) {
return true;
}
3 changes: 3 additions & 0 deletions clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
Expand Up @@ -780,6 +780,9 @@ PathDiagnosticLocation::create(const ProgramPoint& P,
NewAllocElt->getAllocatorExpr()->getBeginLoc(), SMng);
}
llvm_unreachable("Unexpected CFG element at front of block");
} else if (Optional<FunctionExitPoint> FE = P.getAs<FunctionExitPoint>()) {
return PathDiagnosticLocation(FE->getStmt(), SMng,
FE->getLocationContext());
} else {
llvm_unreachable("Unexpected ProgramPoint");
}
Expand Down
91 changes: 91 additions & 0 deletions clang/test/Analysis/return-value-guaranteed.cpp
@@ -0,0 +1,91 @@
// RUN: %clang_analyze_cc1 \
// RUN: -analyzer-checker=core,apiModeling.ReturnValue \
// RUN: -analyzer-output=text -verify=class %s

struct Foo { int Field; };
bool problem();
void doSomething();

// We predefined the return value of 'MCAsmParser::Error' as true and we cannot
// take the false-branches which leads to a "garbage value" false positive.
namespace test_classes {
struct MCAsmParser {
static bool Error();
};

bool parseFoo(Foo &F) {
if (problem()) {
// class-note@-1 {{Assuming the condition is false}}
// class-note@-2 {{Taking false branch}}
return MCAsmParser::Error();
}

F.Field = 0;
// class-note@-1 {{The value 0 is assigned to 'F.Field'}}
return !MCAsmParser::Error();
// class-note@-1 {{'MCAsmParser::Error' returns true}}
}

bool parseFile() {
Foo F;
if (parseFoo(F)) {
// class-note@-1 {{Calling 'parseFoo'}}
// class-note@-2 {{Returning from 'parseFoo'}}
// class-note@-3 {{Taking false branch}}
return true;
}

if (F.Field == 0) {
// class-note@-1 {{Field 'Field' is equal to 0}}
// class-note@-2 {{Taking true branch}}

// no-warning: "The left operand of '==' is a garbage value" was here.
doSomething();
}

(void)(1 / F.Field);
// class-warning@-1 {{Division by zero}}
// class-note@-2 {{Division by zero}}
return false;
}
} // namespace test_classes


// We predefined 'MCAsmParser::Error' as returning true, but now it returns
// false, which breaks our invariant. Test the notes.
namespace test_break {
struct MCAsmParser {
static bool Error() {
return false; // class-note {{'MCAsmParser::Error' returns false}}
}
};

bool parseFoo(Foo &F) {
if (problem()) {
// class-note@-1 {{Assuming the condition is false}}
// class-note@-2 {{Taking false branch}}
return !MCAsmParser::Error();
}

F.Field = 0;
// class-note@-1 {{The value 0 is assigned to 'F.Field'}}
return MCAsmParser::Error();
// class-note@-1 {{Calling 'MCAsmParser::Error'}}
// class-note@-2 {{Returning from 'MCAsmParser::Error'}}
}

bool parseFile() {
Foo F;
if (parseFoo(F)) {
// class-note@-1 {{Calling 'parseFoo'}}
// class-note@-2 {{Returning from 'parseFoo'}}
// class-note@-3 {{Taking false branch}}
return true;
}

(void)(1 / F.Field);
// class-warning@-1 {{Division by zero}}
// class-note@-2 {{Division by zero}}
return false;
}
} // namespace test_classes

0 comments on commit 57835bc

Please sign in to comment.