r365103 - [analyzer] ReturnValueChecker: Model the guaranteed boolean return value of function calls
Csaba Dabis via cfe-commits
cfe-commits at lists.llvm.org
Wed Jul 3 17:50:50 PDT 2019
Author: charusso
Date: Wed Jul 3 17:50:50 2019
New Revision: 365103
URL: http://llvm.org/viewvc/llvm-project?rev=365103&view=rev
Log:
[analyzer] ReturnValueChecker: Model the guaranteed boolean return value 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
Added:
cfe/trunk/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp
cfe/trunk/test/Analysis/return-value-guaranteed.cpp
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.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=365103&r1=365102&r2=365103&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td Wed Jul 3 17:50:50 2019
@@ -274,6 +274,10 @@ def NullableReturnedFromNonnullChecker :
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>;
Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h?rev=365103&r1=365102&r2=365103&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h Wed Jul 3 17:50:50 2019
@@ -219,24 +219,34 @@ public:
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);
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt?rev=365103&r1=365102&r2=365103&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt Wed Jul 3 17:50:50 2019
@@ -83,6 +83,7 @@ add_clang_library(clangStaticAnalyzerChe
RetainCountChecker/RetainCountDiagnostics.cpp
ReturnPointerRangeChecker.cpp
ReturnUndefChecker.cpp
+ ReturnValueChecker.cpp
RunLoopAutoreleaseLeakChecker.cpp
SimpleStreamChecker.cpp
SmartPtrModeling.cpp
Added: cfe/trunk/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp?rev=365103&view=auto
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp (added)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp Wed Jul 3 17:50:50 2019
@@ -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;
+}
Modified: cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp?rev=365103&r1=365102&r2=365103&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp Wed Jul 3 17:50:50 2019
@@ -780,6 +780,9 @@ PathDiagnosticLocation::create(const Pro
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");
}
Added: cfe/trunk/test/Analysis/return-value-guaranteed.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/return-value-guaranteed.cpp?rev=365103&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/return-value-guaranteed.cpp (added)
+++ cfe/trunk/test/Analysis/return-value-guaranteed.cpp Wed Jul 3 17:50:50 2019
@@ -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 at -1 {{Assuming the condition is false}}
+ // class-note at -2 {{Taking false branch}}
+ return MCAsmParser::Error();
+ }
+
+ F.Field = 0;
+ // class-note at -1 {{The value 0 is assigned to 'F.Field'}}
+ return !MCAsmParser::Error();
+ // class-note at -1 {{'MCAsmParser::Error' returns true}}
+}
+
+bool parseFile() {
+ Foo F;
+ if (parseFoo(F)) {
+ // class-note at -1 {{Calling 'parseFoo'}}
+ // class-note at -2 {{Returning from 'parseFoo'}}
+ // class-note at -3 {{Taking false branch}}
+ return true;
+ }
+
+ if (F.Field == 0) {
+ // class-note at -1 {{Field 'Field' is equal to 0}}
+ // class-note at -2 {{Taking true branch}}
+
+ // no-warning: "The left operand of '==' is a garbage value" was here.
+ doSomething();
+ }
+
+ (void)(1 / F.Field);
+ // class-warning at -1 {{Division by zero}}
+ // class-note at -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 at -1 {{Assuming the condition is false}}
+ // class-note at -2 {{Taking false branch}}
+ return !MCAsmParser::Error();
+ }
+
+ F.Field = 0;
+ // class-note at -1 {{The value 0 is assigned to 'F.Field'}}
+ return MCAsmParser::Error();
+ // class-note at -1 {{Calling 'MCAsmParser::Error'}}
+ // class-note at -2 {{Returning from 'MCAsmParser::Error'}}
+}
+
+bool parseFile() {
+ Foo F;
+ if (parseFoo(F)) {
+ // class-note at -1 {{Calling 'parseFoo'}}
+ // class-note at -2 {{Returning from 'parseFoo'}}
+ // class-note at -3 {{Taking false branch}}
+ return true;
+ }
+
+ (void)(1 / F.Field);
+ // class-warning at -1 {{Division by zero}}
+ // class-note at -2 {{Division by zero}}
+ return false;
+}
+} // namespace test_classes
More information about the cfe-commits
mailing list