diff --git a/clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp b/clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp --- a/clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp +++ b/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("register_call"); + const auto *AssignStmt = Result.Nodes.getNodeAs("handler_assign"); + const auto *AssignDecl = Result.Nodes.getNodeAs("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("handler_decl"); const auto *HandlerExpr = Result.Nodes.getNodeAs("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; } diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/signal.h b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/signal.h --- a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/signal.h +++ b/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_ diff --git a/clang-tools-extra/test/clang-tidy/checkers/cert-sig30-c.cpp b/clang-tools-extra/test/clang-tidy/checkers/cert-sig30-c.cpp --- a/clang-tools-extra/test/clang-tidy/checkers/cert-sig30-c.cpp +++ b/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; +}