balazske created this revision.
Herald added subscribers: cfe-commits, martong, gamesh411, Szelethus, dkrupp,
xazax.hun, whisperity.
Herald added a project: clang.
balazske requested review of this revision.
The checker recognizes handlers assigned to members of a
"struct sigaction" variable. If a function is assigned to
these members it is assumable that the function is used as signal handler
(function 'sigaction' is called with it).
Without this simplification the check is possible only in path-sensitive way.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D88140
Files:
clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/signal.h
clang-tools-extra/test/clang-tidy/checkers/cert-sig30-c.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/cert-sig30-c.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/cert-sig30-c.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cert-sig30-c.cpp
@@ -70,3 +70,83 @@
// Do not find problems here.
signal(SIGINT, handler_bad, 1);
}
+
+void handler_sigaction1(int) {
+ printf("1234");
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'printf' is considered as non asynchronous-safe and should not be called from a signal handler [cert-sig30-c]
+}
+
+void handler_sigaction2(int) {
+ printf("1234");
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'printf' is considered as non asynchronous-safe and should not be called from a signal handler [cert-sig30-c]
+}
+
+void handler_sigaction3(int) {
+ printf("1234");
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'printf' is considered as non asynchronous-safe and should not be called from a signal handler [cert-sig30-c]
+}
+
+void handler_sigaction4(int) {
+ printf("1234");
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'printf' is considered as non asynchronous-safe and should not be called from a signal handler [cert-sig30-c]
+}
+
+void sigaction_handler1(int, siginfo_t *, void *) {
+ printf("1234");
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'printf' is considered as non asynchronous-safe and should not be called from a signal handler [cert-sig30-c]
+}
+
+void sigaction_handler2(int, siginfo_t *, void *) {
+ printf("1234");
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'printf' is considered as non asynchronous-safe and should not be called from a signal handler [cert-sig30-c]
+}
+
+void sigaction_handler3(int, siginfo_t *, void *) {
+ printf("1234");
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'printf' is considered as non asynchronous-safe and should not be called from a signal handler [cert-sig30-c]
+}
+
+void sigaction_handler4(int, siginfo_t *, void *) {
+ printf("1234");
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'printf' is considered as non asynchronous-safe and should not be called from a signal handler [cert-sig30-c]
+}
+
+void test_sigaction1() {
+ struct sigaction SA;
+ // The checker can find only assignment of the handler function into the struct sigaction.
+ // It is assumed that the assigned function is (or at least may be) used as signal handler.
+ SA.sa_handler = handler_sigaction1;
+ SA.sa_handler = SIG_IGN;
+
+ struct sigaction SA1 = {handler_sigaction2, 0, 0, 0, 0};
+}
+
+void test_sigaction2() {
+ struct sigaction SA;
+ SA.sa_sigaction = sigaction_handler1;
+
+ struct sigaction SA1 = {0, sigaction_handler2, 0, 0, 0};
+}
+
+void test_sigaction3() {
+ struct sigaction SA;
+ SA.sa_handler = handler_sigaction3;
+ SA.sa_sigaction = sigaction_handler3;
+
+ struct sigaction SA1 = {handler_sigaction4, sigaction_handler4, 0, 0};
+}
+
+void handler_sigaction5(int) {
+ printf("1234");
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'printf' is considered as non asynchronous-safe and should not be called from a signal handler [cert-sig30-c]
+}
+
+void sigaction_handler5(int, siginfo_t *, void *) {
+ printf("1234");
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'printf' is considered as non asynchronous-safe and should not be called from a signal handler [cert-sig30-c]
+}
+
+void test_pointer(struct sigaction *SA) {
+ SA->sa_handler = handler_sigaction5;
+ SA->sa_sigaction = sigaction_handler5;
+}
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/signal.h
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/signal.h
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/signal.h
@@ -19,4 +19,18 @@
typedef void (*sighandler_t)(int);
sighandler_t signal(int signum, sighandler_t handler);
+typedef int siginfo_t;
+typedef int sigset_t;
+
+struct sigaction {
+ void (*sa_handler)(int);
+ void (*sa_sigaction)(int, siginfo_t *, void *);
+ sigset_t sa_mask;
+ int sa_flags;
+ void (*sa_restorer)(void);
+};
+
+int sigaction(int signum, const struct sigaction *act,
+ struct sigaction *oldact);
+
#endif // _SIGNAL_H_
Index: clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp
+++ clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp
@@ -68,21 +68,61 @@
} // namespace
void SignalHandlerCheck::registerMatchers(MatchFinder *Finder) {
- const auto HandlerProtoType = functionProtoType(parameterCountIs(1));
+ const auto HandlerExpr =
+ declRefExpr(hasDeclaration(
+ functionDecl(parameterCountIs(1)).bind("handler_decl")),
+ unless(isExpandedFromMacro("SIG_IGN")),
+ unless(isExpandedFromMacro("SIG_DFL")))
+ .bind("handler_expr");
+ const auto SigactionHandlerExpr =
+ declRefExpr(hasDeclaration(
+ functionDecl(parameterCountIs(3)).bind("handler_decl")))
+ .bind("handler_expr");
+
const auto IsSignalFunction =
callee(functionDecl(hasName(SignalFun), parameterCountIs(2)));
- const auto HandlerAsSecondArg = hasArgument(
- 1, declRefExpr(hasDeclaration(functionDecl().bind("handler_decl")),
- unless(isExpandedFromMacro("SIG_IGN")),
- unless(isExpandedFromMacro("SIG_DFL")))
- .bind("handler_expr"));
- Finder->addMatcher(
- callExpr(IsSignalFunction, HandlerAsSecondArg).bind("register_call"),
- this);
+
+ Finder->addMatcher(callExpr(IsSignalFunction, hasArgument(1, HandlerExpr))
+ .bind("register_call"),
+ this);
+
+ auto StructSigactionObjectExpression =
+ hasObjectExpression(ignoringParenImpCasts(declRefExpr(
+ anyOf(hasType(recordDecl(hasName("sigaction"))),
+ hasType(pointsTo(recordDecl(hasName("sigaction"))))))));
+ auto HandlerMember = member(hasName("sa_handler"));
+ auto SigactionHandlerMember = member(hasName("sa_sigaction"));
+ auto HandlerAssign = binaryOperator(
+ isAssignmentOperator(),
+ hasLHS(memberExpr(StructSigactionObjectExpression, HandlerMember)),
+ hasRHS(ignoringParenImpCasts(HandlerExpr)));
+ auto SigactionHandlerAssign =
+ binaryOperator(isAssignmentOperator(),
+ hasLHS(memberExpr(StructSigactionObjectExpression,
+ SigactionHandlerMember)),
+ hasRHS(ignoringParenImpCasts(SigactionHandlerExpr)));
+ auto HandlerAssignInit =
+ varDecl(hasType(recordDecl(hasName("sigaction"))),
+ hasInitializer(initListExpr(
+ hasInit(0, ignoringParenImpCasts(HandlerExpr)))));
+ auto SigactionHandlerAssignInit =
+ varDecl(hasType(recordDecl(hasName("sigaction"))),
+ hasInitializer(initListExpr(
+ hasInit(1, ignoringParenImpCasts(SigactionHandlerExpr)))));
+
+ Finder->addMatcher(HandlerAssign.bind("handler_assign"), this);
+ Finder->addMatcher(HandlerAssignInit.bind("handler_assign"), this);
+ Finder->addMatcher(SigactionHandlerAssign.bind("handler_assign"), this);
+ Finder->addMatcher(SigactionHandlerAssignInit.bind("handler_assign"), this);
}
void SignalHandlerCheck::check(const MatchFinder::MatchResult &Result) {
const auto *SignalCall = Result.Nodes.getNodeAs<CallExpr>("register_call");
+ const auto *AssignStmt = Result.Nodes.getNodeAs<Stmt>("handler_assign");
+ const auto *AssignDecl = Result.Nodes.getNodeAs<Decl>("handler_assign");
+ assert(!(SignalCall && (AssignStmt || AssignDecl)) &&
+ "A 'signal' call or assign to 'sigaction' member should be found but "
+ "not both together.");
const auto *HandlerDecl =
Result.Nodes.getNodeAs<FunctionDecl>("handler_decl");
const auto *HandlerExpr = Result.Nodes.getNodeAs<DeclRefExpr>("handler_expr");
@@ -117,10 +157,17 @@
"'%0' is considered as non asynchronous-safe and "
"should not be called from a signal handler")
<< FunctionToCheck->getName();
- diag(SignalCall->getSourceRange().getBegin(),
- "signal handler registered here", DiagnosticIDs::Note);
diag(HandlerDecl->getBeginLoc(), "handler function declared here",
DiagnosticIDs::Note);
+ if (SignalCall)
+ diag(SignalCall->getSourceRange().getBegin(),
+ "signal handler registered here", DiagnosticIDs::Note);
+ if (AssignStmt)
+ diag(AssignStmt->getSourceRange().getBegin(), "signal handler set here",
+ DiagnosticIDs::Note);
+ if (AssignDecl)
+ diag(AssignDecl->getSourceRange().getBegin(), "signal handler set here",
+ DiagnosticIDs::Note);
break;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
