balazske updated this revision to Diff 303444.
balazske added a comment.
Small fixes in test.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D90691/new/
https://reviews.llvm.org/D90691
Files:
clang/docs/analyzer/checkers.rst
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
clang/lib/StaticAnalyzer/Checkers/UncheckedReturnValueChecker.cpp
clang/test/Analysis/Inputs/system-header-simulator-cxx.h
clang/test/Analysis/unchecked-return-value.cpp
Index: clang/test/Analysis/unchecked-return-value.cpp
===================================================================
--- /dev/null
+++ clang/test/Analysis/unchecked-return-value.cpp
@@ -0,0 +1,76 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=security.UncheckedReturnValue -verify %s
+
+#include "Inputs/system-header-simulator-cxx.h"
+#include "Inputs/system-header-simulator.h"
+
+extern void extern_f(int);
+int btowc(int);
+
+struct S {
+ int getInt() const;
+};
+
+int f1(int X) {
+ scanf("%*d"); // expected-warning {{Return value is not checked in call to 'scanf' [security.UncheckedReturnValue]}}
+ std::scanf("%*d"); // expected-warning {{Return value is not checked in call to 'scanf' [security.UncheckedReturnValue]}}
+ other_ns::scanf("%*d");
+ {
+ scanf("%*d"); // expected-warning {{Return value is not checked in call to 'scanf' [security.UncheckedReturnValue]}}
+ scanf("%*d"); // expected-warning {{Return value is not checked in call to 'scanf' [security.UncheckedReturnValue]}}
+ }
+
+ if (X) {
+ scanf("%*d"); // expected-warning {{Return value is not checked in call to 'scanf' [security.UncheckedReturnValue]}}
+ }
+
+ if (X)
+ scanf("%*d"); // expected-warning {{Return value is not checked in call to 'scanf' [security.UncheckedReturnValue]}}
+ if (scanf("%*d")) {
+ }
+ if (scanf("%*d") > 1) {
+ }
+
+ while (true)
+ scanf("%*d"); // expected-warning {{Return value is not checked in call to 'scanf' [security.UncheckedReturnValue]}}
+ while (scanf("%*d")) {
+ }
+
+ do
+ scanf("%*d"); // expected-warning {{Return value is not checked in call to 'scanf' [security.UncheckedReturnValue]}}
+ while (true);
+ do {
+ } while (scanf("%*d"));
+
+ for (int Y = 1; Y < 10; ++Y)
+ scanf("%*d"); // expected-warning {{Return value is not checked in call to 'scanf' [security.UncheckedReturnValue]}}
+ for (int Y = 1; Y < 10; scanf("%*d")) { // expected-warning {{Return value is not checked in call to 'scanf' [security.UncheckedReturnValue]}}
+ }
+ for (int Y = 1; scanf("%*d"); ++Y) {
+ }
+ int Y = 0;
+ for (scanf("%*d"); Y < 10; ++Y) { // expected-warning {{Return value is not checked in call to 'scanf' [security.UncheckedReturnValue]}}
+ }
+
+ std::vector<int> Vec;
+ for (int I : Vec)
+ scanf("%*d"); // expected-warning {{Return value is not checked in call to 'scanf' [security.UncheckedReturnValue]}}
+
+ switch (scanf("%*d")) {
+ }
+ switch (X) {
+ case 1:
+ scanf("%*d"); // expected-warning {{Return value is not checked in call to 'scanf' [security.UncheckedReturnValue]}}
+ break;
+ }
+
+ // This call is wrapped in an ExprWithCleanups.
+ scanf("", S().getInt()); // expected-warning {{Return value is not checked in call to 'scanf' [security.UncheckedReturnValue]}}
+
+ int Z1 = scanf("%*d");
+ Z1 = scanf("%*d");
+ extern_f(scanf("%*d"));
+
+ btowc(22); // no warning for non-system function
+
+ return scanf("%*d"); // no warning if value is returned
+}
Index: clang/test/Analysis/Inputs/system-header-simulator-cxx.h
===================================================================
--- clang/test/Analysis/Inputs/system-header-simulator-cxx.h
+++ clang/test/Analysis/Inputs/system-header-simulator-cxx.h
@@ -1139,4 +1139,10 @@
// TODO: Add some actual implementation.
};
+int scanf(const char *format, ...);
+
} // namespace std
+
+namespace other_ns {
+int scanf(const char *format, ...);
+}
Index: clang/lib/StaticAnalyzer/Checkers/UncheckedReturnValueChecker.cpp
===================================================================
--- /dev/null
+++ clang/lib/StaticAnalyzer/Checkers/UncheckedReturnValueChecker.cpp
@@ -0,0 +1,178 @@
+//===- 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 UncheckedReturnValueChecker, which checks for calls to functions
+// whose return value has to be observed by the program but is ignored.
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringMap.h"
+
+using namespace clang;
+using namespace ento;
+using namespace ast_matchers;
+
+namespace {
+
+static bool isInNoNamespace(const FunctionDecl *FD) {
+ const DeclContext *DC = FD->getDeclContext();
+ while (DC && DC->isTransparentContext())
+ DC = DC->getParent();
+ assert(DC && "Function should have a declaration context.");
+ return DC->isTranslationUnit();
+}
+
+class UncheckedReturnValueChecker : public Checker<check::ASTCodeBody> {
+public:
+ void checkASTCodeBody(const Decl *D, AnalysisManager &AM,
+ BugReporter &BR) const {
+ auto FoundCall = callExpr().bind("call");
+ auto CallInCompound = compoundStmt(forEach(FoundCall));
+ auto CallInIf = ifStmt(anyOf(hasThen(FoundCall), hasElse(FoundCall)));
+ auto CallInFor = forStmt(anyOf(hasBody(FoundCall), hasLoopInit(FoundCall),
+ hasIncrement(FoundCall)));
+ auto CallInForRange = cxxForRangeStmt(hasBody(FoundCall));
+ auto CallInWhile = whileStmt(hasBody(FoundCall));
+ auto CallInDo = doStmt(hasBody(FoundCall));
+ auto CallInCase = caseStmt(has(FoundCall));
+ auto CallInAnyStmt =
+ stmt(anyOf(CallInCompound, CallInIf, CallInFor, CallInForRange,
+ CallInWhile, CallInDo, CallInCase));
+ auto Matches =
+ match(stmt(findAll(CallInAnyStmt)), *D->getBody(), AM.getASTContext());
+
+ for (BoundNodes Match : Matches) {
+ const auto *CE = Match.getNodeAs<CallExpr>("call");
+ assert(CE);
+ const FunctionDecl *FD = CE->getDirectCallee();
+ if (!FD)
+ continue;
+ FD = FD->getCanonicalDecl();
+
+ if (!AM.getSourceManager().isInSystemHeader(FD->getLocation()))
+ continue;
+ if (!FD->isInStdNamespace() && !isInNoNamespace(FD))
+ continue;
+
+ if (isFunctionToCheck(FD)) {
+ // Issue a warning.
+ SmallString<512> buf;
+ llvm::raw_svector_ostream os(buf);
+ os << "Return value is not checked in call to '" << *FD << "'.";
+
+ PathDiagnosticLocation CELoc = PathDiagnosticLocation::createBegin(
+ CE, BR.getSourceManager(), AM.getAnalysisDeclContext(D));
+
+ BR.EmitBasicReport(D, getCheckerName(), "Unchecked return value",
+ categories::SecurityError, os.str(), CELoc);
+ }
+ }
+ }
+
+private:
+ llvm::StringMap<int> FunctionsToCheck = {
+ {"aligned_alloc", 2}, {"asctime_s", 3}, {"at_quick_exit", 1},
+ {"atexit", 1}, {"bsearch", 5}, {"bsearch_s", 6},
+ {"btowc", 1}, {"c16rtomb", 3}, {"c32rtomb", 3},
+ {"calloc", 2}, {"clock", 0}, {"cnd_broadcast", 1},
+ {"cnd_init", 1}, {"cnd_signal", 1}, {"cnd_timedwait", 3},
+ {"cnd_wait", 2}, {"ctime_s", 3}, {"fclose", 1},
+ {"fflush", 1}, {"fgetc", 1}, {"fgetpos", 2},
+ {"fgets", 3}, {"fgetwc", 1}, {"fopen", 2},
+ {"fopen_s", 3}, {"fprintf", -1}, {"fprintf_s", -1},
+ {"fputc", 2}, {"fputs", 2}, {"fputwc", 2},
+ {"fputws", 2}, {"fread", 4}, {"freopen", 3},
+ {"freopen_s", 4}, {"fscanf", -1}, {"fscanf_s", -1},
+ {"fseek", 3}, {"fsetpos", 2}, {"ftell", 1},
+ {"fwprintf", -1}, {"fwprintf_s", -1}, {"fwrite", 4},
+ {"fwscanf", -1}, {"fwscanf_s", -1}, {"getc", 1},
+ {"getchar", 0}, {"getenv", 1}, {"getenv_s", 4},
+ {"gets_s", 2}, {"getwc", 1}, {"getwchar", 0},
+ {"gmtime", 1}, {"gmtime_s", 2}, {"localtime", 1},
+ {"localtime_s", 2}, {"malloc", 1}, {"mblen", 2},
+ {"mbrlen", 3}, {"mbrtoc16", 4}, {"mbrtoc32", 4},
+ {"mbrtowc", 4}, {"mbsrtowcs", 4}, {"mbsrtowcs_s", 6},
+ {"mbstowcs", 3}, {"mbstowcs_s", 5}, {"mbtowc", 3},
+ {"memchr", 3}, {"mktime", 1}, {"mtx_init", 2},
+ {"mtx_lock", 1}, {"mtx_timedlock", 2}, {"mtx_trylock", 1},
+ {"mtx_unlock", 1}, {"printf_s", -1}, {"putc", 2},
+ {"putwc", 2}, {"raise", 1}, {"realloc", 2},
+ {"remove", 1}, {"rename", 2}, {"setlocale", 2},
+ {"setvbuf", 4}, {"scanf", -1}, {"scanf_s", -1},
+ {"signal", 2}, {"snprintf", -1}, {"snprintf_s", -1},
+ {"snwprintf_s", -1}, {"sprintf", -1}, {"sprintf_s", -1},
+ {"sscanf", -1}, {"sscanf_s", -1}, {"strchr", 2},
+ {"strerror_s", 3}, {"strftime", 4}, {"strpbrk", 2},
+ {"strrchr", 2}, {"strstr", 2}, {"strtod", 2},
+ {"strtof", 2}, {"strtoimax", 3}, {"strtok", 2},
+ {"strtok_s", 4}, {"strtol", 3}, {"strtold", 2},
+ {"strtoll", 3}, {"strtoumax", 3}, {"strtoul", 3},
+ {"strtoull", 3}, {"strxfrm", 3}, {"swprintf", -1},
+ {"swprintf_s", -1}, {"swscanf", -1}, {"swscanf_s", -1},
+ {"thrd_create", 3}, {"thrd_detach", 1}, {"thrd_join", 2},
+ {"thrd_sleep", 2}, {"time", 1}, {"timespec_get", 2},
+ {"tmpfile", 0}, {"tmpfile_s", 1}, {"tmpnam", 1},
+ {"tmpnam_s", 2}, {"tss_create", 2}, {"tss_get", 1},
+ {"tss_set", 2}, {"ungetc", 2}, {"ungetwc", 2},
+ {"vfprintf", 3}, {"vfprintf_s", 3}, {"vfscanf", 3},
+ {"vfscanf_s", 3}, {"vfwprintf", 3}, {"vfwprintf_s", 3},
+ {"vfwscanf", 3}, {"vfwscanf_s", 3}, {"vprintf", 2},
+ {"vprintf_s", 2}, {"vscanf", 2}, {"vscanf_s", 2},
+ {"vsnprintf", 4}, {"vsnprintf_s", 4}, {"vsnwprintf_s", 4},
+ {"vsprintf", 3}, {"vsprintf_s", 4}, {"vsscanf", 3},
+ {"vsscanf_s", 3}, {"vswprintf", 4}, {"vswprintf_s", 4},
+ {"vswscanf", 3}, {"vswscanf_s", 3}, {"vwprintf_s", 2},
+ {"vwscanf", 2}, {"vwscanf_s", 2}, {"wcrtomb", 3},
+ {"wcrtomb_s", 5}, {"wcschr", 2}, {"wcsftime", 4},
+ {"wcspbrk", 2}, {"wcsrchr", 2}, {"wcsrtombs", 4},
+ {"wcsrtombs_s", 5}, {"wcsstr", 2}, {"wcstod", 2},
+ {"wcstof", 2}, {"wcstoimax", 3}, {"wcstok", 3},
+ {"wcstok_s", 4}, {"wcstol", 3}, {"wcstold", 2},
+ {"wcstoll", 3}, {"wcstombs", 3}, {"wcstombs_s", 5},
+ {"wcstoumax", 3}, {"wcstoul", 3}, {"wcstoull", 3},
+ {"wcsxfrm", 3}, {"wctob", 1}, {"wctomb", 2},
+ {"wctomb_s", 4}, {"wctrans", 1}, {"wctype", 1},
+ {"wmemchr", 3}, {"wprintf_s", -1}, {"wscanf", -1},
+ {"wscanf_s", -1}};
+
+ bool isFunctionToCheck(const FunctionDecl *FD) const {
+ // Ignore functions without a name.
+ const IdentifierInfo *II = FD->getIdentifier();
+ if (!II)
+ return false;
+
+ auto FoundFunction = FunctionsToCheck.find(II->getName());
+ if (FoundFunction == FunctionsToCheck.end())
+ return false;
+
+ // Value -1 indicates that the function should be variadic.
+ if (FoundFunction->second == -1)
+ return FD->isVariadic();
+
+ // Check parameter count.
+ return FD->getNumParams() == static_cast<unsigned>(FoundFunction->second);
+ }
+};
+
+} // namespace
+
+void ento::registerUncheckedReturnValueChecker(CheckerManager &Mgr) {
+ Mgr.registerChecker<UncheckedReturnValueChecker>();
+}
+
+bool ento::shouldRegisterUncheckedReturnValueChecker(
+ const CheckerManager &mgr) {
+ return true;
+}
Index: clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -109,6 +109,7 @@
TestAfterDivZeroChecker.cpp
TraversalChecker.cpp
TrustNonnullChecker.cpp
+ UncheckedReturnValueChecker.cpp
UndefBranchChecker.cpp
UndefCapturedBlockVarChecker.cpp
UndefResultChecker.cpp
Index: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
===================================================================
--- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -926,6 +926,11 @@
Dependencies<[SecuritySyntaxChecker]>,
Documentation<HasDocumentation>;
+def UncheckedReturnValueChecker : Checker<"UncheckedReturnValue">,
+ HelpText<"Warn on uses of functions whose return values must be always "
+ "checked.">,
+ Documentation<HasDocumentation>;
+
} // end "security"
let ParentPackage = POS in {
Index: clang/docs/analyzer/checkers.rst
===================================================================
--- clang/docs/analyzer/checkers.rst
+++ clang/docs/analyzer/checkers.rst
@@ -663,11 +663,29 @@
for (float x = 0.1f; x <= 1.0f; x += 0.1f) {} // warn
}
+security.UncheckedReturnValue
+"""""""""""""""""""""""""""""
+Warn on uses of functions whose return values must be checked (CERT: ERR33-C).
+The detection is limited to the case when the return value of the function
+is completely ignored. The detection is applied in C++ code too.
+List of checked functions is:
+FIXME: Fill the list.
+
+.. code-block:: c
+
+ void f();
+
+ void test() {
+ atexit(f); // warn: return value of atexit should be checked
+ }
+
.. _security-insecureAPI-UncheckedReturn:
security.insecureAPI.UncheckedReturn (C)
""""""""""""""""""""""""""""""""""""""""
Warn on uses of functions whose return values must be always checked.
+This check detects ignored return value at calls to
+'setuid', 'setgid', 'seteuid', 'setegid', 'setreuid', 'setregid'.
.. code-block:: c
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
