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.
Another change of the code design.
Code simplified again, now there is a single place to check
a handler function and less functions for bug report emitting.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D118370
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
@@ -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<clang::CallGraphNode *> Itr,
- const CallExpr *SignalCall,
- const FunctionDecl *HandlerDecl,
- const Expr *HandlerRef);
+ void reportHandlerChain(const llvm::df_iterator<clang::CallGraphNode *> &Itr,
+ const FunctionDecl *HandlerDecl,
+ const Expr *HandlerRef);
clang::CallGraph CG;
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
@@ -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<FunctionDecl>((*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<CallExpr>(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<clang::CallGraphNode *> Itr, const CallExpr *SignalCall,
+void SignalHandlerCheck::reportHandlerChain(
+ const llvm::df_iterator<clang::CallGraphNode *> &Itr,
const FunctionDecl *HandlerDecl, const Expr *HandlerRef) {
int CallLevel = Itr.getPathLength() - 2;
assert(CallLevel >= -1 && "Empty iterator?");
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
@@ -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<clang::CallGraphNode *> Itr,
- const CallExpr *SignalCall,
- const FunctionDecl *HandlerDecl,
- const Expr *HandlerRef);
+ void reportHandlerChain(const llvm::df_iterator<clang::CallGraphNode *> &Itr,
+ const FunctionDecl *HandlerDecl,
+ const Expr *HandlerRef);
clang::CallGraph CG;
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
@@ -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<FunctionDecl>((*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<CallExpr>(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<clang::CallGraphNode *> Itr, const CallExpr *SignalCall,
+void SignalHandlerCheck::reportHandlerChain(
+ const llvm::df_iterator<clang::CallGraphNode *> &Itr,
const FunctionDecl *HandlerDecl, const Expr *HandlerRef) {
int CallLevel = Itr.getPathLength() - 2;
assert(CallLevel >= -1 && "Empty iterator?");
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
