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 @@ -33,14 +33,12 @@ void check(const ast_matchers::MatchFinder::MatchResult &Result) override; private: + bool checkFunction(const FunctionDecl *FD, const Expr *CallOrRef); bool isFunctionAsyncSafe(const FunctionDecl *FD) const; bool isSystemCallAsyncSafe(const FunctionDecl *FD) const; - void reportBug(const FunctionDecl *CalledFunction, const Expr *CallOrRef, - bool DirectHandler); - void reportHandlerCommon(llvm::df_iterator Itr, - const CallExpr *SignalCall, - const FunctionDecl *HandlerDecl, - const Expr *HandlerRef); + void reportHandlerChain(const llvm::df_iterator &Itr, + const FunctionDecl *HandlerDecl, + const Expr *HandlerRef); 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 @@ -142,30 +142,41 @@ "There should be at least one function added to call graph."); } - // Check for special case when the signal handler itself is an unsafe external - // function. - if (!isFunctionAsyncSafe(HandlerDecl)) { - reportBug(HandlerDecl, HandlerExpr, /*DirectHandler=*/true); + if (!HandlerDecl->hasBody()) { + checkFunction(HandlerDecl, HandlerExpr); return; } CallGraphNode *HandlerNode = CG.getNode(HandlerDecl); - // Signal handler can be external but not unsafe, no call graph in this case. - if (!HandlerNode) - return; + assert(HandlerNode && + "Handler has 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) { const auto *CallF = dyn_cast((*Itr)->getDecl()); - if (CallF && !isFunctionAsyncSafe(CallF)) { - assert(Itr.getPathLength() >= 2); - reportBug(CallF, findCallExpr(Itr.getPath(Itr.getPathLength() - 2), *Itr), - /*DirectHandler=*/false); - reportHandlerCommon(Itr, SignalCall, HandlerDecl, HandlerExpr); + if (CallF && CallF != HandlerDecl) { + if (checkFunction( + CallF, findCallExpr(Itr.getPath(Itr.getPathLength() - 2), *Itr))) + reportHandlerChain(Itr, HandlerDecl, HandlerExpr); } } } +bool SignalHandlerCheck::checkFunction(const FunctionDecl *FD, + const Expr *CallOrRef) { + const bool FunctionIsCalled = isa(CallOrRef); + + if (!isFunctionAsyncSafe(FD)) { + diag(CallOrRef->getBeginLoc(), "%0 may not be asynchronous-safe; " + "%select{using it as|calling it from}1 " + "a signal handler may be dangerous") + << FD << FunctionIsCalled; + return true; + } + + return false; +} + bool SignalHandlerCheck::isFunctionAsyncSafe(const FunctionDecl *FD) const { if (isSystemCall(FD)) return isSystemCallAsyncSafe(FD); @@ -186,16 +197,8 @@ return false; } -void SignalHandlerCheck::reportBug(const FunctionDecl *CalledFunction, - const Expr *CallOrRef, bool DirectHandler) { - diag(CallOrRef->getBeginLoc(), - "%0 may not be asynchronous-safe; %select{calling it from|using it as}1 " - "a signal handler may be dangerous") - << CalledFunction << DirectHandler; -} - -void SignalHandlerCheck::reportHandlerCommon( - llvm::df_iterator Itr, const CallExpr *SignalCall, +void SignalHandlerCheck::reportHandlerChain( + const llvm::df_iterator &Itr, const FunctionDecl *HandlerDecl, const Expr *HandlerRef) { int CallLevel = Itr.getPathLength() - 2; assert(CallLevel >= -1 && "Empty iterator?");