balazske created this revision.
Herald added subscribers: carlosgalvezp, steakhal, martong, gamesh411,
Szelethus, dkrupp, xazax.hun.
balazske requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
Using clang::CallGraph to get the called functions.
This makes a better foundation to improve support for
C++ and print the call chain.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D118016
Files:
clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp
clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.h
Index: clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.h
+++ clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.h
@@ -10,6 +10,7 @@
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SIGNALHANDLERCHECK_H
#include "../ClangTidyCheck.h"
+#include "clang/Analysis/CallGraph.h"
#include "llvm/ADT/StringSet.h"
namespace clang {
@@ -31,9 +32,12 @@
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
private:
+ bool isFunctionAsyncSafe(const FunctionDecl *FD) const;
+ bool isSystemCallAsyncSafe(const FunctionDecl *FD) const;
void reportBug(const FunctionDecl *CalledFunction, const Expr *CallOrRef,
const CallExpr *SignalCall, const FunctionDecl *HandlerDecl);
- bool isSystemCallAllowed(const FunctionDecl *FD) const;
+
+ CallGraph CG;
AsyncSafeFunctionSetType AsyncSafeFunctionSet;
llvm::StringSet<> &ConformingFunctions;
Index: clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp
@@ -7,15 +7,9 @@
//===----------------------------------------------------------------------===//
#include "SignalHandlerCheck.h"
-#include "clang/AST/ASTContext.h"
-#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
-#include "clang/Analysis/CallGraph.h"
-#include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/DepthFirstIterator.h"
#include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/SmallVector.h"
-#include <iterator>
-#include <queue>
using namespace clang::ast_matchers;
@@ -42,7 +36,9 @@
namespace bugprone {
-static bool isSystemCall(const FunctionDecl *FD) {
+namespace {
+
+bool isSystemCall(const FunctionDecl *FD) {
// Find a possible redeclaration in system header.
// FIXME: Looking at the canonical declaration is not the most exact way
// to do this.
@@ -73,6 +69,22 @@
FD->getCanonicalDecl()->getLocation());
}
+/// Given a call graph node of a function and another one that is called from
+/// this function, get a CallExpr of the corresponding function call.
+/// It is unspecified which call is found if multiple calls exist, but the order
+/// should be deterministic (depend only on the AST).
+Expr *findCallExpr(const CallGraphNode *Caller, const CallGraphNode *Callee) {
+ auto FoundCallee = llvm::find_if(
+ Caller->callees(), [Callee](const CallGraphNode::CallRecord &Call) {
+ return Call.Callee == Callee;
+ });
+ assert(FoundCallee != Caller->end() &&
+ "Callee should be called from the caller function here.");
+ return FoundCallee->CallExpr;
+}
+
+} // namespace
+
AST_MATCHER(FunctionDecl, isSystemCall) { return isSystemCall(&Node); }
SignalHandlerCheck::SignalHandlerCheck(StringRef Name,
@@ -110,75 +122,57 @@
callExpr(callee(SignalFunction), hasArgument(1, HandlerExpr))
.bind("register_call"),
this);
+
+ Finder->addMatcher(translationUnitDecl().bind("TU"), this);
}
void SignalHandlerCheck::check(const MatchFinder::MatchResult &Result) {
+ if (const auto *TU = Result.Nodes.getNodeAs<TranslationUnitDecl>("TU")) {
+ // Call graph must be populated with the entire TU at the begin.
+ // (It is possible to add a single function but the functions called from it
+ // are not analysed in this case.)
+ CG.addToCallGraph(const_cast<TranslationUnitDecl *>(TU));
+ return;
+ }
+
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");
+ assert(SignalCall && HandlerDecl && HandlerExpr &&
+ "All of these should exist in a match here.");
- // 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;
-
- reportBug(F, CallOrRef, SignalCall, HandlerDecl);
-
- return false;
- }
-
- // Get the body of the encountered non-system call function.
- const FunctionDecl *FBody;
- if (!F->hasBody(FBody)) {
- reportBug(F, CallOrRef, SignalCall, HandlerDecl);
- return false;
- }
-
- // Collect all called functions.
- auto Matches = match(decl(forEachDescendant(callExpr().bind("call"))),
- *FBody, FBody->getASTContext());
- for (const auto &Match : Matches) {
- const auto *CE = Match.getNodeAs<CallExpr>("call");
- if (isa<FunctionDecl>(CE->getCalleeDecl()))
- CalledFunctions.push_back(CE);
- }
-
- return true;
- };
-
- if (!ProcessFunction(HandlerDecl, HandlerExpr))
+ // Check for special case when the signal handler itself is an unsafe external
+ // function.
+ if (!isFunctionAsyncSafe(HandlerDecl)) {
+ reportBug(HandlerDecl, HandlerExpr, SignalCall, HandlerDecl);
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 auto *F = cast<FunctionDecl>(FunctionCall->getCalleeDecl());
-
- if (!ProcessFunction(F, FunctionCall))
- break;
+ CallGraphNode *HandlerNode = CG.getNode(HandlerDecl);
+ // Signal handler can be external but not unsafe, no call graph in this case.
+ if (!HandlerNode)
+ return;
+ // 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<FunctionDecl>((*Itr)->getDecl());
+ if (CallF && !isFunctionAsyncSafe(CallF)) {
+ assert(Itr.getPathLength() >= 2);
+ reportBug(CallF, findCallExpr(Itr.getPath(Itr.getPathLength() - 2), *Itr),
+ SignalCall, HandlerDecl);
+ }
}
}
-bool SignalHandlerCheck::isSystemCallAllowed(const FunctionDecl *FD) const {
+bool SignalHandlerCheck::isFunctionAsyncSafe(const FunctionDecl *FD) const {
+ if (isSystemCall(FD))
+ return isSystemCallAsyncSafe(FD);
+ // For external (not checkable) functions assume that these are unsafe.
+ return FD->hasBody();
+}
+
+bool SignalHandlerCheck::isSystemCallAsyncSafe(const FunctionDecl *FD) const {
const IdentifierInfo *II = FD->getIdentifier();
// Unnamed functions are not explicitly allowed.
if (!II)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
