njames93 added a comment.
It would be a good idea to emit notes to help explain the problem code, I'm
thinking something along the lines of:
warning: exit-handler potentially calls a jump function. Handlers should
terminate by returning [cert-env32-c]
note: exit-handler declared here
note: call to a jump function declared here
The second note would require you to track the call site along with the
FunctionDecl.
I wouldn't go as far as trying to track the entire call stack when functions in
exit handlers call functions that exit. While its possible, it could end up
looking messy.
================
Comment at: clang-tools-extra/clang-tidy/cert/ExitHandlerCheck.cpp:26
+namespace {
+constexpr StringRef EF__Exit = "_Exit";
+constexpr StringRef EF_exit = "exit";
----------------
Listen to the clang-tidy warnings.
================
Comment at: clang-tools-extra/clang-tidy/cert/ExitHandlerCheck.cpp:72
+void ExitHandlerCheck::registerMatchers(MatchFinder *Finder) {
+ // clang-format off
+ Finder->addMatcher(
----------------
Is there a reason for disabling format here? How messy is the matcher code
when formatted?
================
Comment at: clang-tools-extra/clang-tidy/cert/ExitHandlerCheck.cpp:78
+ anyOf(
+ hasName(RF_atexit),
+ hasName(RF_at_quick_exit)
----------------
Use `hasAnyName`
================
Comment at: clang-tools-extra/clang-tidy/cert/ExitHandlerCheck.cpp:120-122
+ if (SeenFunctions.count(Current))
+ continue;
+ SeenFunctions.insert(Current);
----------------
nit:
```
if (!SeenFunctions.insert(Current).second)
continue;
```
================
Comment at: clang-tools-extra/clang-tidy/cert/ExitHandlerCheck.cpp:151
+ // Add called functions to the worklist.
+ std::copy(Collector.begin(), Collector.end(),
+ std::back_inserter(CalledFunctions));
----------------
Use `llvm::copy` instead of `std::copy`
================
Comment at: clang-tools-extra/clang-tidy/cert/ExitHandlerCheck.h:18
+
+///
+/// Checker for SEI CERT rule ENV32-C
----------------
Unnecessary empty line
================
Comment at: clang-tools-extra/docs/ReleaseNotes.rst:87
+ <clang-tidy/checks/cert-env32-c>` check.
+ Finds functions registered by ``atexit`` and ``at_quick_exit`` those are
calling
+ exit functions ``_Exit``, ``exit``, ``quick_exit`` or ``longjmp``.
----------------
s/those/that
================
Comment at: clang-tools-extra/docs/clang-tidy/checks/cert-env32-c.rst:114
+
+This compliant solution does not call longjmp()but instead returns from the
exit handler normally:
+
----------------
Space after `longjmp() `
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83717/new/
https://reviews.llvm.org/D83717
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
