balazske updated this revision to Diff 291238.
balazske added a comment.
Added entry to release notes and fixed wrong comment in test headers.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87449/new/
https://reviews.llvm.org/D87449
Files:
clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
clang-tools-extra/clang-tidy/cert/CMakeLists.txt
clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp
clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/cert-sig30-c.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/signal.h
clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/stdlib.h
clang-tools-extra/test/clang-tidy/checkers/cert-sig30-c.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/cert-sig30-c.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cert-sig30-c.cpp
@@ -0,0 +1,72 @@
+// RUN: %check_clang_tidy %s cert-sig30-c %t -- -- -isystem %S/Inputs/Headers
+
+#include "signal.h"
+#include "stdio.h"
+#include "stdlib.h"
+
+void handler_abort(int) {
+ abort();
+}
+
+void handler__Exit(int) {
+ _Exit(0);
+}
+
+void handler_quick_exit(int) {
+ quick_exit(0);
+}
+
+void handler_other(int) {
+ printf("1234");
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Signal handler potentially calls non asynchronous-safe function. This may result in undefined behavior. [cert-sig30-c]
+}
+
+void handler_signal(int) {
+ // FIXME: It is only OK to call signal with the current signal number.
+ signal(0, SIG_DFL);
+}
+
+void f_ok() {
+ abort();
+}
+
+void f_bad() {
+ printf("1234");
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Signal handler potentially calls non asynchronous-safe function. This may result in undefined behavior. [cert-sig30-c]
+}
+
+void f_extern();
+
+void handler_ok(int) {
+ f_ok();
+ f_extern();
+}
+
+void handler_bad(int) {
+ f_bad();
+}
+
+// Function called "signal" that is not to be recognized by the checker.
+typedef void (*callback_t)(int);
+void signal(int, callback_t, int);
+
+void test() {
+ signal(SIGINT, handler_abort);
+ signal(SIGINT, handler__Exit);
+ signal(SIGINT, handler_quick_exit);
+ signal(SIGINT, handler_signal);
+ signal(SIGINT, handler_other);
+
+ signal(SIGINT, handler_ok);
+ signal(SIGINT, handler_bad);
+
+ signal(SIGINT, quick_exit);
+ signal(SIGINT, other_call);
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: Signal handler potentially calls non asynchronous-safe function. This may result in undefined behavior. [cert-sig30-c]
+
+ signal(SIGINT, SIG_IGN);
+ signal(SIGINT, SIG_DFL);
+
+ // Do not find problems here.
+ signal(SIGINT, handler_bad, 1);
+}
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/stdlib.h
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/stdlib.h
@@ -0,0 +1,18 @@
+//===--- stdlib.h - Stub header for tests -----------------------*- 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef _STDLIB_H_
+#define _STDLIB_H_
+
+void abort(void);
+void _Exit(int __status);
+void quick_exit(int __status);
+
+void other_call(int);
+
+#endif // _STDLIB_H_
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/signal.h
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/signal.h
@@ -0,0 +1,22 @@
+//===--- signal.h - Stub header for tests -----------------------*- 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef _SIGNAL_H_
+#define _SIGNAL_H_
+
+void _sig_ign(int);
+void _sig_dfl(int);
+
+#define SIGINT 1
+#define SIG_IGN _sig_ign
+#define SIG_DFL _sig_dfl
+
+typedef void (*sighandler_t)(int);
+sighandler_t signal(int signum, sighandler_t handler);
+
+#endif // _SIGNAL_H_
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -114,6 +114,7 @@
`cert-msc51-cpp <cert-msc51-cpp.html>`_,
`cert-oop57-cpp <cert-oop57-cpp.html>`_,
`cert-oop58-cpp <cert-oop58-cpp.html>`_,
+ `cert-sig30-c <cert-sig30-c.html>`_,
`clang-analyzer-core.DynamicTypePropagation <clang-analyzer-core.DynamicTypePropagation.html>`_,
`clang-analyzer-core.uninitialized.CapturedBlockVariable <clang-analyzer-core.uninitialized.CapturedBlockVariable.html>`_,
`clang-analyzer-cplusplus.InnerPointer <clang-analyzer-cplusplus.InnerPointer.html>`_,
Index: clang-tools-extra/docs/clang-tidy/checks/cert-sig30-c.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/cert-sig30-c.rst
@@ -0,0 +1,15 @@
+.. title:: clang-tidy - cert-sig30-c
+
+cert-sig30-c
+============
+
+Finds functions registered as signal handlers that call non asynchronous-safe functions.
+User functions called from the handlers are checked too, as far as possible.
+
+The minimal list of asynchronous-safe system functions is:
+``abort()``, ``_Exit()``, ``quick_exit()`` and ``signal()`` (for ``signal`` there are additional conditions that are not checked).
+Every other system call is considered as non asynchronous-safe by the checker.
+
+This check corresponds to the CERT C Coding Standard rule
+`SIG30-C. Call only asynchronous-safe functions within signal handlers
+<https://www.securecoding.cert.org/confluence/display/c/SIG30-C.+Call+only+asynchronous-safe+functions+within+signal+handlers>`_.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -88,6 +88,15 @@
Added an option `GetConfigPerFile` to support including files which use
different naming styles.
+New checks
+^^^^^^^^^^
+
+- New :doc:`cert-sig30-c
+ <clang-tidy/checks/cert-sig30-c>` check.
+
+ Finds functions registered as signal handlers that call non asynchronous-safe
+ functions.
+
Improvements to include-fixer
-----------------------------
Index: clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.h
@@ -0,0 +1,34 @@
+//===--- SignalHandlerCheck.h - clang-tidy ----------------------*- 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_SIGNALHANDLERCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_SIGNALHANDLERCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace cert {
+
+/// Checker for SEI CERT rule SIG30-C
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/cert-signal-handler-check.html
+class SignalHandlerCheck : public ClangTidyCheck {
+public:
+ SignalHandlerCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace cert
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_SIGNALHANDLERCHECK_H
Index: clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp
@@ -0,0 +1,142 @@
+//===--- ExitHandlerCheck.cpp - clang-tidy --------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#include "SignalHandlerCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallVector.h"
+#include <deque>
+#include <iterator>
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace cert {
+
+static constexpr StringRef SignalFun = "signal";
+static constexpr StringRef AbortFun = "abort";
+static constexpr StringRef ExitFun = "_Exit";
+static constexpr StringRef QuickExitFun = "quick_exit";
+
+static bool isSystemCall(const FunctionDecl *FD) {
+ // This check does not work with function calls in std namespace.
+ if (!FD->isGlobal() || FD->isInStdNamespace())
+ return false;
+ // It is assumed that the function has no other re-declaration that is not
+ // in a system header. Otherwise this may produce wrong result.
+ return FD->getASTContext().getSourceManager().isInSystemHeader(
+ FD->getLocation());
+}
+
+static bool isAllowedSystemCall(const FunctionDecl *FD) {
+ if (!FD->getIdentifier())
+ return true;
+ const StringRef N = FD->getName();
+ if (N == AbortFun || N == ExitFun || N == QuickExitFun || N == SignalFun)
+ return true;
+ return false;
+}
+
+namespace {
+class CalledFunctionsCollector
+ : public RecursiveASTVisitor<CalledFunctionsCollector> {
+public:
+ using CallbackType =
+ std::function<void(const FunctionDecl *, const CallExpr *)>;
+
+ CalledFunctionsCollector(CallbackType FindCallback)
+ : FindCallback{FindCallback} {}
+
+ bool VisitCallExpr(const CallExpr *CE) {
+ if (const auto *F = dyn_cast<FunctionDecl>(CE->getCalleeDecl()))
+ FindCallback(F, CE);
+ return true;
+ }
+
+private:
+ CallbackType FindCallback;
+};
+} // namespace
+
+void SignalHandlerCheck::registerMatchers(MatchFinder *Finder) {
+ const auto HandlerProtoType = functionProtoType(parameterCountIs(1));
+ const auto IsSignalFunction =
+ callee(functionDecl(hasName(SignalFun), parameterCountIs(2)));
+ const auto HandlerAsSecondArg = hasArgument(
+ 1, declRefExpr(hasDeclaration(functionDecl().bind("handler_decl")),
+ unless(isExpandedFromMacro("SIG_IGN")),
+ unless(isExpandedFromMacro("SIG_DFL")))
+ .bind("handler_expr"));
+ Finder->addMatcher(
+ callExpr(IsSignalFunction, HandlerAsSecondArg).bind("register_call"),
+ this);
+}
+
+void SignalHandlerCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *SignalCall = Result.Nodes.getNodeAs<CallExpr>("register_call");
+ const auto *HandlerDecl =
+ Result.Nodes.getNodeAs<FunctionDecl>("handler_decl");
+ const auto *HandlerExpr = Result.Nodes.getNodeAs<DeclRefExpr>("handler_expr");
+
+ // Visit each function encountered in the callgraph only once.
+ llvm::DenseSet<const FunctionDecl *> SeenFunctions;
+
+ // The worklist of the callgraph visitation algorithm.
+ std::deque<std::pair<const FunctionDecl *, const Expr *>> CalledFunctions{
+ {HandlerDecl, HandlerExpr}};
+
+ // Visit the definition of every function referenced by the handler function.
+ // Check for allowed function calls.
+ while (!CalledFunctions.empty()) {
+ // Use the canonical declaration.
+ const FunctionDecl *FunctionToCheck =
+ CalledFunctions.front().first->getCanonicalDecl();
+ const Expr *FunctionCall = CalledFunctions.front().second;
+ CalledFunctions.pop_front();
+
+ // Do not visit function if already encountered.
+ if (!SeenFunctions.insert(FunctionToCheck).second)
+ continue;
+
+ // Check if the call is allowed.
+ // Only system calls are to be checked.
+ if (isSystemCall(FunctionToCheck)) {
+ if (isAllowedSystemCall(FunctionToCheck))
+ continue;
+
+ diag(FunctionCall->getBeginLoc(),
+ "Signal handler potentially calls non asynchronous-safe function. "
+ "This may result in undefined behavior.");
+ diag(SignalCall->getSourceRange().getBegin(),
+ "Signal handler registered here.", DiagnosticIDs::Note);
+ diag(HandlerDecl->getBeginLoc(), "Handler function declared here.",
+ DiagnosticIDs::Note);
+ break;
+ }
+
+ // Get the body of the encountered non-system call function.
+ const FunctionDecl *FunctionBody;
+ if (!FunctionToCheck->hasBody(FunctionBody))
+ continue;
+
+ // Collect all called functions.
+ CalledFunctionsCollector Collector{
+ [&CalledFunctions](const FunctionDecl *FD, const CallExpr *CE) {
+ CalledFunctions.push_back(std::make_pair(FD, CE));
+ }};
+ Collector.TraverseStmt(FunctionBody->getBody());
+ }
+}
+
+} // namespace cert
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/clang-tidy/cert/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/cert/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/cert/CMakeLists.txt
@@ -15,6 +15,7 @@
PostfixOperatorCheck.cpp
ProperlySeededRandomGeneratorCheck.cpp
SetLongJmpCheck.cpp
+ SignalHandlerCheck.cpp
StaticObjectExceptionCheck.cpp
StrToNumCheck.cpp
ThrownExceptionTypeCheck.cpp
Index: clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
+++ clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
@@ -31,6 +31,7 @@
#include "PostfixOperatorCheck.h"
#include "ProperlySeededRandomGeneratorCheck.h"
#include "SetLongJmpCheck.h"
+#include "SignalHandlerCheck.h"
#include "StaticObjectExceptionCheck.h"
#include "StrToNumCheck.h"
#include "ThrownExceptionTypeCheck.h"
@@ -109,6 +110,8 @@
// POS
CheckFactories.registerCheck<bugprone::BadSignalToKillThreadCheck>(
"cert-pos44-c");
+ // SIG
+ CheckFactories.registerCheck<SignalHandlerCheck>("cert-sig30-c");
// STR
CheckFactories.registerCheck<bugprone::SignedCharMisuseCheck>(
"cert-str34-c");
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
