diff --git a/clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.h b/clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.h --- a/clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.h @@ -37,13 +37,24 @@ /// Should test the properties of the function, and check in the code body. /// Should not check function calls in the code (this part is done by the call /// graph scan). - /// @param FD The function to check. It may or may not have a definition. - /// @param CallOrRef Location of the call to this function (in another + /// Bug reports are generated for the whole code body (no stop at the first + /// found issue). For issues that are not in the code body, only one + /// bug report is generated. + /// \param FD The function to check. It may or may not have a definition. + /// \param CallOrRef Location of the call to this function (in another /// function) or the reference to the function (if it is used as a registered /// signal handler). This is the location where diagnostics are to be placed. - /// @return Returns true if a diagnostic was emitted for this function. - bool checkFunction(const FunctionDecl *FD, const Expr *CallOrRef); - /// Returns true if a standard library function is considered as + /// \param ChainReporter A function that adds bug report notes to display the + /// chain of called functions from signal handler registration to the current + /// function. This is called at every generated bug report. + /// The bool parameter is used like \c SkipPathEnd in \c reportHandlerChain . + /// \return Returns true if a diagnostic was emitted for this function. + bool checkFunction(const FunctionDecl *FD, const Expr *CallOrRef, + std::function ChainReporter); + /// Similar as \c checkFunction but only check for C++14 rules. + bool checkFunctionCPP14(const FunctionDecl *FD, const Expr *CallOrRef, + std::function ChainReporter); + /// Returns true if a standard library function is considered /// asynchronous-safe. bool isStandardFunctionAsyncSafe(const FunctionDecl *FD) const; /// Add diagnostic notes to show the call chain of functions from a signal @@ -54,8 +65,10 @@ /// function call. /// @param HandlerRef Reference to the signal handler function where it is /// registered as signal handler. + /// @param SkipPathEnd If true the last item of the call chain (farthest away + /// from the \c signal call) is omitted from note generation. void reportHandlerChain(const llvm::df_iterator &Itr, - const DeclRefExpr *HandlerRef); + const DeclRefExpr *HandlerRef, bool SkipPathEnd); clang::CallGraph CG; diff --git a/clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp --- a/clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp @@ -243,6 +243,16 @@ namespace { +/*bool isInGlobalNamespace(const FunctionDecl *FD) { + // FIXME: Check if this works for C++20 modules + // if the checker will be enabled for C++20. + const DeclContext *DC = FD->getDeclContext(); + while (DC && DC->isTransparentContext()) + DC = DC->getParent(); + assert(DC && "Function should have a declaration context."); + return DC->isTranslationUnit(); +}*/ + /// Returns if a function is declared inside a system header. /// These functions are considered to be "standard" (system-provided) library /// functions. @@ -291,6 +301,18 @@ return FoundCallee->CallExpr; } +SourceRange getSourceRangeOfStmt(const Stmt *S, ASTContext &Ctx) { + ParentMapContext &PM = Ctx.getParentMapContext(); + DynTypedNode P = DynTypedNode::create(*S); + while (P.getSourceRange().isInvalid()) { + DynTypedNodeList PL = PM.getParents(P); + if (PL.size() != 1) + return {}; + P = PL[0]; + } + return P.getSourceRange(); +} + } // namespace AST_MATCHER(FunctionDecl, isStandardFunction) { @@ -313,11 +335,7 @@ bool SignalHandlerCheck::isLanguageVersionSupported( const LangOptions &LangOpts) const { - // FIXME: Make the checker useful on C++ code. - if (LangOpts.CPlusPlus) - return false; - - return true; + return LangOpts.CPlusPlus17 == 0; } void SignalHandlerCheck::registerMatchers(MatchFinder *Finder) { @@ -328,13 +346,23 @@ unless(isExpandedFromMacro("SIG_IGN")), unless(isExpandedFromMacro("SIG_DFL"))) .bind("handler_expr"); - Finder->addMatcher( - callExpr(callee(SignalFunction), hasArgument(1, HandlerExpr)) - .bind("register_call"), - this); + auto HandlerLambda = cxxMemberCallExpr( + on(expr(ignoringParenImpCasts(lambdaExpr().bind("handler_lambda"))))); + Finder->addMatcher(callExpr(callee(SignalFunction), + hasArgument(1, anyOf(HandlerExpr, HandlerLambda))) + .bind("register_call"), + this); } void SignalHandlerCheck::check(const MatchFinder::MatchResult &Result) { + if (const auto *HandlerLambda = + Result.Nodes.getNodeAs("handler_lambda")) { + diag(HandlerLambda->getBeginLoc(), + "lambda function is not allowed as signal handler (until C++17)") + << HandlerLambda->getSourceRange(); + return; + } + const auto *SignalCall = Result.Nodes.getNodeAs("register_call"); const auto *HandlerDecl = Result.Nodes.getNodeAs("handler_decl"); @@ -353,32 +381,48 @@ } if (!HandlerDecl->hasBody()) { - (void)checkFunction(HandlerDecl, HandlerExpr); - // Here checkFunction will put the messages to HandlerExpr. - // No need to show a call chain. - // Without code body there is nothing more to check. + // Check the handler function. + // The warning is placed to the signal handler registration. + // No need to display a call chain and no need for more checks. + (void)checkFunction(HandlerDecl, HandlerExpr, {}); return; } + // FIXME: Update CallGraph::getNode to use canonical decl? CallGraphNode *HandlerNode = CG.getNode(HandlerDecl->getCanonicalDecl()); assert(HandlerNode && "Handler with body should be present in the call graph."); // Start from signal handler and visit every function call. - for (auto Itr = llvm::df_begin(HandlerNode), ItrE = llvm::df_end(HandlerNode); - Itr != ItrE; ++Itr) { - if (const auto *CallF = dyn_cast((*Itr)->getDecl())) { - unsigned int PathL = Itr.getPathLength(); - const Expr *CallOrRef = (PathL == 1) - ? HandlerExpr - : findCallExpr(Itr.getPath(PathL - 2), *Itr); - if (checkFunction(CallF, CallOrRef)) - reportHandlerChain(Itr, HandlerExpr); + auto Itr = llvm::df_begin(HandlerNode), ItrE = llvm::df_end(HandlerNode); + while (Itr != ItrE) { + const auto *CallF = dyn_cast((*Itr)->getDecl()); + unsigned int PathL = Itr.getPathLength(); + if (CallF) { + // The problem was found in a function that was called from a signal + // handler, or in the direct signal handler function. + // Generate notes for the whole call chain (including the signal handler + // registration). + const Expr *CallOrRef = (PathL > 1) + ? findCallExpr(Itr.getPath(PathL - 2), *Itr) + : HandlerExpr; + auto ChainReporter = [this, &Itr, HandlerExpr](bool SkipPathEnd) { + reportHandlerChain(Itr, HandlerExpr, SkipPathEnd); + }; + // If problems were found in a function (`CallF`), skip the analysis of + // functions that are called from it. + if (checkFunction(CallF, CallOrRef, ChainReporter)) + Itr.skipChildren(); + else + ++Itr; + } else { + ++Itr; } } } -bool SignalHandlerCheck::checkFunction(const FunctionDecl *FD, - const Expr *CallOrRef) { +bool SignalHandlerCheck::checkFunction( + const FunctionDecl *FD, const Expr *CallOrRef, + std::function ChainReporter) { bool FunctionIsCalled = isa(CallOrRef); if (isStandardFunction(FD)) { @@ -387,7 +431,9 @@ "asynchronous-safe; " "%select{using it as|calling it from}1 " "a signal handler may be dangerous") - << FD << FunctionIsCalled; + << FD << FunctionIsCalled << CallOrRef->getSourceRange(); + if (ChainReporter) + ChainReporter(/*SkipPathEnd=*/true); return true; } return false; @@ -398,23 +444,75 @@ "asynchronous-safe; " "%select{using it as|calling it from}1 " "a signal handler may be dangerous") - << FD << FunctionIsCalled; + << FD << FunctionIsCalled << CallOrRef->getSourceRange(); + if (ChainReporter) + ChainReporter(/*SkipPathEnd=*/true); return true; } + if (getLangOpts().CPlusPlus) + return checkFunctionCPP14(FD, CallOrRef, ChainReporter); + return false; } +bool SignalHandlerCheck::checkFunctionCPP14( + const FunctionDecl *FD, const Expr *CallOrRef, + std::function ChainReporter) { + if (!FD->isExternC()) { + diag(CallOrRef->getBeginLoc(), + "functions with other than C linkage are not allowed as signal " + "handler (until C++17)"); + if (ChainReporter) + ChainReporter(/*SkipPathEnd=*/true); + return true; + } + + const FunctionDecl *FBody; + const Stmt *BodyS = FD->getBody(FBody); + if (!BodyS) + return false; + + bool StmtProblemsFound = false; + ASTContext &Ctx = FBody->getASTContext(); + auto Matches = + match(decl(forEachDescendant(stmt().bind("stmt"))), *FBody, Ctx); + for (const auto &Match : Matches) { + const auto *FoundS = Match.getNodeAs("stmt"); + StringRef Name = FoundS->getStmtClassName(); + if (Name.startswith("CXX")) { + SourceRange R = getSourceRangeOfStmt(FoundS, Ctx); + if (R.isInvalid()) + continue; + diag(R.getBegin(), + "C++-only construct is not allowed in signal handler (until C++17)") + << R; + diag(R.getBegin(), "internally, the statement is parsed as a '%0'", + DiagnosticIDs::Remark) + << Name; + if (ChainReporter) + ChainReporter(/*SkipPathEnd=*/false); + StmtProblemsFound = true; + } + } + + return StmtProblemsFound; +} + bool SignalHandlerCheck::isStandardFunctionAsyncSafe( const FunctionDecl *FD) const { assert(isStandardFunction(FD)); const IdentifierInfo *II = FD->getIdentifier(); // Unnamed functions are not explicitly allowed. + // C++ std operators may be unsafe and not within the + // "common subset of C and C++". if (!II) return false; - // FIXME: Improve for C++ (check for namespace). + if (!FD->isInStdNamespace() && !FD->isGlobal()) + return false; + if (ConformingFunctions.count(II->getName())) return true; @@ -423,7 +521,7 @@ void SignalHandlerCheck::reportHandlerChain( const llvm::df_iterator &Itr, - const DeclRefExpr *HandlerRef) { + const DeclRefExpr *HandlerRef, bool SkipPathEnd) { int CallLevel = Itr.getPathLength() - 2; assert(CallLevel >= -1 && "Empty iterator?"); @@ -432,16 +530,21 @@ Callee = Caller; Caller = Itr.getPath(CallLevel); const Expr *CE = findCallExpr(Caller, Callee); - diag(CE->getBeginLoc(), "function %0 called here from %1", - DiagnosticIDs::Note) - << cast(Callee->getDecl()) - << cast(Caller->getDecl()); + if (SkipPathEnd) + SkipPathEnd = false; + else + diag(CE->getBeginLoc(), "function %0 called here from %1", + DiagnosticIDs::Note) + << cast(Callee->getDecl()) + << cast(Caller->getDecl()); --CallLevel; } - diag(HandlerRef->getBeginLoc(), - "function %0 registered here as signal handler", DiagnosticIDs::Note) - << cast(Caller->getDecl()) << HandlerRef->getSourceRange(); + if (!SkipPathEnd) + diag(HandlerRef->getBeginLoc(), + "function %0 registered here as signal handler", DiagnosticIDs::Note) + << cast(Caller->getDecl()) + << HandlerRef->getSourceRange(); } } // namespace bugprone diff --git a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp --- a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp @@ -265,6 +265,8 @@ CheckFactories.registerCheck("cert-msc50-cpp"); CheckFactories.registerCheck( "cert-msc51-cpp"); + CheckFactories.registerCheck( + "cert-msc54-cpp"); // OOP CheckFactories.registerCheck( "cert-oop11-cpp"); diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -106,9 +106,19 @@ New check aliases ^^^^^^^^^^^^^^^^^ +- New alias :doc:`cert-msc54-cpp + ` to + :doc:`bugprone-signal-handler + ` was added. + Changes in existing checks ^^^^^^^^^^^^^^^^^^^^^^^^^^ +- :doc:`bugprone-signal-handler + ` check now partially + supports signal handler rules for C++14. Bug report generation is + improved. + Removed checks ^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone-signal-handler.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone-signal-handler.rst --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone-signal-handler.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone-signal-handler.rst @@ -3,20 +3,37 @@ bugprone-signal-handler ======================= -Finds functions registered as signal handlers that call non asynchronous-safe -functions. Any function that cannot be determined to be an asynchronous-safe +Finds functions registered as signal handlers that contain specific +constructs that can cause undefined behavior. The rules for what is +allowed are different for various C++ language versions. + +For C it is checked if non-asynchronous-safe functions are called. + +For C++ until (and including) C++14 such code constructs are found +that are C++-specific and therefore not recommended in signal handlers. +Additionally the calls to non-asynchronous-safe functions are found +too and a signal handler or any function called from it is allowed +to have only extern C linkage. Restrictions related to atomic +lock-free operations are not checked. +In C++17 there are more clarified rules but these are not +implemented yet in this check. The check does not run on C++17 code. + +Any function that cannot be determined to be an asynchronous-safe function call is assumed to be non-asynchronous-safe by the checker, including user functions for which only the declaration is visible. User function calls with visible definition are checked recursively. -The check handles only C code. Only the function names are considered and the -fact that the function is a system-call, but no other restrictions on the -arguments passed to the functions (the ``signal`` call is allowed without +Only the function names are considered and the fact that the function +is a system-call, but no other restrictions on the arguments passed +to the functions (the ``signal`` call is allowed without restrictions). -This check corresponds to the CERT C Coding Standard rule +This check implements the CERT C Coding Standard rule `SIG30-C. Call only asynchronous-safe functions within signal handlers `_ -and has an alias name ``cert-sig30-c``. +and the rule +`MSC54-CPP. A signal handler must be a plain old function +`_. +It has alias names ``cert-sig30-c`` and ``cert-msc54-cpp``. .. option:: AsyncSafeFunctionSet diff --git a/clang-tools-extra/docs/clang-tidy/checks/cert-msc54-cpp.rst b/clang-tools-extra/docs/clang-tidy/checks/cert-msc54-cpp.rst new file mode 100644 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/cert-msc54-cpp.rst @@ -0,0 +1,10 @@ +.. title:: clang-tidy - cert-msc54-cpp +.. meta:: + :http-equiv=refresh: 5;URL=bugprone-signal-handler.html + +cert-msc54-cpp +============== + +The cert-msc54-cpp check is an alias, please see +`bugprone-signal-handler `_ +for more information. diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/signal.h b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/signal.h --- a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/signal.h +++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/signal.h @@ -13,10 +13,12 @@ void _sig_ign(int); void _sig_dfl(int); +void _sig_err(int); #define SIGINT 1 #define SIG_IGN _sig_ign #define SIG_DFL _sig_dfl +#define SIG_ERR _sig_err sighandler_t signal(int, sighandler_t); diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/signal.h b/clang-tools-extra/test/clang-tidy/checkers/Inputs/bugprone-signal-handler/stdcpp.h copy from clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/signal.h copy to clang-tools-extra/test/clang-tidy/checkers/Inputs/bugprone-signal-handler/stdcpp.h --- a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/signal.h +++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/bugprone-signal-handler/stdcpp.h @@ -1,4 +1,4 @@ -//===--- signal.h - Stub header for tests -----------------------*- C++ -*-===// +//===--- Header for test bugprone-signal-handler.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. @@ -6,18 +6,33 @@ // //===----------------------------------------------------------------------===// -#ifndef _SIGNAL_H_ -#define _SIGNAL_H_ +#define SIGINT 1 +#define SIG_IGN std::_sig_ign +#define SIG_DFL std::_sig_dfl -typedef void (*sighandler_t)(int); +namespace std { 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, sighandler_t); + +void abort(); +void _Exit(int); +void quick_exit(int); +void other_call(); + +struct SysStruct { + void operator<<(int); +}; + +} // namespace std + +namespace system_other { + +typedef void (*sighandler_t)(int); sighandler_t signal(int, sighandler_t); -#endif // _SIGNAL_H_ +} // namespace system_other diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-handler.c b/clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-handler.c --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-handler.c +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-handler.c @@ -18,7 +18,6 @@ void handler_printf(int) { printf("1234"); // CHECK-NOTES: :[[@LINE-1]]:3: warning: standard function 'printf' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler] - // CHECK-NOTES: :[[@LINE-2]]:3: note: function 'printf' called here from 'handler_printf' // CHECK-NOTES: :[[@LINE+4]]:18: note: function 'handler_printf' registered here as signal handler } @@ -29,7 +28,6 @@ void handler_extern(int) { f_extern(); // CHECK-NOTES: :[[@LINE-1]]:3: warning: cannot verify that external function 'f_extern' is asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler] - // CHECK-NOTES: :[[@LINE-2]]:3: note: function 'f_extern' called here from 'handler_extern' // CHECK-NOTES: :[[@LINE+4]]:18: note: function 'handler_extern' registered here as signal handler } @@ -52,7 +50,6 @@ void f_bad() { printf("1234"); // CHECK-NOTES: :[[@LINE-1]]:3: warning: standard function 'printf' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler] - // CHECK-NOTES: :[[@LINE-2]]:3: note: function 'printf' called here from 'f_bad' // CHECK-NOTES: :[[@LINE+5]]:3: note: function 'f_bad' called here from 'handler_bad' // CHECK-NOTES: :[[@LINE+8]]:18: note: function 'handler_bad' registered here as signal handler } @@ -68,7 +65,6 @@ void f_bad1() { printf("1234"); // CHECK-NOTES: :[[@LINE-1]]:3: warning: standard function 'printf' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler] - // CHECK-NOTES: :[[@LINE-2]]:3: note: function 'printf' called here from 'f_bad1' // CHECK-NOTES: :[[@LINE+6]]:3: note: function 'f_bad1' called here from 'f_bad2' // CHECK-NOTES: :[[@LINE+9]]:3: note: function 'f_bad2' called here from 'handler_bad1' // CHECK-NOTES: :[[@LINE+13]]:18: note: function 'handler_bad1' registered here as signal handler @@ -100,7 +96,6 @@ if (0) printf("1234"); // CHECK-NOTES: :[[@LINE-1]]:5: warning: standard function 'printf' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler] - // CHECK-NOTES: :[[@LINE-2]]:5: note: function 'printf' called here from 'handler_false_condition' // CHECK-NOTES: :[[@LINE+4]]:18: note: function 'handler_false_condition' registered here as signal handler } @@ -111,11 +106,9 @@ void handler_multiple_calls(int) { f_extern(); // CHECK-NOTES: :[[@LINE-1]]:3: warning: cannot verify that external function 'f_extern' is asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler] - // CHECK-NOTES: :[[@LINE-2]]:3: note: function 'f_extern' called here from 'handler_multiple_calls' - // CHECK-NOTES: :[[@LINE+10]]:18: note: function 'handler_multiple_calls' registered here as signal handler + // CHECK-NOTES: :[[@LINE+9]]:18: note: function 'handler_multiple_calls' registered here as signal handler printf("1234"); // CHECK-NOTES: :[[@LINE-1]]:3: warning: standard function 'printf' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler] - // CHECK-NOTES: :[[@LINE-2]]:3: note: function 'printf' called here from 'handler_multiple_calls' // CHECK-NOTES: :[[@LINE+6]]:18: note: function 'handler_multiple_calls' registered here as signal handler f_extern(); // first 'f_extern' call found only @@ -136,13 +129,11 @@ void f_recursive() { f_extern(); // CHECK-NOTES: :[[@LINE-1]]:3: warning: cannot verify that external function 'f_extern' is asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler] - // CHECK-NOTES: :[[@LINE-2]]:3: note: function 'f_extern' called here from 'f_recursive' - // CHECK-NOTES: :[[@LINE-9]]:3: note: function 'f_recursive' called here from 'handler_recursive' - // CHECK-NOTES: :[[@LINE+10]]:18: note: function 'handler_recursive' registered here as signal handler + // CHECK-NOTES: :[[@LINE-8]]:3: note: function 'f_recursive' called here from 'handler_recursive' + // CHECK-NOTES: :[[@LINE+9]]:18: note: function 'handler_recursive' registered here as signal handler printf(""); // CHECK-NOTES: :[[@LINE-1]]:3: warning: standard function 'printf' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler] - // CHECK-NOTES: :[[@LINE-2]]:3: note: function 'printf' called here from 'f_recursive' - // CHECK-NOTES: :[[@LINE-14]]:3: note: function 'f_recursive' called here from 'handler_recursive' + // CHECK-NOTES: :[[@LINE-12]]:3: note: function 'f_recursive' called here from 'handler_recursive' // CHECK-NOTES: :[[@LINE+5]]:18: note: function 'handler_recursive' registered here as signal handler handler_recursive(2); } @@ -154,7 +145,6 @@ void f_multiple_paths() { printf(""); // CHECK-NOTES: :[[@LINE-1]]:3: warning: standard function 'printf' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler] - // CHECK-NOTES: :[[@LINE-2]]:3: note: function 'printf' called here from 'f_multiple_paths' // CHECK-NOTES: :[[@LINE+5]]:3: note: function 'f_multiple_paths' called here from 'handler_multiple_paths' // CHECK-NOTES: :[[@LINE+9]]:18: note: function 'handler_multiple_paths' registered here as signal handler } diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-handler.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-handler.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-handler.cpp @@ -0,0 +1,228 @@ +// RUN: %check_clang_tidy -std=c++14 %s bugprone-signal-handler %t -- -- -isystem %S/Inputs/Headers -isystem %S/Inputs/bugprone-signal-handler + +#include "stdcpp.h" +#include "stdio.h" + +// Functions called "signal" that are different from the system version. +typedef void (*callback_t)(int); +void signal(int, callback_t, int); +namespace ns { +void signal(int, callback_t); +} + +extern "C" void handler_unsafe(int) { + printf("xxx"); +} + +extern "C" void handler_unsafe_1(int) { + printf("xxx"); +} + +namespace test_invalid_handler { + +void handler_non_extern_c(int) { + printf("xxx"); +} + +struct A { + static void handler_member(int) { + printf("xxx"); + } +}; + +void test() { + std::signal(SIGINT, handler_unsafe_1); + // CHECK-MESSAGES: :[[@LINE-17]]:3: warning: standard function 'printf' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler] + // CHECK-MESSAGES: :[[@LINE-2]]:23: note: function 'handler_unsafe_1' registered here as signal handler + + std::signal(SIGINT, handler_non_extern_c); + // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: functions with other than C linkage are not allowed as signal handler (until C++17) [bugprone-signal-handler] + std::signal(SIGINT, A::handler_member); + // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: functions with other than C linkage are not allowed as signal handler (until C++17) [bugprone-signal-handler] + std::signal(SIGINT, [](int) { printf("xxx"); }); + // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: lambda function is not allowed as signal handler (until C++17) [bugprone-signal-handler] + + // This case is (deliberately) not found by the checker. + std::signal(SIGINT, [](int) -> callback_t { return &handler_unsafe; }(1)); +} + +} // namespace test_invalid_handler + +namespace test_non_standard_signal_call { + +struct Signal { + static void signal(int, callback_t); +}; + +void test() { + // No diagnostics here. All these signal calls differ from the standard system one. + signal(SIGINT, handler_unsafe, 1); + ns::signal(SIGINT, handler_unsafe); + Signal::signal(SIGINT, handler_unsafe); + system_other::signal(SIGINT, handler_unsafe); +} + +} // namespace test_non_standard_signal_call + +namespace test_cpp_construct_in_handler { + +struct Struct { + virtual ~Struct() {} + void f1(); + int *begin(); + int *end(); + static void f2(); +}; +struct Derived : public Struct { +}; + +struct X { + X(int, float); +}; + +Struct *S_Global; +const Struct *S_GlobalConst; + +void f_non_extern_c() { +} + +void f_default_arg(int P1 = 0) { +} + +extern "C" void handler_cpp(int) { + using namespace ::test_cpp_construct_in_handler; + + // These calls are not found as problems. + // (Called functions are not analyzed if the current function has already + // other problems.) + f_non_extern_c(); + Struct::f2(); + // 'auto' is not disallowed + auto Auto = 28u; + + Struct S; + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler] + // CHECK-MESSAGES: :[[@LINE-2]]:10: remark: internally, the statement is parsed as a 'CXXConstructExpr' + // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler + S_Global->f1(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler] + // CHECK-MESSAGES: :[[@LINE-2]]:3: remark: internally, the statement is parsed as a 'CXXMemberCallExpr' + // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler + const Struct &SRef = Struct(); + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler] + // CHECK-MESSAGES: :[[@LINE-2]]:24: remark: internally, the statement is parsed as a 'CXXBindTemporaryExpr' + // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler + X(3, 4.4); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler] + // CHECK-MESSAGES: :[[@LINE-2]]:3: remark: internally, the statement is parsed as a 'CXXTemporaryObjectExpr' + // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler + + auto L = [](int i) { printf("%d", i); }; + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler] + // CHECK-MESSAGES: :[[@LINE-2]]:12: remark: internally, the statement is parsed as a 'CXXConstructExpr' + // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler + L(2); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler] + // CHECK-MESSAGES: :[[@LINE-2]]:3: remark: internally, the statement is parsed as a 'CXXOperatorCallExpr' + // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler + + try { + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler] + // CHECK-MESSAGES: :[[@LINE-2]]:3: remark: internally, the statement is parsed as a 'CXXTryStmt' + // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler + int A; + } catch (int) { + }; + // CHECK-MESSAGES: :[[@LINE-2]]:5: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler] + // CHECK-MESSAGES: :[[@LINE-3]]:5: remark: internally, the statement is parsed as a 'CXXCatchStmt' + // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler + + throw(12); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler] + // CHECK-MESSAGES: :[[@LINE-2]]:3: remark: internally, the statement is parsed as a 'CXXThrowExpr' + // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler + + for (int I : S) { + } + // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler] + // CHECK-MESSAGES: :[[@LINE-3]]:3: remark: internally, the statement is parsed as a 'CXXForRangeStmt' + // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler + // CHECK-MESSAGES: :[[@LINE-5]]:14: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler] + // CHECK-MESSAGES: :[[@LINE-6]]:14: remark: internally, the statement is parsed as a 'CXXMemberCallExpr' + // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler + + int Int = *(reinterpret_cast(&S)); + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler] + // CHECK-MESSAGES: :[[@LINE-2]]:15: remark: internally, the statement is parsed as a 'CXXReinterpretCastExpr' + // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler + Int = static_cast(12.34); + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler] + // CHECK-MESSAGES: :[[@LINE-2]]:9: remark: internally, the statement is parsed as a 'CXXStaticCastExpr' + // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler + Derived *Der = dynamic_cast(S_Global); + // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler] + // CHECK-MESSAGES: :[[@LINE-2]]:18: remark: internally, the statement is parsed as a 'CXXDynamicCastExpr' + // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler + Struct *SPtr = const_cast(S_GlobalConst); + // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler] + // CHECK-MESSAGES: :[[@LINE-2]]:18: remark: internally, the statement is parsed as a 'CXXConstCastExpr' + // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler + Int = int(12.34); + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler] + // CHECK-MESSAGES: :[[@LINE-2]]:9: remark: internally, the statement is parsed as a 'CXXFunctionalCastExpr' + // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler + + int *IPtr = new int[10]; + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler] + // CHECK-MESSAGES: :[[@LINE-2]]:15: remark: internally, the statement is parsed as a 'CXXNewExpr' + // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler + delete[] IPtr; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler] + // CHECK-MESSAGES: :[[@LINE-2]]:3: remark: internally, the statement is parsed as a 'CXXDeleteExpr' + // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler + IPtr = nullptr; + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler] + // CHECK-MESSAGES: :[[@LINE-2]]:10: remark: internally, the statement is parsed as a 'CXXNullPtrLiteralExpr' + // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler + bool Bool = true; + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler] + // CHECK-MESSAGES: :[[@LINE-2]]:15: remark: internally, the statement is parsed as a 'CXXBoolLiteralExpr' + // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler + f_default_arg(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler] + // CHECK-MESSAGES: :[[@LINE-2]]:3: remark: internally, the statement is parsed as a 'CXXDefaultArgExpr' + // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler +} + +void test() { + std::signal(SIGINT, handler_cpp); +} + +} // namespace test_cpp_construct_in_handler + +namespace test_cpp_indirect { + +void non_extern_c() { + int *P = nullptr; +} + +extern "C" void call_cpp_indirect() { + int *P = nullptr; + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler] + // CHECK-MESSAGES: :[[@LINE-2]]:12: remark: internally, the statement is parsed as a 'CXXNullPtrLiteralExpr' + // CHECK-MESSAGES: :[[@LINE+8]]:3: note: function 'call_cpp_indirect' called here from 'handler_cpp_indirect' + // CHECK-MESSAGES: :[[@LINE+11]]:23: note: function 'handler_cpp_indirect' registered here as signal handler +} + +extern "C" void handler_cpp_indirect(int) { + non_extern_c(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: functions with other than C linkage are not allowed as signal handler (until C++17) [bugprone-signal-handler] + // CHECK-MESSAGES: :[[@LINE+5]]:23: note: function 'handler_cpp_indirect' registered here as signal handler + call_cpp_indirect(); +} + +void test() { + std::signal(SIGINT, handler_cpp_indirect); +} + +} // namespace test_cpp_indirect