Charusso updated this revision to Diff 207366.
Charusso marked 8 inline comments as done.
Charusso edited the summary of this revision.
Charusso added a comment.
Herald added subscribers: aheejin, dschuff.
- Revert the `Calls` option. It turned out too difficult to create. (May it
useless, as we have tons of similar checkers without any option.)
- Halve the test. We do not want to catch only a call, we have to catch the
class as well to prevent `error()`s like NoReturnFunctionChecker produce.
- Prunable notes added to `CheckerContext`.
- `CallDescriptionMap::lookup()` now `Optional`.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63915/new/
https://reviews.llvm.org/D63915
Files:
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp
clang/test/Analysis/return-value-guaranteed.cpp
clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
Index: clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
===================================================================
--- clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
+++ clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
@@ -33,7 +33,7 @@
CallEventRef<> Call =
Eng.getStateManager().getCallEventManager().getCall(CE, State, SFC);
- const bool *LookupResult = CDM.lookup(*Call);
+ Optional<bool> LookupResult = CDM.lookup(*Call);
// Check that we've found the function in the map
// with the correct description.
assert(LookupResult && *LookupResult);
Index: clang/test/Analysis/return-value-guaranteed.cpp
===================================================================
--- /dev/null
+++ clang/test/Analysis/return-value-guaranteed.cpp
@@ -0,0 +1,51 @@
+// 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 {{'Error' always 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
Index: clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp
===================================================================
--- /dev/null
+++ clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp
@@ -0,0 +1,100 @@
+//===- 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/Basic/IdentifierTable.h"
+#include "clang/Driver/DriverDiagnostic.h"
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.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"
+#include "llvm/ADT/StringExtras.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+class ReturnValueChecker : public Checker<check::PostCall> {
+public:
+ void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
+
+private:
+ // These are known in the LLVM project.
+ // The pairs are in the following form: {{{class, call}}, return value}
+ CallDescriptionMap<bool> CDM = {
+ // '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},
+ // FIXME: 'error()': NoReturnFunctionChecker overlap.
+ {{{"MIParser", "error"}}, true},
+ {{{"WasmAsmParser", "error"}}, true},
+ {{{"WebAssemblyAsmParser", "error"}}, true},
+ // Other
+ {{{"AsmParser", "printError"}}, true}};
+};
+} // namespace
+
+void ReturnValueChecker::checkPostCall(const CallEvent &Call,
+ CheckerContext &C) const {
+ // Get the concrete return value.
+ Optional<bool> Value = CDM.lookup(Call);
+ if (!Value)
+ return;
+
+ // Get the symbolic return value.
+ ProgramStateRef State = C.getState();
+ SVal RetV = Call.getReturnValue();
+ Optional<DefinedOrUnknownSVal> RetDV = RetV.getAs<DefinedOrUnknownSVal>();
+ if (!RetDV.hasValue())
+ return;
+
+ // Create a note.
+ StringRef Name = Call.getCalleeIdentifier()->getName();
+ const NoteTag *CallTag = C.getNoteTag(
+ [Name, Value](BugReport &BR) -> std::string {
+ SmallString<128> Msg;
+ llvm::raw_svector_ostream Out(Msg);
+
+ Out << '\'' << Name << "' always returns "
+ << (*Value ? "true" : "false");
+
+ return Out.str();
+ },
+ /*IsPrunable=*/true);
+
+ // Set the concrete return value.
+ State = State->assume(*RetDV, *Value);
+ C.addTransition(State, CallTag);
+}
+
+void ento::registerReturnValueChecker(CheckerManager &Mgr) {
+ Mgr.registerChecker<ReturnValueChecker>();
+}
+
+bool ento::shouldRegisterReturnValueChecker(const LangOptions &LO) {
+ return true;
+}
Index: clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -83,6 +83,7 @@
RetainCountChecker/RetainCountDiagnostics.cpp
ReturnPointerRangeChecker.cpp
ReturnUndefChecker.cpp
+ ReturnValueChecker.cpp
RunLoopAutoreleaseLeakChecker.cpp
SimpleStreamChecker.cpp
SmartPtrModeling.cpp
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
@@ -225,18 +225,19 @@
/// 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));
+ 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) {
+ 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);
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
@@ -1116,14 +1116,14 @@
CallDescriptionMap(const CallDescriptionMap &) = delete;
CallDescriptionMap &operator=(const CallDescription &) = delete;
- const T *lookup(const CallEvent &Call) const {
+ Optional<T> lookup(const CallEvent &Call) const {
// Slow path: linear lookup.
// TODO: Implement some sort of fast path.
for (const std::pair<CallDescription, T> &I : LinearMap)
if (Call.isCalled(I.first))
- return &I.second;
+ return I.second;
- return nullptr;
+ return {};
}
};
Index: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
===================================================================
--- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -95,9 +95,9 @@
def LLVM : Package<"llvm">;
def LLVMAlpha : Package<"llvm">, ParentPackage<Alpha>;
-// 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 target triple.
+// The APIModeling package is for checkers that model APIs. These checkers are
+// always turned on; this package is intended for API modeling that is not
+// controlled by the target triple.
def APIModeling : Package<"apiModeling">, Hidden;
def GoogleAPIModeling : Package<"google">, ParentPackage<APIModeling>, Hidden;
@@ -274,6 +274,10 @@
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>;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
