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; -// 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, Hidden; @@ -274,6 +274,10 @@ let ParentPackage = APIModeling in { +def ReturnValueChecker : Checker<"ReturnValue">, + HelpText<"Model the guaranteed boolean return value of function calls">, + Documentation; + def StdCLibraryFunctionsChecker : Checker<"StdCLibraryFunctions">, HelpText<"Improve modeling of the C standard library functions">, Documentation; 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 lookup(const CallEvent &Call) const { // Slow path: linear lookup. // TODO: Implement some sort of fast path. for (const std::pair &I : LinearMap) if (Call.isCalled(I.first)) - return &I.second; + return I.second; - return nullptr; + return {}; } }; 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 &&Cb) { + const NoteTag *getNoteTag(std::function &&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/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/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 { +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 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 Value = CDM.lookup(Call); + if (!Value) + return; + + // Get the symbolic return value. + ProgramStateRef State = C.getState(); + SVal RetV = Call.getReturnValue(); + Optional RetDV = RetV.getAs(); + 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(); +} + +bool ento::shouldRegisterReturnValueChecker(const LangOptions &LO) { + return true; +} 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/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 LookupResult = CDM.lookup(*Call); // Check that we've found the function in the map // with the correct description. assert(LookupResult && *LookupResult);