aaron.ballman added inline comments.
================ Comment at: clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp:27 + // Find a possible redeclaration in system header. + for (const FunctionDecl *D : FD->redecls()) + if (FD->getASTContext().getSourceManager().isInSystemHeader( ---------------- ``` return llvm::any_of(FD->redecls(), [](const FunctionDecl *D) { return D->getASTContext().getSourceManager().isInSystemHeader(D->getLocation()); }); ``` ================ Comment at: clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp:117-118 + FunctionCallCollector Collector{[&CalledFunctions](const CallExpr *CE) { + if (isa<FunctionDecl>(CE->getCalleeDecl())) + CalledFunctions.push_back(CE); + }}; ---------------- For correctness, I think you need to handle more than just calls to function declarations -- for instance, this should be just as problematic: ``` void some_signal_handler(int sig) { []{ puts("this should not be an escape hatch for the check); }(); } ``` even though the call expression in the signal handler doesn't resolve back to a function declaration. (Similar for blocks instead of lambdas.) WDYT? ================ Comment at: clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp:134 + // At insertion we have already ensured that only function calls are there. + const FunctionDecl *F = cast<FunctionDecl>(FunctionCall->getCalleeDecl()); + ---------------- `const auto *` ================ Comment at: clang-tools-extra/test/clang-tidy/checkers/cert-sig30-c.cpp:52 +void test() { + std::signal(SIGINT, handler_abort); + std::signal(SIGINT, handler__Exit); ---------------- I'd also like to see a test case where the handler to `signal` call is itself not a function call: ``` std::signal(SIGINT, [](int sig) { puts("I did the bad thing this way"); // should be diagnosed, yes? }); ``` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87449/new/ https://reviews.llvm.org/D87449 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits