[clang-tidy] Support C++14 in bugprone-signal-handler.

[balazske]

Summary

Check bugprone-signal-handler is improved to check for
C++-specific constructs in signal handlers. This check is
valid until C++17.

[LegalizeAdulthood]

clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp

13–14

return LangOpts.CPlusPlus17 == 0;

13–14

Is there any utility/performance to be gained by combining these into a single matcher?
e.g.

callExpr(callee(SignalFunction), anyOf(hasArgument(1, HandlerExpr), hasArgument(1, HandlerLambda)))

(not sure if that is syntactically valid, but you get my point)

13–14

drop braces on 'then' block per style guide

13–14

drop braces per style guide

14

isInGlobalNamespace might be a better name?

14

How can the first expression here ever be false when
we rejected C++17 in the isLanguageVersionSupported
override?

23–31

Style guide says no braces on single statement blocks

36

Hrm, I was curious what getStmtClassName() does, but there doesn't seem to be any doxygen on it or the underlying data structure it accesses. Drilling into it with the IDE seems to say that it's a bunch of enums and corresponding names for each statement kind?

[balazske]

Rebase, small fixes.

[balazske]

clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp

13–14

I think that readability does not get better if the braces are removed here, specially if only from one branch. The rules here are not strict in this case.

14

I was thinking for the case when this check supports C++17 too. The change can be added later, this makes current code more readable.

23–31

Improved the code but I think if an if branch has braces then the else should have too.

36

It should return the statement class name, comes somehow from TableGen. Here if the class name contains CXX it is recognized as a C++ statement. This check is not fully accurate (for example LambdaExpr is missing) but there is likely some other CXX statement around.

[whisperity]

clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp

13–14

Or perhaps

callExpr(callee(SignalFunction), hasArgument(1, expr(anyOf(HandlerExpr, HandlerLambda))));

13–14

problem?

13–14

🔮 Ever since D98635, diag() of Clang-Tidy should support full undersquigglying of entities in diagnostics. Perhaps if you're working on this check already, it would be a nice addition to this check's output! All you need to do is << /* something that is a SourceRange */; into the result of a diag() call. 😃

13–14

(🔮 Ditto.)

14

Is this feature not available in (some parent class of) FunctionDecl?

14

Okay, this is interesting... Why is Ctx not const? Unless getSomething() is changing things (which is non-intuitive then...), as far as I can tell, you're only reading from Ctx.

14

(Nit: maybe it is better without this newline, to visibly demarcate the "local state init "block"" of the function.)

14

Either this... 🗡️

14

🗡️ ... or this.

14

(🗡️ Ditto.)

14

(True. I think @LegalizeAdulthood's comment can be marked as Done.)

14–15

16

Are these objects cleaned up properly in their destructor (same question for DynTypeNodeList)?

16

What is happening here? Why is the last parameter a callback(?) taking a bool? Why is the lambda empty? I see it is a std::function, can't we instead pass an empty function object there, and guard against calling it in checkFunction, making things a bit more explicit?

17

SourceRange itself should have a sentinel query. Why is only the beginning of it interesting?

17

I am trying to find some source for this claim but so far hit a blank. 😦 Could you please elaborate where this information comes from? Most notably, std::signal on CppReference makes no mention of this, which would be the first place I would expect most people to look at.

Maybe this claim is derived from the rule that signal handlers MUST have C linkage? (If so, is there a warning against people setting C++-linkage functions as signal handlers in this check in general?)

18–20

(🔮 Ditto.)

19

(Perhaps a >= C++20 fixme that this query here might not handle modules well?)

19–21

size != 1 could still mean size == 0 in which case taking an element seems dangerous. Is triggering such UB possible here?

21

(🗡️ Ditto.)

41–43

44–45

I think this is a fine moment to use the Remark diagnostic category! ✨ It is very likely that the average client will not look into Clang internals here... (It is dubious whether or not emitting this diagnostic is meaningful in the first place, but at least it helps with some debugging/understanding for those who wish to dig deep.)

clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.h

27

Reading "set type" below where this is used, I would've thought this will be a specific instantiation of SparseSet at first.

39–40

What is a "function property"? (Or is that an implementation detail of the check?)

46–57

48

(Nit: to keep consistency with others and to ensure documentation renders properly, use @p checkFunction instead of backtick.)

50

(Format ditto.)

clang-tools-extra/docs/ReleaseNotes.rst

136–137

swap supports and partially

clang-tools-extra/docs/clang-tidy/checks/bugprone-signal-handler.rst

8

(Is it the same for C all across the board?)

10–19

I would turn these into bullet points:

• For C, it is checked [...]
• Up to and including C++14, [...]
• Since C++17, [...]
  • C++17 and newer standards are not implemented in this check.

12–16

Turning these into bullet points will make it easier for the clients to read and you can optimise away this troublesome sentence.

• Up to and including C++14, there are several language constructs which are unsafe to call from a signal handler.
  • The signal handler, and every function it calls, MUST have C linkage.
  • Called functions must be asynchronous-safe.
  • etc.

16

22

24–25

25–28

I would rephrase this paragraph. You say that only the names are considered but the "against what" is not clear at this moment. Start with the expected outcome, the "result" function of this consideration:

Asnychronous-safety is determined by comparing the function's name against a set of known functions. In addition, the function must be a system call. The (possible) arguments passed to the function are not checked.

Just as in D118370, I'm not exactly sure what is a system call here, as printf and memcpy are (standard) library functions. Either this system call terminology should be dropped in favour of standard function perhaps, or the details of this be made explicit. Looking at the implementation of the check, the predicate for system call is: comes from a system header include + is in a global namespace + ???.

36

40–53

Again, this could use a re-working as a list of bullet-points.

42

(I'd repeat the external URL and make CERT SIG30-C rule clickable.)

[whisperity]

clang-tools-extra/docs/clang-tidy/checks/bugprone-signal-handler.rst

44–47

Nit: Perhaps the link could be worked into the POSIX.1-2017 somehow?

that is listed in POSIX.1-2017 (§ 2.4.3. Signal actions).

clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-handler.c

21

I really like that this trivial bit stops being repeated! Is this what the SkipPathEnd does in practice?

35

I went back to the implementation but I am still unsure what is being exercised by this change. Is it that we are not sensitive to conditionals and still think that the signal call will set handler_extern to be a handler no matter what, and thus we produce diagnostics? I think this is worthy of a comment in the file itself, because as far as I can tell there are no matchers that try to catch ifs in the check's implementation.

167–169

What is the expected behavioural change by this? I don't see lines referring these signal() calls in other test cases. Should there be diagnostics but not yet implemented? Or should there just be no crashes? Both cases I believe should be mentioned in the comments.

clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-handler.cpp

6

Stale comment. Or perhaps this is important?

60

[balazske]

clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp

17

This check is made to comply with a CERT rule MSC54-CPP. According to this only the subset of C and C++ should be used in a signal handler, and it should have extern "C" linkage. This does not allow lambda functions because the linkage restriction (if not others). (From C++17 on other rules apply, this check is not applicable for such code.)

clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.h

48

I see that the backtick character is used at other places. It should work with Doxygen. The \c is the normal Doxygen command to make code-style text format, \p is used for function parameters.

[whisperity]

clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp

17

Ah, right, I found it a bit further down the page:

Signal handlers are expected to have C linkage and, in general, only use the features from the common subset of C and C++. It is implementation-defined if a function with C++ linkage can be used as a signal handler. (until C++17)

[balazske]

rebase, applied the review comments

[balazske]

Code is updated, documentation (rst) not yet. I want to use \ format of tags in the code comments. Many review comments are now out of place (because the list of functions was moved around in the file.)

clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp

14

It does only reading, but getParentMapContext does not work with const.

16

I think it should work if there is not a function for deallocating.

16

Documentation of the function checkFunction tells what is the last parameter for: It is used to display the call chain notes (works as callback). A function object is used because then more information can be passed into it (instead of additional parameters to checkFunction) and it is possible to pass an empty function if needed. Here this function is not used, it is empty.

19–21

size is exactly 1 after the if

44–45

The statement class is often relatively easy to understand without looking into source code. Still it is a hint only, but there is no easy way to get a description for any statement class (without hard-coding into the checker).

clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.h

39–40

I meant a property of the function in everyday sense, for example is it "extern C" or not, const or not, and similar (anything that is not in the body part).

clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-handler.c

21

Yes SkipPathEnd is added to remove these redundant notes.

[balazske]

clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp

254

This will be removed (FunctionDecl::isGlobal should work for this purpose).

[balazske]

Removed a left comment and improved another comment.

[balazske]

Rebase to newer 'main' branch.

[whisperity]

After adding improvements to the documentation, I think this will be good to go, and thank you! Perhaps just for a safety measure you could run it on a few projects (LLVM itself?) to ensure we didn't miss a case where it might magically crash, but I wonder how many specifically "C++14" projects will use signal handlers in the first place.

clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp

347

Aren't these bools?

[njames93]

clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp

13–14

Is this check valid on Objective-C code?

472

s/with other than/without

491

This just feels very hacky and it's non exhaustive, would lambda for a start as that isn't prefixed internally with CXX.

[steakhal]

In D118996#3473934, @whisperity wrote:

After adding improvements to the documentation, I think this will be good to go, and thank you! Perhaps just for a safety measure you could run it on a few projects (LLVM itself?) to ensure we didn't miss a case where it might magically crash, but I wonder how many specifically "C++14" projects will use signal handlers in the first place.

It did not crash on llvm, nor on our internal test set.

[balazske]

clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp

13–14

I do not know if the check is applicable to Objective-C, it may depend on if the C standard (or POSIX) is applicable to Objective-C.

347

This is a 1-bit bit field member.

491

This is somewhat "hacky" but is more future-proof if new Stmt classes are added. I extend the list with isa checks for classes in ExprCXX.h and StmtCXX.h. But it is likely that when a new C++-only node is added this list is not updated (maybe no problem because new things are for later than C++17).

[balazske]

Applied the review comments.

[whisperity]

@njames93 What do you think about the current approach? It will under-approximate the problem-inducing node set but at least cover what we know about C++ now.

(@balazske please mark "Done" the answered discussion threads.)

clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp

284–296

isa<> is a variadic template since a few major releases ago, i.e. isa<T1, T2, T3>(X) is supported too.

(Also, please order this alphabetically, so it's easier to read and insert into at later edits.)

285

Is CUDA built upon C++-specific features?

[balazske]

Using one isa statement for all class names.
Changed the documentation.

[balazske]

clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp

285

It looks like no, but the class CUDAKernelCallExpr resides in ExprCXX.h so I think it is a C++ specific class. This may be a C++ extension.

[balazske]

Ping
I have a question with Objective-C support.
@njames93 Could you look at this again?

clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp

13–14

I could not find out if it is valid for Objective-C, probably not? But there are other checkers (for example in the concurrency module) that look POSIX or runtime-library specific and these do not contain check for ObjC language.

[LegalizeAdulthood]

Clang-Tidy tests and docs have moved to module subdirectories.

Please rebase this onto main:HEAD and:

• fold your changes into the appropriate subdirs, stripping the module prefix from new files.
• make the target check-clang-extra to validate your tests
• make the target docs-clang-tools-html to validate any docs changes

[balazske]

updated for new directory layout, fixed documentation

[LegalizeAdulthood]

clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp

281

I'm not a fan of this macro, and we're only using it once as the argument to isa<>; is there some problem with writing it inline?

clang-tools-extra/docs/ReleaseNotes.rst

161

Please keep the section sorted by check, so this should be inserted before bugprone-use-after-move

clang-tools-extra/test/clang-tidy/checkers/bugprone/signal-handler.cpp

1 ↗

(On Diff #439751)

Prefer -isystem %clang_tidy_headers instead of -isystem %S/../Inputs/Headers

[balazske]

clang-tools-extra/docs/ReleaseNotes.rst

161

The list is already not sorted.

[balazske]

Applied the review comments.

[LegalizeAdulthood]

clang-tools-extra/docs/ReleaseNotes.rst

158

sort by check name

[balazske]

Sorted the whole changed-check list in release notes.

[balazske]

Ping

[whisperity]

Alright, I think we should have this in and let C++17 things to be future work. Please see the inline comment, but otherwise this should be good enough. Can always improve in a future version. 😉

LLVM 15 has branched off, so this should be rebased for the last time against the current main branch, just to ensure everything is good.

clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp

283–297

I have an overarching solution idea for this, @balazske, which requires a separate patch independently of this. (I believe this will not be a hard thing to implement, just tedious, because of all the classes that need to be changed.)

Let's consider adding a new member function to Decl, Stmt, Type, etc. instances. While bool isCXX() const seems tempting, I believe it should be something like bool isAvailableIn(const LangOptions&) const, which returns whether a particular node (representing a language feature) is available in a particular language option set. And this way, we could query if a node is available in C++ (whichever version) and not in C, in which case the check can deduce that it is "C++-only".

This way, the future maintenance burden will be completely lifted from the check, and the AST library/API gets a useful generic feature that we can start using elsewhere, too!

---

So, e.g., LambdaExpr will have:

bool LambdaExpr::isAvailableIn(const LangOptions& LO) const
{
  return LO.CPlusPlus11;
}

[balazske]

Rebase.

[RKSimon]

@balazske This is failing on https://lab.llvm.org/buildbot/#/builders/139/builds/26335 - do you have a fix in progress or should I revert?

[balazske]

In D118996#3712218, @RKSimon wrote:

@balazske This is failing on https://lab.llvm.org/buildbot/#/builders/139/builds/26335 - do you have a fix in progress or should I revert?

Fix is not in progress, it may take some days.

[andrewng]

In D118996#3712493, @balazske wrote:

In D118996#3712218, @RKSimon wrote:

@balazske This is failing on https://lab.llvm.org/buildbot/#/builders/139/builds/26335 - do you have a fix in progress or should I revert?

Fix is not in progress, it may take some days.

I believe the problem is due to target specific default settings. Therefore, adding an explicit target, e.g. -target x86_64-unknown-unknown, to the %check_clang_tidy call should fix the problem.

[balazske]

The target option was added, but I can not verify if the change fixes all the buildbots, we will see if it works.

clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.h

 namespace bugprone {
 
 /// Checker for signal handler functions.
 ///
 /// For the user-facing documentation see:
 /// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-signal-handler-check.html
 class SignalHandlerCheck : public ClangTidyCheck {
 public:
   enum class AsyncSafeFunctionSetKind { Minimal, POSIX };
 
   SignalHandlerCheck(StringRef Name, ClangTidyContext *Context);
   void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   bool isLanguageVersionSupported(const LangOptions &LangOpts) const override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
 
 private:
   /// Check if a function is allowed as a signal handler.
   /// Should test the properties of the function, and check in the code body.
   /// Should not check function calls in the code (this part is done by the call
   /// graph scan).
-  /// @param FD The function to check. It may or may not have a definition.
-  /// @param CallOrRef Location of the call to this function (in another
+  /// Bug reports are generated for the whole code body (no stop at the first
+  /// found issue). For issues that are not in the code body, only one
+  /// bug report is generated.
+  /// \param FD The function to check. It may or may not have a definition.
+  /// \param CallOrRef Location of the call to this function (in another
   /// function) or the reference to the function (if it is used as a registered
   /// signal handler). This is the location where diagnostics are to be placed.
-  /// @return Returns true if a diagnostic was emitted for this function.
-  bool checkFunction(const FunctionDecl *FD, const Expr *CallOrRef);
-  /// Returns true if a standard library function is considered as
+  /// \param ChainReporter A function that adds bug report notes to display the
+  /// chain of called functions from signal handler registration to the current
+  /// function. This is called at every generated bug report.
+  /// The bool parameter is used like \c SkipPathEnd in \c reportHandlerChain .
+  /// \return Returns true if a diagnostic was emitted for this function.
+  bool checkFunction(const FunctionDecl *FD, const Expr *CallOrRef,
+                     std::function<void(bool)> ChainReporter);
+  /// Similar as \c checkFunction but only check for C++14 rules.
+  bool checkFunctionCPP14(const FunctionDecl *FD, const Expr *CallOrRef,
+                          std::function<void(bool)> ChainReporter);
+  /// Returns true if a standard library function is considered
   /// asynchronous-safe.
   bool isStandardFunctionAsyncSafe(const FunctionDecl *FD) const;
   /// Add diagnostic notes to show the call chain of functions from a signal
   /// handler to a function that is called (directly or indirectly) from it.
   /// Also add a note to the place where the signal handler is registered.
   /// @param Itr Position during a call graph depth-first iteration. It contains
   /// the "path" (call chain) from the signal handler to the actual found
   /// function call.
   /// @param HandlerRef Reference to the signal handler function where it is
   /// registered as signal handler.
+  /// @param SkipPathEnd If true the last item of the call chain (farthest away
+  /// from the \c signal call) is omitted from note generation.
   void reportHandlerChain(const llvm::df_iterator<clang::CallGraphNode *> &Itr,
-                          const DeclRefExpr *HandlerRef);
+                          const DeclRefExpr *HandlerRef, bool SkipPathEnd);
 
   clang::CallGraph CG;
 
   AsyncSafeFunctionSetKind AsyncSafeFunctionSet;
   const llvm::StringSet<> ConformingFunctions;
 };
 
 } // namespace bugprone
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SIGNALHANDLERCHECK_H

clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp

 //===--- SignalHandlerCheck.cpp - clang-tidy ------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "SignalHandlerCheck.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "llvm/ADT/DepthFirstIterator.h"
 #include "llvm/ADT/STLExtras.h"
 
 // This is the minimal set of safe functions.
 // https://wiki.sei.cmu.edu/confluence/display/c/SIG30-C.+Call+only+asynchronous-safe+functions+within+signal+handlers
 constexpr std::initializer_list<llvm::StringRef> MinimalConformingFunctions = {
     "signal", "abort", "_Exit", "quick_exit"};
 
 // The POSIX-defined set of safe functions.
 // https://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#tag_15_04_03
 // 'quick_exit' is added to the set additionally because it looks like the
 // mentioned POSIX specification was not updated after 'quick_exit' appeared
 // in the C11 standard.
 // Also, we want to keep the "minimal set" a subset of the "POSIX set".
 constexpr std::initializer_list<llvm::StringRef> POSIXConformingFunctions = {
     "_Exit",
     "_exit",
     "abort",
     "accept",
     "access",
     "aio_error",
     "aio_return",
     "aio_suspend",
     "alarm",
     "bind",
     "cfgetispeed",
     "cfgetospeed",
     "cfsetispeed",
     "cfsetospeed",
     "chdir",
     "chmod",
     "chown",
     "clock_gettime",
     "close",
     "connect",
     "creat",
     "dup",
     "dup2",
     "execl",
     "execle",
     "execv",
     "execve",
     "faccessat",

 /// Returns if a function is declared inside a system header.
 /// These functions are considered to be "standard" (system-provided) library
 /// functions.
 bool isStandardFunction(const FunctionDecl *FD) {
   // Find a possible redeclaration in system header.
   // FIXME: Looking at the canonical declaration is not the most exact way
   // to do this.
 
   // Most common case will be inclusion directly from a header.
   // This works fine by using canonical declaration.
   // a.c
   // #include <sysheader.h>
 
   // Next most common case will be extern declaration.
   // Can't catch this with either approach.
   // b.c
   // extern void sysfunc(void);
bool isStandardFunction(const FunctionDecl *FD) {
   // d.c
   // extern void sysfunc(void); // Canonical declaration, not in system header.
   // #include <sysheader.h>
 
   return FD->getASTContext().getSourceManager().isInSystemHeader(
       FD->getCanonicalDecl()->getLocation());
 }
 
+bool isCXXOnlyStmt(const Stmt *S) {
+  StringRef Name = S->getStmtClassName();
+  if (Name.startswith("CXX"))
+    return true;
+  return isa<CoreturnStmt>(S) || isa<CoroutineBodyStmt>(S) ||
+         isa<MSDependentExistsStmt>(S) || isa<CUDAKernelCallExpr>(S) ||
+         isa<MSPropertyRefExpr>(S) || isa<MSPropertySubscriptExpr>(S) ||
+         isa<LambdaExpr>(S) || isa<TypeTraitExpr>(S) ||
+         isa<ArrayTypeTraitExpr>(S) || isa<ExpressionTraitExpr>(S) ||
+         isa<OverloadExpr>(S) || isa<DependentScopeDeclRefExpr>(S) ||
+         isa<ExprWithCleanups>(S) || isa<PackExpansionExpr>(S) ||
+         isa<SizeOfPackExpr>(S) || isa<SubstNonTypeTemplateParmExpr>(S) ||
+         isa<SubstNonTypeTemplateParmPackExpr>(S) ||
+         isa<FunctionParmPackExpr>(S) || isa<MaterializeTemporaryExpr>(S) ||
+         isa<CoroutineSuspendExpr>(S) || isa<CoawaitExpr>(S) ||
+         isa<DependentCoawaitExpr>(S) || isa<CoyieldExpr>(S) ||
+         isa<BuiltinBitCastExpr>(S) || isa<UserDefinedLiteral>(S);
+}
+
 /// Given a call graph node of a \p Caller function and a \p Callee that is
 /// called from \p Caller, get a \c CallExpr of the corresponding function call.
 /// It is unspecified which call is found if multiple calls exist, but the order
 /// should be deterministic (depend only on the AST).
 Expr *findCallExpr(const CallGraphNode *Caller, const CallGraphNode *Callee) {
   auto FoundCallee = llvm::find_if(
       Caller->callees(), [Callee](const CallGraphNode::CallRecord &Call) {
         return Call.Callee == Callee;
       });
   assert(FoundCallee != Caller->end() &&
          "Callee should be called from the caller function here.");
   return FoundCallee->CallExpr;
 }
 
+SourceRange getSourceRangeOfStmt(const Stmt *S, ASTContext &Ctx) {
+  ParentMapContext &PM = Ctx.getParentMapContext();
+  DynTypedNode P = DynTypedNode::create(*S);
+  while (P.getSourceRange().isInvalid()) {
+    DynTypedNodeList PL = PM.getParents(P);
+    if (PL.size() != 1)
+      return {};
+    P = PL[0];
+  }
+  return P.getSourceRange();
+}
+
 } // namespace
 
 AST_MATCHER(FunctionDecl, isStandardFunction) {
   return isStandardFunction(&Node);
 }
 
 SignalHandlerCheck::SignalHandlerCheck(StringRef Name,
                                        ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
       AsyncSafeFunctionSet(
           Options.get("AsyncSafeFunctionSet", AsyncSafeFunctionSetKind::POSIX)),
       ConformingFunctions(AsyncSafeFunctionSet ==
                                   AsyncSafeFunctionSetKind::Minimal
                               ? MinimalConformingFunctions
                               : POSIXConformingFunctions) {}
 
 void SignalHandlerCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "AsyncSafeFunctionSet", AsyncSafeFunctionSet);
 }
 
 bool SignalHandlerCheck::isLanguageVersionSupported(
     const LangOptions &LangOpts) const {
-  // FIXME: Make the checker useful on C++ code.
-  if (LangOpts.CPlusPlus)
-    return false;
-
-  return true;
+  return !LangOpts.CPlusPlus17;
 }
 
 void SignalHandlerCheck::registerMatchers(MatchFinder *Finder) {
   auto SignalFunction = functionDecl(hasAnyName("::signal", "::std::signal"),
                                      parameterCountIs(2), isStandardFunction());
   auto HandlerExpr =
       declRefExpr(hasDeclaration(functionDecl().bind("handler_decl")),
                   unless(isExpandedFromMacro("SIG_IGN")),
                   unless(isExpandedFromMacro("SIG_DFL")))
           .bind("handler_expr");
-  Finder->addMatcher(
-      callExpr(callee(SignalFunction), hasArgument(1, HandlerExpr))
+  auto HandlerLambda = cxxMemberCallExpr(
+      on(expr(ignoringParenImpCasts(lambdaExpr().bind("handler_lambda")))));
+  Finder->addMatcher(callExpr(callee(SignalFunction),
+                              hasArgument(1, anyOf(HandlerExpr, HandlerLambda)))
                          .bind("register_call"),
                      this);
 }
 
 void SignalHandlerCheck::check(const MatchFinder::MatchResult &Result) {
+  if (const auto *HandlerLambda =
+          Result.Nodes.getNodeAs<LambdaExpr>("handler_lambda")) {
+    diag(HandlerLambda->getBeginLoc(),
+         "lambda function is not allowed as signal handler (until C++17)")
+        << HandlerLambda->getSourceRange();
+    return;
+  }
+
   const auto *HandlerDecl =
       Result.Nodes.getNodeAs<FunctionDecl>("handler_decl");
   const auto *HandlerExpr = Result.Nodes.getNodeAs<DeclRefExpr>("handler_expr");
   assert(Result.Nodes.getNodeAs<CallExpr>("register_call") && HandlerDecl &&
          HandlerExpr && "All of these should exist in a match here.");
 
   if (CG.size() <= 1) {
     // Call graph must be populated with the entire TU at the beginning.
     // (It is possible to add a single function but the functions called from it
     // are not analysed in this case.)
     CG.addToCallGraph(const_cast<TranslationUnitDecl *>(
         HandlerDecl->getTranslationUnitDecl()));
     assert(CG.size() > 1 &&
            "There should be at least one function added to call graph.");
   }
 
   if (!HandlerDecl->hasBody()) {
-    (void)checkFunction(HandlerDecl, HandlerExpr);
-    // Here checkFunction will put the messages to HandlerExpr.
-    // No need to show a call chain.
-    // Without code body there is nothing more to check.
+    // Check the handler function.
+    // The warning is placed to the signal handler registration.
+    // No need to display a call chain and no need for more checks.
+    (void)checkFunction(HandlerDecl, HandlerExpr, {});
     return;
   }
 
+  // FIXME: Update CallGraph::getNode to use canonical decl?
   CallGraphNode *HandlerNode = CG.getNode(HandlerDecl->getCanonicalDecl());
   assert(HandlerNode &&
          "Handler with body should be present in the call graph.");
   // Start from signal handler and visit every function call.
-  for (auto Itr = llvm::df_begin(HandlerNode), ItrE = llvm::df_end(HandlerNode);
-       Itr != ItrE; ++Itr) {
-    if (const auto *CallF = dyn_cast<FunctionDecl>((*Itr)->getDecl())) {
-      unsigned int PathL = Itr.getPathLength();
-      const Expr *CallOrRef = (PathL == 1)
-                                  ? HandlerExpr
-                                  : findCallExpr(Itr.getPath(PathL - 2), *Itr);
-      if (checkFunction(CallF, CallOrRef))
-        reportHandlerChain(Itr, HandlerExpr);
+  auto Itr = llvm::df_begin(HandlerNode), ItrE = llvm::df_end(HandlerNode);
+  while (Itr != ItrE) {
+    const auto *CallF = dyn_cast<FunctionDecl>((*Itr)->getDecl());
+    if (CallF) {
+      // A signal handler or a function transitively reachable from the signal
+      // handler was found to be unsafe.
+      // Generate notes for the whole call chain (including the signal handler
+      // registration).
+      const Expr *CallOrRef = (PathL > 1)
+                                  ? findCallExpr(Itr.getPath(PathL - 2), *Itr)
+                                  : HandlerExpr;
+      auto ChainReporter = [this, &Itr, HandlerExpr](bool SkipPathEnd) {
+        reportHandlerChain(Itr, HandlerExpr, SkipPathEnd);
+      };
+      // If problems were found in a function (`CallF`), skip the analysis of
+      // functions that are called from it.
+      if (checkFunction(CallF, CallOrRef, ChainReporter))
+        Itr.skipChildren();
+      else
+        ++Itr;
+    } else {
+      ++Itr;
     }
   }
 }
 
-bool SignalHandlerCheck::checkFunction(const FunctionDecl *FD,
-                                       const Expr *CallOrRef) {
+bool SignalHandlerCheck::checkFunction(
+    const FunctionDecl *FD, const Expr *CallOrRef,
+    std::function<void(bool)> ChainReporter) {
   bool FunctionIsCalled = isa<CallExpr>(CallOrRef);
 
   if (isStandardFunction(FD)) {
     if (!isStandardFunctionAsyncSafe(FD)) {
       diag(CallOrRef->getBeginLoc(), "standard function %0 may not be "
                                      "asynchronous-safe; "
                                      "%select{using it as|calling it from}1 "
                                      "a signal handler may be dangerous")
-          << FD << FunctionIsCalled;
+          << FD << FunctionIsCalled << CallOrRef->getSourceRange();
+      if (ChainReporter)
+        ChainReporter(/*SkipPathEnd=*/true);
       return true;
     }
     return false;
   }
 
   if (!FD->hasBody()) {
     diag(CallOrRef->getBeginLoc(), "cannot verify that external function %0 is "
                                    "asynchronous-safe; "
                                    "%select{using it as|calling it from}1 "
                                    "a signal handler may be dangerous")
-        << FD << FunctionIsCalled;
+        << FD << FunctionIsCalled << CallOrRef->getSourceRange();
+    if (ChainReporter)
+      ChainReporter(/*SkipPathEnd=*/true);
     return true;
   }
 
+  if (getLangOpts().CPlusPlus)
+    return checkFunctionCPP14(FD, CallOrRef, ChainReporter);
+
   return false;
 }
 
+bool SignalHandlerCheck::checkFunctionCPP14(
+    const FunctionDecl *FD, const Expr *CallOrRef,
+    std::function<void(bool)> ChainReporter) {
+  if (!FD->isExternC()) {
+    diag(CallOrRef->getBeginLoc(),
+         "functions without C linkage are not allowed as signal "
+         "handler (until C++17)");
+    if (ChainReporter)
+      ChainReporter(/*SkipPathEnd=*/true);
+    return true;
+  }
+
+  const FunctionDecl *FBody;
+  const Stmt *BodyS = FD->getBody(FBody);
+  if (!BodyS)
+    return false;
+
+  bool StmtProblemsFound = false;
+  ASTContext &Ctx = FBody->getASTContext();
+  auto Matches =
+      match(decl(forEachDescendant(stmt().bind("stmt"))), *FBody, Ctx);
+  for (const auto &Match : Matches) {
+    const auto *FoundS = Match.getNodeAs<Stmt>("stmt");
+    if (isCXXOnlyStmt(FoundS)) {
+      SourceRange R = getSourceRangeOfStmt(FoundS, Ctx);
+      if (R.isInvalid())
+        continue;
+      diag(R.getBegin(),
+           "C++-only construct is not allowed in signal handler (until C++17)")
+          << R;
+      diag(R.getBegin(), "internally, the statement is parsed as a '%0'",
+           DiagnosticIDs::Remark)
+          << FoundS->getStmtClassName();
+      if (ChainReporter)
+        ChainReporter(/*SkipPathEnd=*/false);
+      StmtProblemsFound = true;
+    }
+  }
+
+  return StmtProblemsFound;
+}
+
 bool SignalHandlerCheck::isStandardFunctionAsyncSafe(
     const FunctionDecl *FD) const {
   assert(isStandardFunction(FD));
 
   const IdentifierInfo *II = FD->getIdentifier();
   // Unnamed functions are not explicitly allowed.
+  // C++ std operators may be unsafe and not within the
+  // "common subset of C and C++".
   if (!II)
     return false;
 
-  // FIXME: Improve for C++ (check for namespace).
+  if (!FD->isInStdNamespace() && !FD->isGlobal())
+    return false;
+
   if (ConformingFunctions.count(II->getName()))
     return true;
 
   return false;
 }
 
 void SignalHandlerCheck::reportHandlerChain(
     const llvm::df_iterator<clang::CallGraphNode *> &Itr,
-    const DeclRefExpr *HandlerRef) {
+    const DeclRefExpr *HandlerRef, bool SkipPathEnd) {
   int CallLevel = Itr.getPathLength() - 2;
   assert(CallLevel >= -1 && "Empty iterator?");
 
   const CallGraphNode *Caller = Itr.getPath(CallLevel + 1), *Callee = nullptr;
   while (CallLevel >= 0) {
     Callee = Caller;
     Caller = Itr.getPath(CallLevel);
     const Expr *CE = findCallExpr(Caller, Callee);
+    if (SkipPathEnd)
+      SkipPathEnd = false;
+    else
       diag(CE->getBeginLoc(), "function %0 called here from %1",
            DiagnosticIDs::Note)
           << cast<FunctionDecl>(Callee->getDecl())
           << cast<FunctionDecl>(Caller->getDecl());
     --CallLevel;
   }
 
+  if (!SkipPathEnd)
     diag(HandlerRef->getBeginLoc(),
          "function %0 registered here as signal handler", DiagnosticIDs::Note)
-      << cast<FunctionDecl>(Caller->getDecl()) << HandlerRef->getSourceRange();
+        << cast<FunctionDecl>(Caller->getDecl())
+        << HandlerRef->getSourceRange();
 }
 
 } // namespace bugprone
 } // namespace tidy
 } // namespace clang

clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp

CheckFactories.registerCheck<misc::ThrowByValueCatchByReferenceCheck>(
         "cert-err61-cpp");
     // MEM
     CheckFactories.registerCheck<DefaultOperatorNewAlignmentCheck>(
         "cert-mem57-cpp");
     // MSC
     CheckFactories.registerCheck<LimitedRandomnessCheck>("cert-msc50-cpp");
     CheckFactories.registerCheck<ProperlySeededRandomGeneratorCheck>(
         "cert-msc51-cpp");
+    CheckFactories.registerCheck<bugprone::SignalHandlerCheck>(
+        "cert-msc54-cpp");
     // OOP
     CheckFactories.registerCheck<performance::MoveConstructorInitCheck>(
         "cert-oop11-cpp");
     CheckFactories.registerCheck<bugprone::UnhandledSelfAssignmentCheck>(
         "cert-oop54-cpp");
     CheckFactories.registerCheck<NonTrivialTypesLibcMemoryCallsCheck>(
         "cert-oop57-cpp");
     CheckFactories.registerCheck<MutatingCopyCheck>(

clang-tools-extra/docs/ReleaseNotes.rst

 - New :doc:`modernize-macro-to-enum
   <clang-tidy/checks/modernize-macro-to-enum>` check.
 
   Replaces groups of adjacent macros with an unscoped anonymous enum.
 
 New check aliases
 ^^^^^^^^^^^^^^^^^
 
+- New alias :doc:`cert-msc54-cpp
+  <clang-tidy/checks/cert-msc54-cpp>` to
+  :doc:`bugprone-signal-handler
+  <clang-tidy/checks/bugprone-signal-handler>` was added.
+
 - New alias :doc:`cppcoreguidelines-macro-to-enum
   <clang-tidy/checks/cppcoreguidelines-macro-to-enum>` to :doc:`modernize-macro-to-enum
   <clang-tidy/checks/modernize-macro-to-enum>` was added.
 
 Changes in existing checks
 ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 - Improved :doc:`performance-inefficient-vector-operation 
   <clang-tidy/checks/performance-inefficient-vector-operation>` to work when
   the vector is a member of a structure.
 
 - Fixed a false positive in :doc:`readability-non-const-parameter
   <clang-tidy/checks/readability-non-const-parameter>` when the parameter is referenced by an lvalue.
 
 - Fixed a crash in :doc:`readability-const-return-type
   <clang-tidy/checks/readability-const-return-type>` when a pure virtual function
   overrided has a const return type. Removed the fix for a virtual function.
 
 - Fixed a false positive in :doc:`misc-redundant-expression <clang-tidy/checks/misc-redundant-expression>`
   involving overloaded comparison operators.
 
 - Fixed a crash in :doc:`bugprone-sizeof-expression <clang-tidy/checks/bugprone-sizeof-expression>` when
   `sizeof(...)` is compared agains a `__int128_t`.
   
 - Improved :doc:`cppcoreguidelines-prefer-member-initializer
   <clang-tidy/checks/cppcoreguidelines-prefer-member-initializer>` check.
 
   Fixed an issue when there was already an initializer in the constructor and
   the check would try to create another initializer for the same member.
 
 - Fixed a false positive in :doc:`misc-redundant-expression <clang-tidy/checks/misc-redundant-expression>`
   involving assignments in conditions. This fixes `Issue 35853 <https://github.com/llvm/llvm-project/issues/35853>`_.
 
+- Improved :doc:`bugprone-signal-handler
+  <clang-tidy/checks/bugprone-signal-handler>` check. Partial
+  support for C++14 signal handler rules was added. Bug report generation was
+  improved.
+
 Removed checks
 ^^^^^^^^^^^^^^
 
 Improvements to include-fixer
 -----------------------------
 
 The improvements are...
 

clang-tools-extra/docs/clang-tidy/checks/bugprone-signal-handler.rst

 .. title:: clang-tidy - bugprone-signal-handler
 
 bugprone-signal-handler
 =======================
 
-Finds functions registered as signal handlers that call non asynchronous-safe
-functions. Any function that cannot be determined to be an asynchronous-safe
+Finds functions registered as signal handlers that contain specific
+constructs that can cause undefined behavior. The rules for what is
+allowed are different for various C++ language versions.
+
+For C it is checked if non-asynchronous-safe functions are called.
+
+For C++ until (and including) C++14 such code constructs are found
+that are C++-specific and therefore not recommended in signal handlers.
+Additionally the calls to non-asynchronous-safe functions are found
+too and a signal handler or any function called from it is allowed
+to have only extern C linkage. Restrictions related to atomic
+lock-free operations are not checked.
+In C++17 there are more clarified rules but these are not
+implemented yet in this check. The check does not run on C++17 code.
+
+Any function that cannot be determined to be an asynchronous-safe
 function call is assumed to be non-asynchronous-safe by the checker,
 including user functions for which only the declaration is visible.
 User function calls with visible definition are checked recursively.
-The check handles only C code. Only the function names are considered and the
-fact that the function is a system-call, but no other restrictions on the
-arguments passed to the functions (the ``signal`` call is allowed without
+Only the function names are considered and the fact that the function
+is a system-call, but no other restrictions on the arguments passed
+to the functions (the ``signal`` call is allowed without
 restrictions).
 
-This check corresponds to the CERT C Coding Standard rule
+This check implements the CERT C Coding Standard rule
 `SIG30-C. Call only asynchronous-safe functions within signal handlers
 <https://www.securecoding.cert.org/confluence/display/c/SIG30-C.+Call+only+asynchronous-safe+functions+within+signal+handlers>`_
-and has an alias name ``cert-sig30-c``.
+and the rule
+`MSC54-CPP. A signal handler must be a plain old function
+<https://wiki.sei.cmu.edu/confluence/display/cplusplus/MSC54-CPP.+A+signal+handler+must+be+a+plain+old+function>`_.
+It has alias names ``cert-sig30-c`` and ``cert-msc54-cpp``.
 
 .. option:: AsyncSafeFunctionSet
 
   Selects which set of functions is considered as asynchronous-safe
   (and therefore allowed in signal handlers). Value ``minimal`` selects
   a minimal set that is defined in the CERT SIG30-C rule and includes functions
   ``abort()``, ``_Exit()``, ``quick_exit()`` and ``signal()``. Value ``POSIX``
   selects a larger set of functions that is listed in POSIX.1-2017 (see `this
   link
   <https://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#tag_15_04_03>`_
   for more information).
   The function ``quick_exit`` is not included in the shown list. It is
   assumable that  the reason is that the list was not updated for C11.
   The checker includes ``quick_exit`` in the set of safe functions.
   Functions registered as exit handlers are not checked.
 
   Default is ``POSIX``.

clang-tools-extra/docs/clang-tidy/checks/cert-msc54-cpp.rst

+.. title:: clang-tidy - cert-msc54-cpp
+.. meta::
+   :http-equiv=refresh: 5;URL=bugprone-signal-handler.html
+
+cert-msc54-cpp
+==============
+
+The cert-msc54-cpp check is an alias, please see
+`bugprone-signal-handler <bugprone-signal-handler.html>`_
+for more information.

clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/signal.h

 //===--- signal.h - Stub header for tests -----------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef _SIGNAL_H_
 #define _SIGNAL_H_
 
 typedef void (*sighandler_t)(int);
 
 void _sig_ign(int);
 void _sig_dfl(int);
+void _sig_err(int);
 
 #define SIGINT 1
 #define SIG_IGN _sig_ign
 #define SIG_DFL _sig_dfl
+#define SIG_ERR _sig_err
 
 sighandler_t signal(int, sighandler_t);
 
 #endif // _SIGNAL_H_

clang-tools-extra/test/clang-tidy/checkers/Inputs/bugprone-signal-handler/stdcpp.h

-//===--- signal.h - Stub header for tests -----------------------*- C++ -*-===//
+//===--- Header for test bugprone-signal-handler.cpp ------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef _SIGNAL_H_
-#define _SIGNAL_H_
+#define SIGINT 1
+#define SIG_IGN std::_sig_ign
+#define SIG_DFL std::_sig_dfl
 
-typedef void (*sighandler_t)(int);
+namespace std {
 
 void _sig_ign(int);
 void _sig_dfl(int);
 
-#define SIGINT 1
-#define SIG_IGN _sig_ign
-#define SIG_DFL _sig_dfl
+typedef void (*sighandler_t)(int);
+sighandler_t signal(int, sighandler_t);
+
+void abort();
+void _Exit(int);
+void quick_exit(int);
 
+void other_call();
+
+struct SysStruct {
+  void operator<<(int);
+};
+
+} // namespace std
+
+namespace system_other {
+
+typedef void (*sighandler_t)(int);
 sighandler_t signal(int, sighandler_t);
 
-#endif // _SIGNAL_H_
+} // namespace system_other

clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-handler.c

 typedef void (*sighandler_t)(int);
 sighandler_t signal(int signum, sighandler_t handler);
 
 void f_extern(void);
 
 void handler_printf(int) {
   printf("1234");
   // CHECK-NOTES: :[[@LINE-1]]:3: warning: standard function 'printf' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
-  // CHECK-NOTES: :[[@LINE-2]]:3: note: function 'printf' called here from 'handler_printf'
   // CHECK-NOTES: :[[@LINE+4]]:18: note: function 'handler_printf' registered here as signal handler
 }
 
 void test_printf(void) {
   signal(SIGINT, handler_printf);
 }
 
 void handler_extern(int) {
   f_extern();
   // CHECK-NOTES: :[[@LINE-1]]:3: warning: cannot verify that external function 'f_extern' is asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
-  // CHECK-NOTES: :[[@LINE-2]]:3: note: function 'f_extern' called here from 'handler_extern'
   // CHECK-NOTES: :[[@LINE+4]]:18: note: function 'handler_extern' registered here as signal handler
 }
 
 void test_extern(void) {
   signal(SIGINT, handler_extern);
 }
 
 void f_ok(void) {
   abort();
 }
 
 void handler_ok(int) {
   f_ok();
 }
 
 void test_ok(void) {
   signal(SIGINT, handler_ok);
 }
 
 void f_bad(void) {
   printf("1234");
   // CHECK-NOTES: :[[@LINE-1]]:3: warning: standard function 'printf' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
-  // CHECK-NOTES: :[[@LINE-2]]:3: note: function 'printf' called here from 'f_bad'
   // CHECK-NOTES: :[[@LINE+5]]:3: note: function 'f_bad' called here from 'handler_bad'
   // CHECK-NOTES: :[[@LINE+8]]:18: note: function 'handler_bad' registered here as signal handler
 }
 
 void handler_bad(int) {
   f_bad();
 }
 
 void test_bad(void) {
   signal(SIGINT, handler_bad);
 }
 
 void f_bad1(void) {
   printf("1234");
   // CHECK-NOTES: :[[@LINE-1]]:3: warning: standard function 'printf' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
-  // CHECK-NOTES: :[[@LINE-2]]:3: note: function 'printf' called here from 'f_bad1'
   // CHECK-NOTES: :[[@LINE+6]]:3: note: function 'f_bad1' called here from 'f_bad2'
   // CHECK-NOTES: :[[@LINE+9]]:3: note: function 'f_bad2' called here from 'handler_bad1'
   // CHECK-NOTES: :[[@LINE+13]]:18: note: function 'handler_bad1' registered here as signal handler
 }
 
 void f_bad2(void) {
   f_bad1();
 }
void handler_signal(int) {
   // FIXME: It is only OK to call signal with the current signal number.
   signal(0, SIG_DFL);
 }
 
 void handler_false_condition(int) {
   if (0)
     printf("1234");
   // CHECK-NOTES: :[[@LINE-1]]:5: warning: standard function 'printf' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
-  // CHECK-NOTES: :[[@LINE-2]]:5: note: function 'printf' called here from 'handler_false_condition'
   // CHECK-NOTES: :[[@LINE+4]]:18: note: function 'handler_false_condition' registered here as signal handler
 }
 
 void test_false_condition(void) {
   signal(SIGINT, handler_false_condition);
 }
 
 void handler_multiple_calls(int) {
   f_extern();
   // CHECK-NOTES: :[[@LINE-1]]:3: warning: cannot verify that external function 'f_extern' is asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
-  // CHECK-NOTES: :[[@LINE-2]]:3: note: function 'f_extern' called here from 'handler_multiple_calls'
-  // CHECK-NOTES: :[[@LINE+10]]:18: note: function 'handler_multiple_calls' registered here as signal handler
+  // CHECK-NOTES: :[[@LINE+9]]:18: note: function 'handler_multiple_calls' registered here as signal handler
   printf("1234");
   // CHECK-NOTES: :[[@LINE-1]]:3: warning: standard function 'printf' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
-  // CHECK-NOTES: :[[@LINE-2]]:3: note: function 'printf' called here from 'handler_multiple_calls'
   // CHECK-NOTES: :[[@LINE+6]]:18: note: function 'handler_multiple_calls' registered here as signal handler
   f_extern();
   // first 'f_extern' call found only
 }
 
 void test_multiple_calls(void) {
   signal(SIGINT, handler_multiple_calls);
 }
 
 void f_recursive(void);
 
 void handler_recursive(int) {
   f_recursive();
   printf("");
   // first 'printf' call (in f_recursive) found only
 }
 
 void f_recursive(void) {
   f_extern();
   // CHECK-NOTES: :[[@LINE-1]]:3: warning: cannot verify that external function 'f_extern' is asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
-  // CHECK-NOTES: :[[@LINE-2]]:3: note: function 'f_extern' called here from 'f_recursive'
-  // CHECK-NOTES: :[[@LINE-9]]:3: note: function 'f_recursive' called here from 'handler_recursive'
-  // CHECK-NOTES: :[[@LINE+10]]:18: note: function 'handler_recursive' registered here as signal handler
+  // CHECK-NOTES: :[[@LINE-8]]:3: note: function 'f_recursive' called here from 'handler_recursive'
+  // CHECK-NOTES: :[[@LINE+9]]:18: note: function 'handler_recursive' registered here as signal handler
   printf("");
   // CHECK-NOTES: :[[@LINE-1]]:3: warning: standard function 'printf' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
-  // CHECK-NOTES: :[[@LINE-2]]:3: note: function 'printf' called here from 'f_recursive'
-  // CHECK-NOTES: :[[@LINE-14]]:3: note: function 'f_recursive' called here from 'handler_recursive'
+  // CHECK-NOTES: :[[@LINE-12]]:3: note: function 'f_recursive' called here from 'handler_recursive'
   // CHECK-NOTES: :[[@LINE+5]]:18: note: function 'handler_recursive' registered here as signal handler
   handler_recursive(2);
 }
 
 void test_recursive(void) {
   signal(SIGINT, handler_recursive);
 }
 
 void f_multiple_paths(void) {
   printf("");
   // CHECK-NOTES: :[[@LINE-1]]:3: warning: standard function 'printf' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
-  // CHECK-NOTES: :[[@LINE-2]]:3: note: function 'printf' called here from 'f_multiple_paths'
   // CHECK-NOTES: :[[@LINE+5]]:3: note: function 'f_multiple_paths' called here from 'handler_multiple_paths'
   // CHECK-NOTES: :[[@LINE+9]]:18: note: function 'handler_multiple_paths' registered here as signal handler
 }
 
 void handler_multiple_paths(int) {
   f_multiple_paths();
   f_multiple_paths();
 }
 
 void test_multiple_paths(void) {
   signal(SIGINT, handler_multiple_paths);
 }
 
 void handler_function_pointer(int) {
   void (*fp)(void) = f_extern;
   // Call with function pointer is not evalauted by the check.
   (*fp)();
 }
 
 void test_function_pointer(void) {
   signal(SIGINT, handler_function_pointer);
 }
 
 void test_other(void) {
   signal(SIGINT, handler_abort);
   signal(SIGINT, handler_signal);
 
   signal(SIGINT, _Exit);
   signal(SIGINT, other_call);
   // CHECK-NOTES: :[[@LINE-1]]:18: warning: standard function 'other_call' may not be asynchronous-safe; using it as a signal handler may be dangerous [bugprone-signal-handler]
   signal(SIGINT, f_extern);
   // CHECK-NOTES: :[[@LINE-1]]:18: warning: cannot verify that external function 'f_extern' is asynchronous-safe; using it as a signal handler may be dangerous [bugprone-signal-handler]
 
   signal(SIGINT, SIG_IGN);
   signal(SIGINT, SIG_DFL);
 }

clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-handler.cpp

+// RUN: %check_clang_tidy -std=c++14 %s bugprone-signal-handler %t -- -- -isystem %S/Inputs/Headers -isystem %S/Inputs/bugprone-signal-handler
+
+#include "stdcpp.h"
+#include "stdio.h"
+
+// Functions called "signal" that are different from the system version.
+typedef void (*callback_t)(int);
+void signal(int, callback_t, int);
+namespace ns {
+void signal(int, callback_t);
+}
+
+extern "C" void handler_unsafe(int) {
+  printf("xxx");
+}
+
+extern "C" void handler_unsafe_1(int) {
+  printf("xxx");
+}
+
+namespace test_invalid_handler {
+
+void handler_non_extern_c(int) {
+  printf("xxx");
+}
+
+struct A {
+  static void handler_member(int) {
+    printf("xxx");
+  }
+};
+
+void test() {
+  std::signal(SIGINT, handler_unsafe_1);
+  // CHECK-MESSAGES: :[[@LINE-17]]:3: warning: standard function 'printf' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
+  // CHECK-MESSAGES: :[[@LINE-2]]:23: note: function 'handler_unsafe_1' registered here as signal handler
+
+  std::signal(SIGINT, handler_non_extern_c);
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: functions without C linkage are not allowed as signal handler (until C++17) [bugprone-signal-handler]
+  std::signal(SIGINT, A::handler_member);
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: functions without C linkage are not allowed as signal handler (until C++17) [bugprone-signal-handler]
+  std::signal(SIGINT, [](int) { printf("xxx"); });
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: lambda function is not allowed as signal handler (until C++17) [bugprone-signal-handler]
+
+  // This case is (deliberately) not found by the checker.
+  std::signal(SIGINT, [](int) -> callback_t { return &handler_unsafe; }(1));
+}
+
+} // namespace test_invalid_handler
+
+namespace test_non_standard_signal_call {
+
+struct Signal {
+  static void signal(int, callback_t);
+};
+
+void test() {
+  // No diagnostics here. All these signal calls differ from the standard system one.
+  signal(SIGINT, handler_unsafe, 1);
+  ns::signal(SIGINT, handler_unsafe);
+  Signal::signal(SIGINT, handler_unsafe);
+  system_other::signal(SIGINT, handler_unsafe);
+}
+
+} // namespace test_non_standard_signal_call
+
+namespace test_cpp_construct_in_handler {
+
+struct Struct {
+  virtual ~Struct() {}
+  void f1();
+  int *begin();
+  int *end();
+  static void f2();
+};
+struct Derived : public Struct {
+};
+
+struct X {
+  X(int, float);
+};
+
+Struct *S_Global;
+const Struct *S_GlobalConst;
+
+void f_non_extern_c() {
+}
+
+void f_default_arg(int P1 = 0) {
+}
+
+extern "C" void handler_cpp(int) {
+  using namespace ::test_cpp_construct_in_handler;
+
+  // These calls are not found as problems.
+  // (Called functions are not analyzed if the current function has already
+  // other problems.)
+  f_non_extern_c();
+  Struct::f2();
+  // 'auto' is not disallowed
+  auto Auto = 28u;
+
+  Struct S;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler]
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: remark: internally, the statement is parsed as a 'CXXConstructExpr'
+  // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler
+  S_Global->f1();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler]
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: remark: internally, the statement is parsed as a 'CXXMemberCallExpr'
+  // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler
+  const Struct &SRef = Struct();
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler]
+  // CHECK-MESSAGES: :[[@LINE-2]]:24: remark: internally, the statement is parsed as a 'CXXBindTemporaryExpr'
+  // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler
+  X(3, 4.4);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler]
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: remark: internally, the statement is parsed as a 'CXXTemporaryObjectExpr'
+  // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler
+
+  auto L = [](int i) { printf("%d", i); };
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler]
+  // CHECK-MESSAGES: :[[@LINE-2]]:12: remark: internally, the statement is parsed as a 'CXXConstructExpr'
+  // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler
+  L(2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler]
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: remark: internally, the statement is parsed as a 'CXXOperatorCallExpr'
+  // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler
+
+  try {
+    // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler]
+    // CHECK-MESSAGES: :[[@LINE-2]]:3: remark: internally, the statement is parsed as a 'CXXTryStmt'
+    // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler
+    int A;
+  } catch (int) {
+  };
+  // CHECK-MESSAGES: :[[@LINE-2]]:5: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler]
+  // CHECK-MESSAGES: :[[@LINE-3]]:5: remark: internally, the statement is parsed as a 'CXXCatchStmt'
+  // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler
+
+  throw(12);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler]
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: remark: internally, the statement is parsed as a 'CXXThrowExpr'
+  // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler
+
+  for (int I : S) {
+  }
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler]
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: remark: internally, the statement is parsed as a 'CXXForRangeStmt'
+  // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler
+  // CHECK-MESSAGES: :[[@LINE-5]]:14: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler]
+  // CHECK-MESSAGES: :[[@LINE-6]]:14: remark: internally, the statement is parsed as a 'CXXMemberCallExpr'
+  // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler
+
+  int Int = *(reinterpret_cast<int *>(&S));
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler]
+  // CHECK-MESSAGES: :[[@LINE-2]]:15: remark: internally, the statement is parsed as a 'CXXReinterpretCastExpr'
+  // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler
+  Int = static_cast<int>(12.34);
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler]
+  // CHECK-MESSAGES: :[[@LINE-2]]:9: remark: internally, the statement is parsed as a 'CXXStaticCastExpr'
+  // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler
+  Derived *Der = dynamic_cast<Derived *>(S_Global);
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler]
+  // CHECK-MESSAGES: :[[@LINE-2]]:18: remark: internally, the statement is parsed as a 'CXXDynamicCastExpr'
+  // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler
+  Struct *SPtr = const_cast<Struct *>(S_GlobalConst);
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler]
+  // CHECK-MESSAGES: :[[@LINE-2]]:18: remark: internally, the statement is parsed as a 'CXXConstCastExpr'
+  // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler
+  Int = int(12.34);
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler]
+  // CHECK-MESSAGES: :[[@LINE-2]]:9: remark: internally, the statement is parsed as a 'CXXFunctionalCastExpr'
+  // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler
+
+  int *IPtr = new int[10];
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler]
+  // CHECK-MESSAGES: :[[@LINE-2]]:15: remark: internally, the statement is parsed as a 'CXXNewExpr'
+  // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler
+  delete[] IPtr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler]
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: remark: internally, the statement is parsed as a 'CXXDeleteExpr'
+  // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler
+  IPtr = nullptr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler]
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: remark: internally, the statement is parsed as a 'CXXNullPtrLiteralExpr'
+  // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler
+  bool Bool = true;
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler]
+  // CHECK-MESSAGES: :[[@LINE-2]]:15: remark: internally, the statement is parsed as a 'CXXBoolLiteralExpr'
+  // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler
+  f_default_arg();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler]
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: remark: internally, the statement is parsed as a 'CXXDefaultArgExpr'
+  // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler
+}
+
+void test() {
+  std::signal(SIGINT, handler_cpp);
+}
+
+} // namespace test_cpp_construct_in_handler
+
+namespace test_cpp_indirect {
+
+void non_extern_c() {
+  int *P = nullptr;
+}
+
+extern "C" void call_cpp_indirect() {
+  int *P = nullptr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler]
+  // CHECK-MESSAGES: :[[@LINE-2]]:12: remark: internally, the statement is parsed as a 'CXXNullPtrLiteralExpr'
+  // CHECK-MESSAGES: :[[@LINE+8]]:3: note: function 'call_cpp_indirect' called here from 'handler_cpp_indirect'
+  // CHECK-MESSAGES: :[[@LINE+11]]:23: note: function 'handler_cpp_indirect' registered here as signal handler
+}
+
+extern "C" void handler_cpp_indirect(int) {
+  non_extern_c();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: functions without C linkage are not allowed as signal handler (until C++17) [bugprone-signal-handler]
+  // CHECK-MESSAGES: :[[@LINE+5]]:23: note: function 'handler_cpp_indirect' registered here as signal handler
+  call_cpp_indirect();
+}
+
+void test() {
+  std::signal(SIGINT, handler_cpp_indirect);
+}
+
+} // namespace test_cpp_indirect