balazske updated this revision to Diff 295538.
balazske added a comment.
Added support for C++ code.
Improved detection of 'signal' function.
Simplified collection of called functions.
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/cert-sig30-c_cpp.h
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.c
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,64 @@
+// RUN: %check_clang_tidy %s cert-sig30-c %t -- -- -isystem %S/Inputs/Headers
+
+#include "cert-sig30-c_cpp.h"
+#include "stdio.h"
+
+void handler_abort(int) {
+ std::abort();
+}
+
+void handler__Exit(int) {
+ std::_Exit(0);
+}
+
+void handler_quick_exit(int) {
+ std::quick_exit(0);
+}
+
+void handler_bad1(int) {
+ std::something(1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'something' is considered as non asynchronous-safe and should not be called from a signal handler [cert-sig30-c]
+}
+
+void handler_bad2(int) {
+ std::SysStruct S;
+ S << 1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'operator<<' is considered as non asynchronous-safe and should not be called from a signal handler [cert-sig30-c]
+}
+
+void handler_signal(int) {
+ // FIXME: It is only OK to call signal with the current signal number.
+ std::signal(0, SIG_DFL);
+ std::signal(0, SIG_IGN);
+}
+
+void handler_nowarn(int) {
+ std::something(2);
+}
+
+// Function called "signal" that is not to be recognized by the checker.
+typedef void (*callback_t)(int);
+void signal(int, callback_t, int);
+
+namespace ns {
+void signal(int, callback_t);
+}
+
+struct S {
+ static void signal(int, callback_t);
+};
+
+void test() {
+ std::signal(SIGINT, handler_abort);
+ std::signal(SIGINT, handler__Exit);
+ std::signal(SIGINT, handler_quick_exit);
+ std::signal(SIGINT, handler_signal);
+ std::signal(SIGINT, handler_bad1);
+ std::signal(SIGINT, handler_bad2);
+
+ // Do not find problems here.
+ signal(SIGINT, handler_nowarn, 1);
+ ns::signal(SIGINT, handler_nowarn);
+ S::signal(SIGINT, handler_nowarn);
+ system_other::signal(SIGINT, handler_nowarn);
+}
Index: clang-tools-extra/test/clang-tidy/checkers/cert-sig30-c.c
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cert-sig30-c.c
@@ -0,0 +1,65 @@
+// 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: 'printf' is considered as non asynchronous-safe and should not be called from a signal handler [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: 'printf' is considered as non asynchronous-safe and should not be called from a signal handler [cert-sig30-c]
+}
+
+void f_extern();
+
+void handler_ok(int) {
+ f_ok();
+ f_extern();
+}
+
+void handler_bad(int) {
+ f_bad();
+}
+
+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: 'other_call' is considered as non asynchronous-safe and should not be called from a signal handler [cert-sig30-c]
+
+ signal(SIGINT, SIG_IGN);
+ signal(SIGINT, SIG_DFL);
+}
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/test/clang-tidy/checkers/Inputs/Headers/cert-sig30-c_cpp.h
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cert-sig30-c_cpp.h
@@ -0,0 +1,38 @@
+//===--- Header for test cert-sig30-c.cpp -----------------------*- 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
+//
+//===----------------------------------------------------------------------===//
+
+#define SIGINT 1
+#define SIG_IGN std::_sig_ign
+#define SIG_DFL std::_sig_dfl
+
+namespace std {
+
+void _sig_ign(int);
+void _sig_dfl(int);
+
+typedef void (*sighandler_t)(int);
+sighandler_t signal(int signum, sighandler_t handler);
+
+void abort();
+void _Exit(int __status);
+void quick_exit(int __status);
+
+void something(int);
+
+struct SysStruct {
+ void operator<<(int);
+};
+
+} // namespace std
+
+namespace system_other {
+
+typedef void (*sighandler_t)(int);
+sighandler_t signal(int signum, sighandler_t handler);
+
+} // namespace system_other
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
@@ -115,6 +115,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,17 @@
+.. 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
@@ -103,6 +103,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,39 @@
+//===--- 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"
+#include "llvm/ADT/StringSet.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);
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+ bool isSystemCallAllowed(const FunctionDecl *FD) const;
+
+ static llvm::StringSet<> StrictConformingFunctions;
+};
+
+} // 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,177 @@
+//===--- SignalHandlerCheck.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";
+
+AST_MATCHER(FunctionDecl, isTopLevelSystemFunction) {
+ if (isa<CXXMethodDecl>(Node))
+ return false;
+ if (Node.getDeclContext()->isNamespace())
+ return false;
+
+ for (const FunctionDecl *D : Node.redecls())
+ if (D->getASTContext().getSourceManager().isInSystemHeader(
+ D->getLocation()))
+ return true;
+
+ return false;
+}
+
+static bool isSystemCall(const FunctionDecl *FD) {
+ // "std" functions are always system calls.
+ if (FD->isInStdNamespace())
+ return true;
+
+ // Find a possible redeclaration in system header.
+ for (const FunctionDecl *D : FD->redecls())
+ if (FD->getASTContext().getSourceManager().isInSystemHeader(
+ D->getLocation()))
+ return true;
+
+ return false;
+}
+
+namespace {
+class FunctionCallCollector
+ : public RecursiveASTVisitor<FunctionCallCollector> {
+public:
+ using CallbackType = std::function<void(const CallExpr *)>;
+
+ FunctionCallCollector(CallbackType FindCallback)
+ : FindCallback{FindCallback} {}
+
+ bool VisitCallExpr(const CallExpr *CE) {
+ FindCallback(CE);
+ return true;
+ }
+
+private:
+ CallbackType FindCallback;
+};
+} // namespace
+
+llvm::StringSet<> SignalHandlerCheck::StrictConformingFunctions{
+ SignalFun, "abort", "_Exit", "quick_exit"};
+
+SignalHandlerCheck::SignalHandlerCheck(StringRef Name,
+ ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+
+void SignalHandlerCheck::registerMatchers(MatchFinder *Finder) {
+ auto SignalFunction =
+ functionDecl(hasName(SignalFun), parameterCountIs(2),
+ anyOf(isInStdNamespace(), isTopLevelSystemFunction()));
+ auto HandlerExpr =
+ declRefExpr(hasDeclaration(functionDecl().bind("handler_decl")),
+ unless(isExpandedFromMacro("SIG_IGN")),
+ unless(isExpandedFromMacro("SIG_DFL")))
+ .bind("handler_expr");
+ Finder->addMatcher(
+ callExpr(callee(SignalFunction), hasArgument(1, HandlerExpr))
+ .bind("register_call"),
+ this);
+}
+
+void SignalHandlerCheck::check(const MatchFinder::MatchResult &Result) {
+ auto *SignalCall = Result.Nodes.getNodeAs<CallExpr>("register_call");
+ auto *HandlerDecl = Result.Nodes.getNodeAs<FunctionDecl>("handler_decl");
+ 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<const CallExpr *> CalledFunctions;
+
+ auto ProcessFunction = [&](const FunctionDecl *F, const Expr *CallOrRef) {
+ // Ensure that canonical declaration is used.
+ F = F->getCanonicalDecl();
+
+ // Do not visit function if already encountered.
+ if (!SeenFunctions.insert(F).second)
+ return true;
+
+ // Check if the call is allowed.
+ // Non-system calls are not considered.
+ if (isSystemCall(F)) {
+ if (isSystemCallAllowed(F))
+ return true;
+
+ diag(CallOrRef->getBeginLoc(),
+ "'%0' is considered as non asynchronous-safe and "
+ "should not be called from a signal handler")
+ << F->getNameAsString();
+ diag(SignalCall->getSourceRange().getBegin(),
+ "signal handler registered here", DiagnosticIDs::Note);
+ diag(HandlerDecl->getBeginLoc(), "handler function declared here",
+ DiagnosticIDs::Note);
+ return false;
+ }
+
+ // Get the body of the encountered non-system call function.
+ const FunctionDecl *FBody;
+ if (!F->hasBody(FBody))
+ return true;
+
+ // Collect all called functions.
+ FunctionCallCollector Collector{[&CalledFunctions](const CallExpr *CE) {
+ if (isa<FunctionDecl>(CE->getCalleeDecl()))
+ CalledFunctions.push_back(CE);
+ }};
+ Collector.TraverseStmt(FBody->getBody());
+
+ return true;
+ };
+
+ if (!ProcessFunction(HandlerDecl, HandlerExpr))
+ return;
+
+ // Visit the definition of every function referenced by the handler function.
+ // Check for allowed function calls.
+ while (!CalledFunctions.empty()) {
+ const CallExpr *FunctionCall = CalledFunctions.front();
+ CalledFunctions.pop_front();
+ // At insertion we have already ensured that only function calls are there.
+ const FunctionDecl *F = cast<FunctionDecl>(FunctionCall->getCalleeDecl());
+
+ if (!ProcessFunction(F, FunctionCall))
+ break;
+ }
+}
+
+bool SignalHandlerCheck::isSystemCallAllowed(const FunctionDecl *FD) const {
+ const IdentifierInfo *II = FD->getIdentifier();
+ // Nonnamed functions are not explicitly allowed.
+ if (!II)
+ return false;
+
+ if (StrictConformingFunctions.count(II->getName()))
+ return true;
+
+ return false;
+}
+
+} // 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
