[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C

This revision was automatically updated to reflect the committed changes. Closed by commit rGd1b2a523191e: [clang-tidy] Add signal-handler-check for SEI CERT rule SIG30-C (authored by balazske). Changed prior to commit: https://reviews.llvm.org/D87449?vs=302778=302841#toc Repository: rG

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C

balazske updated this revision to Diff 302778. balazske added a comment. Updated according to comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87449/new/ https://reviews.llvm.org/D87449 Files:

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. I think this LGTM aside from a few minor nits. Thank you for working on this! Comment at: clang-tools-extra/docs/clang-tidy/checks/cert-sig30-c.rst:12-14 +This

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C

balazske updated this revision to Diff 302542. balazske added a comment. Rename of the checker. Using canonical decl for system function detection. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87449/new/ https://reviews.llvm.org/D87449 Files:

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C

aaron.ballman added a comment. In D87449#2358579 , @balazske wrote: > I think the name of this checker should be changed. It could in future not > only check for the SIG30-C rule. (Plan is to include C++ checks too, and > SIG31-C could be checked in

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C

balazske added a comment. I think the name of this checker should be changed. It could in future not only check for the SIG30-C rule. (Plan is to include C++ checks too, and SIG31-C could be checked in this checker too.) It can be called "bugprone-signal-handler" instead?

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C

balazske updated this revision to Diff 301215. balazske added a comment. Handled review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87449/new/ https://reviews.llvm.org/D87449 Files:

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C

balazske marked an inline comment as done. balazske added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp:33 + + for (const FunctionDecl *D : Node.redecls()) +if (D->getASTContext().getSourceManager().isInSystemHeader(

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp:131 +} +/*FunctionCallCollector Collector{[](const CallExpr *CE) { + if (isa(CE->getCalleeDecl())) Remove commented-out code?

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C

balazske updated this revision to Diff 297828. balazske added a comment. Removed C++ support. Other small changes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87449/new/ https://reviews.llvm.org/D87449 Files:

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C

balazske added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp:117-118 +FunctionCallCollector Collector{[](const CallExpr *CE) { + if (isa(CE->getCalleeDecl())) +CalledFunctions.push_back(CE); +}};

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp:117-118 +FunctionCallCollector Collector{[](const CallExpr *CE) { + if (isa(CE->getCalleeDecl())) +CalledFunctions.push_back(CE); +}};

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C

balazske added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp:117-118 +FunctionCallCollector Collector{[](const CallExpr *CE) { + if (isa(CE->getCalleeDecl())) +CalledFunctions.push_back(CE); +}};

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp:117-118 +FunctionCallCollector Collector{[](const CallExpr *CE) { + if (isa(CE->getCalleeDecl())) +CalledFunctions.push_back(CE); +}};

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C

balazske added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp:117-118 +FunctionCallCollector Collector{[](const CallExpr *CE) { + if (isa(CE->getCalleeDecl())) +CalledFunctions.push_back(CE); +}};

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp:117-118 +FunctionCallCollector Collector{[](const CallExpr *CE) { + if (isa(CE->getCalleeDecl())) +CalledFunctions.push_back(CE); +}};

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp:117-118 +FunctionCallCollector Collector{[](const CallExpr *CE) { + if (isa(CE->getCalleeDecl())) +CalledFunctions.push_back(CE); +}};

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C

balazske marked 3 inline comments as done. balazske added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp:117-118 +FunctionCallCollector Collector{[](const CallExpr *CE) { + if (isa(CE->getCalleeDecl())) +

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp:41 +static bool isAllowedSystemCall(const FunctionDecl *FD) { + if (!FD->getIdentifier()) +return true; balazske wrote: > aaron.ballman wrote: > > A

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C

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(

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C

balazske marked 9 inline comments as done. balazske added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp:33 + + for (const FunctionDecl *D : Node.redecls()) +if (D->getASTContext().getSourceManager().isInSystemHeader(

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C

balazske updated this revision to Diff 295840. balazske added a comment. Updated check for system function. Updated documentation. Added more test cases. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87449/new/ https://reviews.llvm.org/D87449

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp:33 + + for (const FunctionDecl *D : Node.redecls()) +if (D->getASTContext().getSourceManager().isInSystemHeader( balazske wrote: > aaron.ballman wrote:

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C

balazske added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp:33 + + for (const FunctionDecl *D : Node.redecls()) +if (D->getASTContext().getSourceManager().isInSystemHeader( aaron.ballman wrote: > balazske wrote: > >

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp:33 + + for (const FunctionDecl *D : Node.redecls()) +if (D->getASTContext().getSourceManager().isInSystemHeader( balazske wrote: > aaron.ballman wrote:

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C

balazske added a comment. I plan to add the option for extended set of asynchronous-safe functions (defined by the POSIX list) in a next patch. There is other possible improvement to check at least something of the criteria listed here

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C

balazske marked an inline comment as done. balazske added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp:33 + + for (const FunctionDecl *D : Node.redecls()) +if (D->getASTContext().getSourceManager().isInSystemHeader(

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp:33 + + for (const FunctionDecl *D : Node.redecls()) +if (D->getASTContext().getSourceManager().isInSystemHeader( I'm not certain I understand why we're

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C

balazske marked 12 inline comments as done. balazske added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp:95 + std::deque> CalledFunctions{ + {HandlerDecl, HandlerExpr}}; + baloghadamsoftware wrote: > Do we really

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C

balazske updated this revision to Diff 295538. balazske added a comment. Added support for C++ code. Improved detection of 'signal' function. Simplified collection of called functions. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87449/new/

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp:31-33 + // This check does not work with function calls in std namespace. + if (!FD->isGlobal() || FD->isInStdNamespace()) +return false;

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C.

baloghadamsoftware added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp:31 +static bool isSystemCall(const FunctionDecl *FD) { + // This check does not work with function calls in std namespace. + if (!FD->isGlobal() ||

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C.

balazske added a comment. Ping 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

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C.

balazske updated this revision to Diff 291524. balazske added a comment. Changed checker messages, reformatted text, rebase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87449/new/ https://reviews.llvm.org/D87449 Files:

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C.

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:94 + +- New :doc:`cert-sig30-c + ` check. Please rebase from trunk. New checks section is above and somehow header is missed in your file. Repository: rG LLVM Github

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C.

balazske updated this revision to Diff 291238. balazske added a comment. Added entry to release notes and fixed wrong comment in test headers. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87449/new/ https://reviews.llvm.org/D87449 Files:

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C.

Eugene.Zelenko added a comment. Release Notes were not updated yet. Comment at: clang-tools-extra/docs/clang-tidy/checks/cert-sig30-c.rst:10 +The minimal list of asynchronous-safe system functions is: +``abort()``, ``_Exit()``, ``quick_exit()`` and ``signal()`` (for ``signal``

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C.

balazske marked 4 inline comments as done. balazske added a comment. It looks like that the `clang-tidy/add_new_check.py` script does not work correctly, at least it "corrupts" the **list.rst** file, and creates files with no newline at end. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C.

balazske updated this revision to Diff 291158. balazske added a comment. - Code formatting fixes. - Updated description. - Improved CalledFunctionsCollector. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87449/new/ https://reviews.llvm.org/D87449

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C.

Eugene.Zelenko added a comment. Please add entry in Release Notes. Comment at: clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp:26 +namespace { +constexpr StringRef SignalFun = "signal"; +constexpr StringRef AbortFun = "abort"; Only type definitions

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C.

balazske created this revision. Herald added subscribers: cfe-commits, martong, phosek, gamesh411, Szelethus, dkrupp, xazax.hun, whisperity, mgorny. Herald added a project: clang. balazske requested review of this revision. SIG30-C. Call only asynchronous-safe functions within signal handlers