balazske updated this revision to Diff 329017.
balazske added a comment.
Adding test of notes.
Tests are re-arranged significantly.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97960/new/
https://reviews.llvm.org/D97960
Files:
clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp
clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.h
clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-handler-minimal.c
clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-handler-posix.c
clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-handler.c
clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-handler.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-handler.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-handler.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-handler.cpp
@@ -11,132 +11,198 @@
void signal(int, callback_t);
}
-struct S {
+struct Signal {
static void signal(int, callback_t);
- static void handler(int);
+};
- S() = default;
- S(int) {
- std::other_call();
- // Should be fixed.
- // CHECK-MESSAGES-NOT: :[[@LINE-2]]:5: warning: 'other_call' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
- };
+
+
+struct S_operator {
int operator-() const {
std::other_call();
- // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'other_call' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
+ // CHECK-NOTES: :[[@LINE-1]]:5: warning: system call 'other_call' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
+ // CHECK-NOTES: :[[@LINE-2]]:5: note: function 'other_call' called here from 'operator-'
+ // CHECK-NOTES: :[[@LINE+7]]:3: note: function 'operator-' called here from 'handler_operator'
+ // CHECK-NOTES: :[[@LINE+10]]:23: note: function 'handler_operator' registered here as signal handler
}
};
-struct TestMemberInit {
+extern "C" void handler_operator(int) {
+ S_operator S;
+ -S;
+}
+
+void test_operator() {
+ std::signal(SIGINT, handler_operator);
+}
+
+
+
+struct S_constructor {
+ S_constructor() = default;
+ S_constructor(int) {
+ std::other_call();
+ // Should be fixed.
+ // CHECK-NOTES-NOT: :[[@LINE-2]]:5: warning: system call 'other_call' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
+ };
+};
+
+extern "C" void handler_constructor(int) {
+ S_constructor S(0);
+}
+
+void test_constructor() {
+ std::signal(SIGINT, handler_constructor);
+}
+
+
+
+struct S_MemberInit {
static int memberInit() {
std::other_call();
// Should be fixed.
- // CHECK-MESSAGES-NOT: :[[@LINE-2]]:5: warning: 'other_call' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
+ // CHECK-NOTES-NOT: :[[@LINE-2]]:5: warning: system call 'other_call' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
}
int M = memberInit();
};
+extern "C" void handler_MemberInit(int) {
+ S_MemberInit S;
+}
+
+void test_MemberInit() {
+ std::signal(SIGINT, handler_MemberInit);
+}
+
+
+
int defaultInit() {
std::other_call();
- // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'other_call' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
+ // CHECK-NOTES: :[[@LINE-1]]:3: warning: system call 'other_call' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
+ // CHECK-NOTES: :[[@LINE-2]]:3: note: function 'other_call' called here from 'defaultInit'
+ // CHECK-NOTES: :[[@LINE+4]]:30: note: function 'defaultInit' called here from 'handler_DefaultInit'
+ // CHECK-NOTES: :[[@LINE+11]]:23: note: function 'handler_DefaultInit' registered here as signal handler
}
-void testDefaultInit(int I = defaultInit()) {
+void callDefaultInit(int I = defaultInit()) {
}
-void S::handler(int) {
+extern "C" void handler_DefaultInit(int) {
+ callDefaultInit();
+}
+
+void test_DefaultInit() {
+ std::signal(SIGINT, handler_DefaultInit);
+}
+
+
+
+extern "C" void handler_UnsafeFunction(int) {
std::other_call();
+ // CHECK-NOTES: :[[@LINE-1]]:3: warning: system call 'other_call' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
+ // CHECK-NOTES: :[[@LINE-2]]:3: note: function 'other_call' called here from 'handler_UnsafeFunction'
+ // CHECK-NOTES: :[[@LINE+4]]:23: note: function 'handler_UnsafeFunction' registered here as signal handler
}
-extern "C" {
+void test_UnsafeFunction() {
+ std::signal(SIGINT, handler_UnsafeFunction);
+}
-void handler_abort(int) {
- std::abort();
+
+
+extern "C" void handler_UnsafeOperator(int) {
+ std::SysStruct S;
+ S << 1;
+ // CHECK-NOTES: :[[@LINE-1]]:3: warning: system call 'operator<<' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
+ // CHECK-NOTES: :[[@LINE-2]]:3: note: function 'operator<<' called here from 'handler_UnsafeOperator'
+ // CHECK-NOTES: :[[@LINE+4]]:23: note: function 'handler_UnsafeOperator' registered here as signal handler
}
-void handler__Exit(int) {
- std::_Exit(0);
+void test_UnsafeOperator() {
+ std::signal(SIGINT, handler_UnsafeOperator);
}
-void handler_quick_exit(int) {
- std::quick_exit(0);
+
+
+extern "C" void handler_Lambda(int) {
+ int I = []() { std::other_call(); return 1; }();
+ // CHECK-NOTES: :[[@LINE-1]]:18: warning: system call 'other_call' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
+ // CHECK-NOTES: :[[@LINE-2]]:18: note: function 'other_call' called here from 'handler_Lambda'
+ // CHECK-NOTES: :[[@LINE+4]]:23: note: function 'handler_Lambda' registered here as signal handler
}
-void handler_signal(int) {
- // FIXME: It is only OK to call signal with the current signal number.
- std::signal(0, SIG_DFL);
- std::signal(0, SIG_IGN);
+void test_Lambda() {
+ std::signal(SIGINT, handler_Lambda);
}
-void handler_bad1(int) {
+
+
+void handler_noexternc(int) {
std::other_call();
- // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'other_call' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
}
-void handler_bad2(int) {
- std::SysStruct S;
- S << 1;
- // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'operator<<' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
-}
+struct StaticHandler {
+ static void handler(int);
+};
-void handler_bad3(int) {
- S S1(0);
+void StaticHandler::handler(int) {
+ std::other_call();
}
-void handler_bad4(int) {
- S S1;
- -S1;
+void test_noexternc() {
+ std::signal(SIGINT, handler_noexternc);
+ // CHECK-NOTES: :[[@LINE-1]]:23: warning: function 'handler_noexternc' should have C language linkage if used as signal handler [bugprone-signal-handler]
+ // CHECK-NOTES: :[[@LINE-15]]:1: note: handler function declared here
+ std::signal(SIGINT, StaticHandler::handler);
+ // CHECK-NOTES: :[[@LINE-1]]:23: warning: function 'handler' should have C language linkage if used as signal handler [bugprone-signal-handler]
+ // CHECK-NOTES: :[[@LINE-13]]:3: note: handler function declared here
}
-void handler_bad5(int) {
- TestMemberInit MI;
+
+
+extern "C" {
+
+void handler_abort(int) {
+ std::abort();
}
-void handler_bad6(int) {
- testDefaultInit();
+void handler__Exit(int) {
+ std::_Exit(0);
}
-void handler_bad7(int) {
- int I = []() { std::other_call(); return 2; }();
- // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'other_call' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
+void handler_quick_exit(int) {
+ std::quick_exit(0);
}
+
+void handler_signal(int) {
+ // FIXME: It is only OK to call signal with the current signal number.
+ std::signal(0, SIG_DFL);
+ std::signal(0, SIG_IGN);
}
-void handler_noexternc(int) {
- std::other_call();
}
void test() {
+ // Register signal handlers with allowed functions.
std::signal(SIGINT, handler_abort);
std::signal(SIGINT, handler__Exit);
+ std::signal(SIGINT, handler_quick_exit);
std::signal(SIGINT, handler_signal);
- std::signal(SIGINT, handler_bad1);
- std::signal(SIGINT, handler_bad2);
- // test call of user-defined operator
- std::signal(SIGINT, handler_bad3);
- // test call of constructor
- std::signal(SIGINT, handler_bad4);
- // test call of default member initializer
- std::signal(SIGINT, handler_bad5);
- // test call of default argument initializer
- std::signal(SIGINT, handler_bad6);
- // test lambda call in handler
- std::signal(SIGINT, handler_bad7);
- std::signal(SIGINT, handler_noexternc);
- // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: function 'handler_noexternc' should have C language linkage if used as signal handler [bugprone-signal-handler]
- // make a ExprWithCleanups
- std::signal(-S(), handler_noexternc);
- // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: function 'handler_noexternc' should have C language linkage if used as signal handler [bugprone-signal-handler]
- std::signal(SIGINT, S::handler);
- // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: function 'handler' should have C language linkage if used as signal handler [bugprone-signal-handler]
+ // Use a lambda that returns the signal handler.
+ // There is no warning for this case, no matter what the real handler will be.
+ std::signal(SIGINT, [](int) -> callback_t { return &handler_noexternc; }(1));
+
std::signal(SIGINT, [](int) { std::other_call(); });
- // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: lambda expression is not allowed as signal handler [bugprone-signal-handler]
- std::signal(SIGINT, [](int) -> callback_t { return &handler_bad1; }(1));
+ // CHECK-NOTES: :[[@LINE-1]]:23: warning: lambda expression is not allowed as signal handler [bugprone-signal-handler]
+
+ // make a ExprWithCleanups
+ std::signal(-S_operator(), [](int) { std::other_call(); });
+ // CHECK-NOTES: :[[@LINE-1]]:30: warning: lambda expression is not allowed as signal handler [bugprone-signal-handler]
- // Do not find problems here.
+ // Do not find problems here (non-standard 'signal' function).
signal(SIGINT, handler_abort, 1);
ns::signal(SIGINT, handler_abort);
- S::signal(SIGINT, handler_abort);
+ Signal::signal(SIGINT, handler_abort);
system_other::signal(SIGINT, handler_abort);
}
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-handler.c
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-handler.c
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-handler.c
@@ -13,56 +13,109 @@
typedef void (*sighandler_t)(int);
sighandler_t signal(int signum, sighandler_t handler);
-void handler_abort(int) {
- abort();
-}
+void f_extern();
-void handler_other(int) {
+
+
+void handler_printf(int) {
printf("1234");
- // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'printf' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
+ // CHECK-NOTES: :[[@LINE-1]]:3: warning: system call '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
}
-void handler_signal(int) {
- // FIXME: It is only OK to call signal with the current signal number.
- signal(0, SIG_DFL);
+void test_printf() {
+ signal(SIGINT, handler_printf);
}
-void f_ok() {
- abort();
+
+
+void handler_extern(int) {
+ f_extern();
+ // CHECK-NOTES: :[[@LINE-1]]:3: warning: function 'f_extern' may not be 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
}
-void f_bad() {
- printf("1234");
- // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'printf' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
+void test_extern() {
+ signal(SIGINT, handler_extern);
}
-void f_extern();
+
+
+void f_ok() {
+ abort();
+}
void handler_ok(int) {
f_ok();
}
+void test_ok() {
+ signal(SIGINT, handler_ok);
+}
+
+
+
+void f_bad() {
+ printf("1234");
+ // CHECK-NOTES: :[[@LINE-1]]:3: warning: system call '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
+}
+
void handler_bad(int) {
f_bad();
}
-void handler_extern(int) {
- f_extern();
- // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'f_extern' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
+void test_bad() {
+ signal(SIGINT, handler_bad);
+}
+
+
+
+void f_bad1() {
+ printf("1234");
+ // CHECK-NOTES: :[[@LINE-1]]:3: warning: system call '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+10]]:3: note: function 'f_bad1' called here from 'handler_bad1'
+ // CHECK-NOTES: :[[@LINE+13]]:18: note: function 'handler_bad1' registered here as signal handler
+}
+
+void f_bad2() {
+ f_bad1();
+}
+
+void handler_bad1(int) {
+ f_bad2();
+ f_bad1();
+}
+
+void test_bad1() {
+ signal(SIGINT, handler_bad1);
+}
+
+
+
+void handler_abort(int) {
+ abort();
+}
+
+void handler_signal(int) {
+ // FIXME: It is only OK to call signal with the current signal number.
+ signal(0, SIG_DFL);
}
void test() {
signal(SIGINT, handler_abort);
signal(SIGINT, handler_signal);
- signal(SIGINT, handler_other);
-
- signal(SIGINT, handler_ok);
- signal(SIGINT, handler_bad);
- signal(SIGINT, handler_extern);
signal(SIGINT, _Exit);
signal(SIGINT, other_call);
- // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'other_call' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
+ // CHECK-NOTES: :[[@LINE-1]]:18: warning: system call 'other_call' may not be asynchronous-safe; using it as signal handler may be dangerous [bugprone-signal-handler]
+ signal(SIGINT, f_extern);
+ // CHECK-NOTES: :[[@LINE-1]]:18: warning: function 'f_extern' may not be asynchronous-safe; using it as signal handler may be dangerous [bugprone-signal-handler]
signal(SIGINT, SIG_IGN);
signal(SIGINT, SIG_DFL);
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-handler-posix.c
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-handler-posix.c
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-handler-posix.c
@@ -11,7 +11,7 @@
void handler_bad(int) {
printf("1234");
- // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'printf' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: system call 'printf' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
}
void handler_good(int) {
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-handler-minimal.c
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-handler-minimal.c
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-handler-minimal.c
@@ -10,12 +10,12 @@
void handler_bad1(int) {
_exit(0);
- // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: '_exit' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: system call '_exit' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
}
void handler_bad2(void *dst, const void *src) {
memcpy(dst, src, 10);
- // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memcpy' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: system call 'memcpy' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
}
void handler_good(int) {
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 "llvm/ADT/DenseMap.h"
#include "llvm/ADT/StringSet.h"
namespace clang {
@@ -31,8 +32,14 @@
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
private:
- void reportBug(const FunctionDecl *CalledFunction, const Expr *CallOrRef,
- const CallExpr *SignalCall, const FunctionDecl *HandlerDecl);
+ using ParentMap =
+ llvm::DenseMap<const FunctionDecl *,
+ std::pair<const Expr *, const FunctionDecl *>>;
+
+ bool checkCallAllowed(const FunctionDecl *F, const Expr *CallOrRef,
+ const ParentMap &ParentOfCall);
+ void addCallStackNotes(const FunctionDecl *CheckedFunction,
+ const ParentMap &ParentOfCall);
void reportNonExternCBug(const Expr *HandlerRef,
const FunctionDecl *HandlerDecl);
void reportLambdaBug(const Expr *HandlerRef);
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
@@ -129,103 +129,110 @@
return;
}
- const auto *SignalCall = Result.Nodes.getNodeAs<CallExpr>("register_call");
const auto *HandlerDecl =
- Result.Nodes.getNodeAs<FunctionDecl>("handler_decl");
+ Result.Nodes.getNodeAs<FunctionDecl>("handler_decl")->getCanonicalDecl();
const auto *HandlerExpr = Result.Nodes.getNodeAs<DeclRefExpr>("handler_expr");
+ assert(HandlerDecl && HandlerExpr &&
+ "Handler function and reference to it should both be found.");
if (!HandlerDecl->isExternC()) {
reportNonExternCBug(HandlerExpr, HandlerDecl);
return;
}
- // 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;
- }
+ std::deque<const Expr *> CallsToCheck;
+ // Visit each function encountered in the callgraph only once.
+ // Store its caller expression and the function where it appears.
+ ParentMap ParentOfCall;
+
+ // For the real signal handler we have no parent call but a reference to it
+ // (in the call to 'signal').
+ ParentOfCall[HandlerDecl] = std::make_pair(HandlerExpr, nullptr);
+ CallsToCheck.push_back(HandlerExpr);
+
+ while (!CallsToCheck.empty()) {
+ const Expr *CallOrRef = CallsToCheck.front();
+ CallsToCheck.pop_front();
+ const FunctionDecl *F =
+ (CallOrRef == HandlerExpr)
+ ? HandlerDecl
+ : cast<FunctionDecl>(cast<CallExpr>(CallOrRef)->getCalleeDecl())
+ ->getCanonicalDecl();
+
+ if (!checkCallAllowed(F, CallOrRef, ParentOfCall))
+ continue;
- // Get the body of the encountered non-system call function.
const FunctionDecl *FBody;
- if (!F->hasBody(FBody)) {
- reportBug(F, CallOrRef, SignalCall, HandlerDecl);
- return false;
- }
+ if (!F->hasBody(FBody))
+ continue;
// 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);
+ if (const auto *CalleeF = dyn_cast<FunctionDecl>(CE->getCalleeDecl())) {
+ CalleeF = CalleeF->getCanonicalDecl();
+ if (!ParentOfCall.count(CalleeF)) {
+ ParentOfCall[CalleeF] = std::make_pair(CE, F);
+ CallsToCheck.push_back(CE);
+ }
+ }
}
-
- return true;
- };
-
- if (!ProcessFunction(HandlerDecl, HandlerExpr))
- 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;
}
}
-bool SignalHandlerCheck::isSystemCallAllowed(const FunctionDecl *FD) const {
- const IdentifierInfo *II = FD->getIdentifier();
- // Can not verify if std operators are safe to call.
- if (!II)
- return false;
-
- if (!FD->isInStdNamespace() && !isInNoNamespace(FD))
- return false;
+bool SignalHandlerCheck::checkCallAllowed(const FunctionDecl *F,
+ const Expr *CallOrRef,
+ const ParentMap &ParentOfCall) {
+ StringRef FunctionKindStr;
+ if (isSystemCall(F)) {
+ if (isSystemCallAllowed(F))
+ return true;
+ FunctionKindStr = "system call";
+ } else {
+ if (F->hasBody())
+ return true;
+ FunctionKindStr = "function";
+ }
- if (ConformingFunctions.count(II->getName()))
- return true;
+ if (isa<DeclRefExpr>(CallOrRef)) {
+ // Function F appears directly in 'signal' call.
+ diag(CallOrRef->getBeginLoc(), "%0 %1 may not be asynchronous-safe; using "
+ "it as signal handler may be dangerous")
+ << FunctionKindStr << F;
+ } else {
+ // Function F is called from another function.
+ diag(CallOrRef->getBeginLoc(),
+ "%0 %1 may not be asynchronous-safe; calling it from a signal handler "
+ "may be dangerous")
+ << FunctionKindStr << F;
+ addCallStackNotes(F, ParentOfCall);
+ }
return false;
}
-void SignalHandlerCheck::reportBug(const FunctionDecl *CalledFunction,
- const Expr *CallOrRef,
- const CallExpr *SignalCall,
- const FunctionDecl *HandlerDecl) {
- diag(CallOrRef->getBeginLoc(),
- "%0 may not be asynchronous-safe; "
- "calling it from a signal handler may be dangerous")
- << CalledFunction;
- diag(SignalCall->getSourceRange().getBegin(),
- "signal handler registered here", DiagnosticIDs::Note);
- diag(HandlerDecl->getBeginLoc(), "handler function declared here",
- DiagnosticIDs::Note);
+void SignalHandlerCheck::addCallStackNotes(const FunctionDecl *F,
+ const ParentMap &ParentOfCall) {
+ auto I = ParentOfCall.find(F);
+ while (I != ParentOfCall.end()) {
+ auto Caller = I->second;
+ if (const auto *HandlerRef = dyn_cast<DeclRefExpr>(Caller.first)) {
+ diag(HandlerRef->getBeginLoc(),
+ "function %0 registered here as signal handler", DiagnosticIDs::Note)
+ << F;
+ break;
+ } else {
+ const auto *Call = cast<CallExpr>(Caller.first);
+ diag(Call->getBeginLoc(), "function %0 called here from %1",
+ DiagnosticIDs::Note)
+ << F << Caller.second;
+ }
+ F = Caller.second;
+ I = ParentOfCall.find(F);
+ }
}
void SignalHandlerCheck::reportNonExternCBug(const Expr *HandlerRef,
@@ -242,6 +249,21 @@
"lambda expression is not allowed as signal handler");
}
+bool SignalHandlerCheck::isSystemCallAllowed(const FunctionDecl *FD) const {
+ const IdentifierInfo *II = FD->getIdentifier();
+ // Can not verify if std operators are safe to call.
+ if (!II)
+ return false;
+
+ if (!FD->isInStdNamespace() && !isInNoNamespace(FD))
+ return false;
+
+ if (ConformingFunctions.count(II->getName()))
+ return true;
+
+ return false;
+}
+
// This is the minimal set of safe functions.
// https://wiki.sei.cmu.edu/confluence/display/c/SIG30-C.+Call+only+asynchronous-safe+functions+within+signal+handlers
llvm::StringSet<> SignalHandlerCheck::MinimalConformingFunctions{
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
