Index: clang-tidy/cert/CERTTidyModule.cpp =================================================================== --- clang-tidy/cert/CERTTidyModule.cpp +++ clang-tidy/cert/CERTTidyModule.cpp @@ -22,6 +22,7 @@ #include "LimitedRandomnessCheck.h" #include "PostfixOperatorCheck.h" #include "SetLongJmpCheck.h" +#include "SignalHandlerMustBePlainOldFunctionCheck.h" #include "StaticObjectExceptionCheck.h" #include "StrToNumCheck.h" #include "ThrownExceptionTypeCheck.h" @@ -58,8 +59,9 @@ "cert-err61-cpp"); // MSC CheckFactories.registerCheck("cert-msc50-cpp"); - - // C checkers + CheckFactories.registerCheck( + "cert-msc54-cpp"); + // C checks // DCL CheckFactories.registerCheck("cert-dcl03-c"); // ENV Index: clang-tidy/cert/CMakeLists.txt =================================================================== --- clang-tidy/cert/CMakeLists.txt +++ clang-tidy/cert/CMakeLists.txt @@ -8,6 +8,7 @@ LimitedRandomnessCheck.cpp PostfixOperatorCheck.cpp SetLongJmpCheck.cpp + SignalHandlerMustBePlainOldFunctionCheck.cpp StaticObjectExceptionCheck.cpp StrToNumCheck.cpp ThrownExceptionTypeCheck.cpp Index: clang-tidy/cert/SignalHandlerMustBePlainOldFunctionCheck.h =================================================================== --- /dev/null +++ clang-tidy/cert/SignalHandlerMustBePlainOldFunctionCheck.h @@ -0,0 +1,35 @@ +//===--- SignalHandlerMustBePlainOldFunctionCheck.h - clang-tidy-*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_SIGNAL_HANDLER_MUST_BE_PLAIN_OLD_FUNCTION_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_SIGNAL_HANDLER_MUST_BE_PLAIN_OLD_FUNCTION_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace cert { + +/// A signal handler must be a plain old function. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/cert-msc54-cpp.html +class SignalHandlerMustBePlainOldFunctionCheck : public ClangTidyCheck { +public: + SignalHandlerMustBePlainOldFunctionCheck(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_SIGNAL_HANDLER_MUST_BE_PLAIN_OLD_FUNCTION_H Index: clang-tidy/cert/SignalHandlerMustBePlainOldFunctionCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/cert/SignalHandlerMustBePlainOldFunctionCheck.cpp @@ -0,0 +1,166 @@ +//===--- SignalHandlerMustBePlainOldFunctionCheck.cpp - clang-tidy---------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "SignalHandlerMustBePlainOldFunctionCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace cert { + +namespace { +internal::Matcher hasCxxStmt() { + return hasDescendant( + stmt(anyOf(cxxBindTemporaryExpr(), cxxBoolLiteral(), cxxCatchStmt(), + cxxConstCastExpr(), cxxConstructExpr(), cxxDefaultArgExpr(), + cxxDeleteExpr(), cxxDynamicCastExpr(), cxxForRangeStmt(), + cxxFunctionalCastExpr(), cxxMemberCallExpr(), cxxNewExpr(), + cxxNullPtrLiteralExpr(), cxxOperatorCallExpr(), + cxxReinterpretCastExpr(), cxxStaticCastExpr(), + cxxTemporaryObjectExpr(), cxxThisExpr(), cxxThrowExpr(), + cxxTryStmt(), cxxUnresolvedConstructExpr())) + .bind("cxx_stmt")); +} + +internal::Matcher hasCxxDecl() { + return hasDescendant( + decl(anyOf(cxxConstructorDecl(), cxxConversionDecl(), cxxDestructorDecl(), + cxxMethodDecl(), + cxxRecordDecl(unless(anyOf(isStruct(), isUnion()))))) + .bind("cxx_decl")); +} + +class CallGraphCheck { + std::queue UncheckedCalls; + llvm::DenseSet CheckedCalls; + SourceLocation CxxRepresentation; + SourceLocation FunctionCall; + ASTContext *Context; + + bool hasCxxRepr(const FunctionDecl *Func) { + auto MatchesCxxStmt = match(hasCxxStmt(), *Func, *Context); + if (!MatchesCxxStmt.empty()) { + CxxRepresentation = + MatchesCxxStmt[0].getNodeAs("cxx_stmt")->getLocStart(); + return true; + } + + auto MatchesCxxDecl = match(hasCxxDecl(), *Func, *Context); + if (!MatchesCxxDecl.empty()) { + CxxRepresentation = + MatchesCxxDecl[0].getNodeAs("cxx_decl")->getLocStart(); + return true; + } + return false; + } + + bool callExprContainsCxxRepr(SmallVectorImpl &Calls) { + for (const auto &Match : Calls) { + const auto *Call = Match.getNodeAs("call"); + const FunctionDecl *Func = Call->getDirectCallee(); + if (!Func || !Func->isDefined()) + continue; + auto MatchesCxxMethod = match(decl(cxxMethodDecl()), *Func, *Context); + if (hasCxxRepr(Func->getDefinition()) || !MatchesCxxMethod.empty()) { + FunctionCall = Call->getLocStart(); + return true; + } + if (!CheckedCalls.count(Func)) + UncheckedCalls.push(Func); + } + return false; + } + +public: + CallGraphCheck(const FunctionDecl *SignalHandler, ASTContext *ResultContext) + : Context(ResultContext) { + UncheckedCalls.push(SignalHandler); + } + const SourceLocation callLoc() { return FunctionCall; } + const SourceLocation cxxRepLoc() { return CxxRepresentation; } + + bool checkAllCall() { + while (!UncheckedCalls.empty()) { + CheckedCalls.insert(UncheckedCalls.front()); + auto Matches = match(decl(forEachDescendant(callExpr().bind("call"))), + *UncheckedCalls.front()->getDefinition(), *Context); + UncheckedCalls.pop(); + if (callExprContainsCxxRepr(Matches)) + return true; + } + return false; + } +}; +} // namespace + +void SignalHandlerMustBePlainOldFunctionCheck::registerMatchers( + MatchFinder *Finder) { + auto SignalHas = [](const internal::Matcher &SignalHandler) { + return callExpr(hasDeclaration(functionDecl(hasName("signal"))), + hasArgument(1, declRefExpr(hasDeclaration(SignalHandler)) + .bind("signal_argument"))); + }; + auto SignalHandler = [](const internal::Matcher &HasCxxRepr) { + return functionDecl(isExternC(), HasCxxRepr) + .bind("signal_handler_with_cxx_repr"); + }; + Finder->addMatcher( + SignalHas(functionDecl(unless(isExternC())).bind("signal_handler")), + this); + Finder->addMatcher(SignalHas(SignalHandler(hasCxxStmt())), this); + Finder->addMatcher(SignalHas(SignalHandler(hasCxxDecl())), this); + Finder->addMatcher(SignalHas(functionDecl(hasDescendant(callExpr())) + .bind("signal_handler_with_call_expr")), + this); +} + +void SignalHandlerMustBePlainOldFunctionCheck::check( + const MatchFinder::MatchResult &Result) { + if (const auto *SignalHandler = + Result.Nodes.getNodeAs("signal_handler")) { + diag(SignalHandler->getLocation(), + "signal handlers must be 'extern \"C\"'"); + diag(Result.Nodes.getNodeAs("signal_argument")->getLocation(), + "given as a signal parameter here", DiagnosticIDs::Note); + } + + if (const auto *SingalHandler = Result.Nodes.getNodeAs( + "signal_handler_with_cxx_repr")) { + diag(SingalHandler->getLocation(), + "do not use C++ constructs in signal handlers"); + if (const auto *CxxStmt = Result.Nodes.getNodeAs("cxx_stmt")) + diag(CxxStmt->getLocStart(), "C++ construct used here", + DiagnosticIDs::Note); + else + diag(Result.Nodes.getNodeAs("cxx_decl")->getLocStart(), + "C++ construct used here", DiagnosticIDs::Note); + } + + if (const auto *SignalHandler = Result.Nodes.getNodeAs( + "signal_handler_with_call_expr")) { + CallGraphCheck CallChain(SignalHandler, Result.Context); + if (CallChain.checkAllCall()) { + diag(SignalHandler->getLocation(), "do not call functions with C++ " + "constructs in signal " + "handlers"); + diag(CallChain.callLoc(), "function called here", DiagnosticIDs::Note); + if (CallChain.cxxRepLoc().isValid()) + diag(CallChain.cxxRepLoc(), "C++ construct used here", + DiagnosticIDs::Note); + } + } +} + +} // namespace cert +} // namespace tidy +} // namespace clang Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -56,6 +56,10 @@ Improvements to clang-tidy -------------------------- +- New `cert-msc54-cpp + `_ check + + Checks if a signal handler is an 'extern \"C\" function without C++ constructs. - Renamed checks to use correct term "implicit conversion" instead of "implicit cast" and modified messages and option names accordingly: Index: docs/clang-tidy/checks/cert-msc54-cpp.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/cert-msc54-cpp.rst @@ -0,0 +1,34 @@ +.. title:: clang-tidy - cert-msc54-cpp + +cert-msc54-cpp +============== + +This check will give a warning if a signal handler is not defined +as an `extern "C"` function or if the declaration of the function +contains C++ representation. + +Here's an example: + + .. code-block:: c++ + + static void sig_handler(int sig) {} + // warning: use 'external C' prefix for signal handlers + + void install_signal_handler() { + if (SIG_ERR == std::signal(SIGTERM, sig_handler)) + return; + } + + extern "C" void cpp_signal_handler(int sig) { + // warning: do not use C++ representations in signal handlers + throw "error message"; + } + + void install_cpp_signal_handler() { + if (SIG_ERR == std::signal(SIGTERM, cpp_signal_handler)) + return; + } + +This check corresponds to the CERT C++ Coding Standard rule +`MSC54-CPP. A signal handler must be a plain old function +`_. Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -37,6 +37,7 @@ cert-flp30-c cert-msc30-c (redirects to cert-msc50-cpp) cert-msc50-cpp + cert-msc54-cpp cert-oop11-cpp (redirects to misc-move-constructor-init) cppcoreguidelines-c-copy-assignment-signature cppcoreguidelines-interfaces-global-init Index: test/clang-tidy/cert-signal-handler-must-be-plain-old-function.cpp =================================================================== --- /dev/null +++ test/clang-tidy/cert-signal-handler-must-be-plain-old-function.cpp @@ -0,0 +1,134 @@ +// RUN: %check_clang_tidy %s cert-msc54-cpp %t -- -- -std=c++11 + +namespace std { +extern "C" using signalhandler = void(int); +int signal(int sig, signalhandler *func); +} + +#define SIG_ERR -1 +#define SIGTERM 15 + +static void sig_handler(int sig) {} +// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: signal handlers must be 'extern "C"' [cert-msc54-cpp] +// CHECK-MESSAGES: :[[@LINE+3]]:39: note: given as a signal parameter here + +void install_signal_handler() { + if (SIG_ERR == std::signal(SIGTERM, sig_handler)) + return; +} + +extern "C" void cpp_signal_handler(int sig) { + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not use C++ constructs in signal handlers [cert-msc54-cpp] + // CHECK-MESSAGES: :[[@LINE+1]]:3: note: C++ construct used here + throw "error message"; +} + +void install_cpp_signal_handler() { + if (SIG_ERR == std::signal(SIGTERM, cpp_signal_handler)) + return; +} + +void indirect_recursion(); + +void cpp_like(){ + try { + cpp_signal_handler(SIG_ERR); + } catch(...) { + // handle error + } +} + +void no_body(); + +void recursive_function() { + indirect_recursion(); + cpp_like(); + no_body(); + recursive_function(); +} + +void indirect_recursion() { + recursive_function(); +} + +extern "C" void signal_handler_with_cpp_function_call(int sig) { + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not call functions with C++ constructs in signal handlers [cert-msc54-cpp] + // CHECK-MESSAGES: :[[@LINE-11]]:3: note: function called here + // CHECK-MESSAGES: :[[@LINE-23]]:3: note: C++ construct used here + recursive_function(); +} + +void install_signal_handler_with_cpp_function_call() { + if (SIG_ERR == std::signal(SIGTERM, signal_handler_with_cpp_function_call)) + return; +} + +class TestClass { +public: + static void static_function() {} +}; + +extern "C" void signal_handler_with_cpp_static_method_call(int sig) { + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not call functions with C++ constructs in signal handlers [cert-msc54-cpp] + // CHECK-MESSAGES: :[[@LINE+1]]:3: note: function called here + TestClass::static_function(); +} + +void install_signal_handler_with_cpp_static_method_call() { + if (SIG_ERR == std::signal(SIGTERM, signal_handler_with_cpp_static_method_call)) + return; +} + + +// Something that does not trigger the check: + +extern "C" void c_sig_handler(int sig) { +#define cbrt(X) _Generic((X), long double \ + : cbrtl, default \ + : cbrt)(X) + auto char c = '1'; + extern _Thread_local double _Complex d; + static const unsigned long int i = sizeof(float); + register short s; + void f(); + volatile struct c_struct { + enum e {}; + union u; + }; + typedef bool boolean; +label: + switch (c) { + case ('1'): + break; + default: + d = 1.2; + } + goto label; + for (; i < 42;) { + if (d == 1.2) + continue; + else + return; + } + do { + _Atomic int v = _Alignof(char); + _Static_assert(42 == 42, "True"); + } while (c == 42); +} + +void install_c_sig_handler() { + if (SIG_ERR == std::signal(SIGTERM, c_sig_handler)) { + // Handle error + } +} + +extern "C" void signal_handler_with_function_pointer(int sig) { + void (*funp) (void); + funp = &cpp_like; + funp(); +} + +void install_signal_handler_with_function_pointer() { + if (SIG_ERR == std::signal(SIGTERM, signal_handler_with_function_pointer)) + return; +}