[clang-tidy] bugprone-signal-handler improvements: display call chain

[balazske]

Summary

Add a new feature to display the call chain if an unsafe function is
found in a signal handler or function called from it.
Warning messages are improved too.

[njames93]

This needs tests demonstrating the notes. The defacto way is to replace all CHECK-MESSAGESDirectives with CHECK-NOTES. This'll for file check to look for lines containing note.

[balazske]

I tried to test the notes but could not get it to work. I think the problem is related to the fact that the notes are displayed at different locations that are past or before the current warning. The solution could be to manually specify line numbers in the CHECK-NOTE parts but I do not like this (and did not tried it). Or is there other solution?

[njames93]

The issue is FileCheck expects its directives to be in order they appear in the file being checked. Notes are always emitted just after the warning they are attached to.
So there are a few ways to go:

• use CHECK-NOTES-DAG to disregard the ordering and place them next to the location of the note in the code, to give correct line information.
• place the CHECK-NOTES directly after the warning they are attached.

Going with the first option makes it much harder to reason about.
Going with the second option you'll either have to just put a generic number regex for the line number, use (potentially large) relative @LINE offsets or absolute line numbers.

[balazske]

Adding test of notes.
Tests are re-arranged significantly.

[Eugene.Zelenko]

I think call stack may be useful for other checks too. May be code should be moved to utilities?

[balazske]

In D97960#2611531, @Eugene.Zelenko wrote:

I think call stack may be useful for other checks too. May be code should be moved to utilities?

Yes it is useful for other checks, but the code here is specialized for this problem only. It is possible to make a call graph that stores the "parents" of the calls, probably root items that are of a special kind, and can add notes for call chain. But there is already a clang::CallGraph that can compute call graph, if I remember correctly it was not usable here because the parent node (caller) is not available. That CallGraph could be extended too.

[martong]

So, about the tests, gentle ping @njames93

[balazske]

A new implementation is in D118224.

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

 //===--- SignalHandlerCheck.h - clang-tidy ----------------------*- 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 LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SIGNALHANDLERCHECK_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SIGNALHANDLERCHECK_H
 
 #include "../ClangTidyCheck.h"
+#include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/StringSet.h"
 
 namespace clang {
 namespace tidy {
 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 AsyncSafeFunctionSetType { 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:
-  void reportBug(const FunctionDecl *CalledFunction, const Expr *CallOrRef,
-                 const CallExpr *SignalCall, const FunctionDecl *HandlerDecl);
+  using ParentMap =
+      llvm::DenseMap<const FunctionDecl *,
+                     std::pair<const Expr *, const FunctionDecl *>>;
+
+  bool checkCallAllowed(const FunctionDecl *F, const Expr *CallOrRef,
+                        const ParentMap &ParentOfCall);
+  void addCallStackNotes(const FunctionDecl *CheckedFunction,
+                         const ParentMap &ParentOfCall);
   void reportNonExternCBug(const Expr *HandlerRef,
                            const FunctionDecl *HandlerDecl);
   void reportLambdaBug(const Expr *HandlerRef);
 
   bool isSystemCallAllowed(const FunctionDecl *FD) const;
 
   AsyncSafeFunctionSetType AsyncSafeFunctionSet;
   llvm::StringSet<> &ConformingFunctions;

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
 //
 //===----------------------------------------------------------------------===//
 

 
 void SignalHandlerCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *LambdaHandler = Result.Nodes.getNodeAs<LambdaExpr>("lambda");
   if (LambdaHandler) {
     reportLambdaBug(LambdaHandler);
     return;
   }
 
-  const auto *SignalCall = Result.Nodes.getNodeAs<CallExpr>("register_call");
   const auto *HandlerDecl =
-      Result.Nodes.getNodeAs<FunctionDecl>("handler_decl");
+      Result.Nodes.getNodeAs<FunctionDecl>("handler_decl")->getCanonicalDecl();
   const auto *HandlerExpr = Result.Nodes.getNodeAs<DeclRefExpr>("handler_expr");
+  assert(HandlerDecl && HandlerExpr &&
+         "Handler function and reference to it should both be found.");
 
   if (!HandlerDecl->isExternC()) {
     reportNonExternCBug(HandlerExpr, HandlerDecl);
     return;
   }
 
-  // Visit each function encountered in the callgraph only once.
-  llvm::DenseSet<const FunctionDecl *> SeenFunctions;
-
   // The worklist of the callgraph visitation algorithm.
-  std::deque<const CallExpr *> CalledFunctions;
-
-  auto ProcessFunction = [&](const FunctionDecl *F, const Expr *CallOrRef) {
-    // Ensure that canonical declaration is used.
-    F = F->getCanonicalDecl();
-
-    // Do not visit function if already encountered.
-    if (!SeenFunctions.insert(F).second)
-      return true;
-
-    // Check if the call is allowed.
-    // Non-system calls are not considered.
-    if (isSystemCall(F)) {
-      if (isSystemCallAllowed(F))
-        return true;
+  std::deque<const Expr *> CallsToCheck;
+  // Visit each function encountered in the callgraph only once.
+  // Store its caller expression and the function where it appears.
+  ParentMap ParentOfCall;
 
-      reportBug(F, CallOrRef, SignalCall, HandlerDecl);
+  // For the real signal handler we have no parent call but a reference to it
+  // (in the call to 'signal').
+  ParentOfCall[HandlerDecl] = std::make_pair(HandlerExpr, nullptr);
+  CallsToCheck.push_back(HandlerExpr);
+
+  while (!CallsToCheck.empty()) {
+    const Expr *CallOrRef = CallsToCheck.front();
+    CallsToCheck.pop_front();
+    const FunctionDecl *F =
+        (CallOrRef == HandlerExpr)
+            ? HandlerDecl
+            : cast<FunctionDecl>(cast<CallExpr>(CallOrRef)->getCalleeDecl())
+                  ->getCanonicalDecl();
 
-      return false;
-    }
+    if (!checkCallAllowed(F, CallOrRef, ParentOfCall))
+      continue;
 
-    // Get the body of the encountered non-system call function.
     const FunctionDecl *FBody;
-    if (!F->hasBody(FBody)) {
-      reportBug(F, CallOrRef, SignalCall, HandlerDecl);
-      return false;
-    }
+    if (!F->hasBody(FBody))
+      continue;
 
     // Collect all called functions.
     auto Matches = match(decl(forEachDescendant(callExpr().bind("call"))),
                          *FBody, FBody->getASTContext());
     for (const auto &Match : Matches) {
       const auto *CE = Match.getNodeAs<CallExpr>("call");
-      if (isa<FunctionDecl>(CE->getCalleeDecl()))
-        CalledFunctions.push_back(CE);
+      if (const auto *CalleeF = dyn_cast<FunctionDecl>(CE->getCalleeDecl())) {
+        CalleeF = CalleeF->getCanonicalDecl();
+        if (!ParentOfCall.count(CalleeF)) {
+          ParentOfCall[CalleeF] = std::make_pair(CE, F);
+          CallsToCheck.push_back(CE);
+        }
+      }
     }
-
-    return true;
-  };
-
-  if (!ProcessFunction(HandlerDecl, HandlerExpr))
-    return;
-
-  // Visit the definition of every function referenced by the handler function.
-  // Check for allowed function calls.
-  while (!CalledFunctions.empty()) {
-    const CallExpr *FunctionCall = CalledFunctions.front();
-    CalledFunctions.pop_front();
-    // At insertion we have already ensured that only function calls are there.
-    const auto *F = cast<FunctionDecl>(FunctionCall->getCalleeDecl());
-
-    if (!ProcessFunction(F, FunctionCall))
-      break;
   }
 }
 
-bool SignalHandlerCheck::isSystemCallAllowed(const FunctionDecl *FD) const {
-  const IdentifierInfo *II = FD->getIdentifier();
-  // Can not verify if std operators are safe to call.
-  if (!II)
-    return false;
-
-  if (!FD->isInStdNamespace() && !isInNoNamespace(FD))
-    return false;
-
-  if (ConformingFunctions.count(II->getName()))
+bool SignalHandlerCheck::checkCallAllowed(const FunctionDecl *F,
+                                          const Expr *CallOrRef,
+                                          const ParentMap &ParentOfCall) {
+  StringRef FunctionKindStr;
+  if (isSystemCall(F)) {
+    if (isSystemCallAllowed(F))
+      return true;
+    FunctionKindStr = "system call";
+  } else {
+    if (F->hasBody())
       return true;
+    FunctionKindStr = "function";
+  }
+
+  if (isa<DeclRefExpr>(CallOrRef)) {
+    // Function F appears directly in 'signal' call.
+    diag(CallOrRef->getBeginLoc(), "%0 %1 may not be asynchronous-safe; using "
+                                   "it as signal handler may be dangerous")
+        << FunctionKindStr << F;
+  } else {
+    // Function F is called from another function.
+    diag(CallOrRef->getBeginLoc(),
+         "%0 %1 may not be asynchronous-safe; calling it from a signal handler "
+         "may be dangerous")
+        << FunctionKindStr << F;
+    addCallStackNotes(F, ParentOfCall);
+  }
 
   return false;
 }
 
-void SignalHandlerCheck::reportBug(const FunctionDecl *CalledFunction,
-                                   const Expr *CallOrRef,
-                                   const CallExpr *SignalCall,
-                                   const FunctionDecl *HandlerDecl) {
-  diag(CallOrRef->getBeginLoc(),
-       "%0 may not be asynchronous-safe; "
-       "calling it from a signal handler may be dangerous")
-      << CalledFunction;
-  diag(SignalCall->getSourceRange().getBegin(),
-       "signal handler registered here", DiagnosticIDs::Note);
-  diag(HandlerDecl->getBeginLoc(), "handler function declared here",
-       DiagnosticIDs::Note);
+void SignalHandlerCheck::addCallStackNotes(const FunctionDecl *F,
+                                           const ParentMap &ParentOfCall) {
+  auto I = ParentOfCall.find(F);
+  while (I != ParentOfCall.end()) {
+    auto Caller = I->second;
+    if (const auto *HandlerRef = dyn_cast<DeclRefExpr>(Caller.first)) {
+      diag(HandlerRef->getBeginLoc(),
+           "function %0 registered here as signal handler", DiagnosticIDs::Note)
+          << F;
+      break;
+    } else {
+      const auto *Call = cast<CallExpr>(Caller.first);
+      diag(Call->getBeginLoc(), "function %0 called here from %1",
+           DiagnosticIDs::Note)
+          << F << Caller.second;
+    }
+    F = Caller.second;
+    I = ParentOfCall.find(F);
+  }
 }
 
 void SignalHandlerCheck::reportNonExternCBug(const Expr *HandlerRef,
                                              const FunctionDecl *HandlerDecl) {
   diag(HandlerRef->getBeginLoc(),
        "function %0 should have C language linkage if used as signal handler")
       << HandlerDecl;
   diag(HandlerDecl->getBeginLoc(), "handler function declared here",
        DiagnosticIDs::Note);
 }
 
 void SignalHandlerCheck::reportLambdaBug(const Expr *HandlerRef) {
   diag(HandlerRef->getBeginLoc(),
        "lambda expression is not allowed as signal handler");
 }
 
+bool SignalHandlerCheck::isSystemCallAllowed(const FunctionDecl *FD) const {
+  const IdentifierInfo *II = FD->getIdentifier();
+  // Can not verify if std operators are safe to call.
+  if (!II)
+    return false;
+
+  if (!FD->isInStdNamespace() && !isInNoNamespace(FD))
+    return false;
+
+  if (ConformingFunctions.count(II->getName()))
+    return true;
+
+  return false;
+}
+
 // 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
 llvm::StringSet<> SignalHandlerCheck::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

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

 // RUN: %check_clang_tidy %s bugprone-signal-handler %t \
 // RUN: -config='{CheckOptions: \
 // RUN:  [{key: bugprone-signal-handler.AsyncSafeFunctionSet, value: "minimal"}]}' \
 // RUN: -- -isystem %S/Inputs/Headers
 
 #include "signal.h"
 #include "stdlib.h"
 #include "string.h"
 #include "unistd.h"
 
 void handler_bad1(int) {
   _exit(0);
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: '_exit' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: system call '_exit' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
 }
 
 void handler_bad2(void *dst, const void *src) {
   memcpy(dst, src, 10);
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memcpy' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: system call 'memcpy' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
 }
 
 void handler_good(int) {
   abort();
   _Exit(0);
   quick_exit(0);
   signal(0, SIG_DFL);
 }
 
 void test() {
   signal(SIGINT, handler_bad1);
   signal(SIGINT, handler_bad2);
   signal(SIGINT, handler_good);
 }

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

 // RUN: %check_clang_tidy %s bugprone-signal-handler %t \
 // RUN: -config='{CheckOptions: \
 // RUN:  [{key: bugprone-signal-handler.AsyncSafeFunctionSet, value: "POSIX"}]}' \
 // RUN: -- -isystem %S/Inputs/Headers
 
 #include "signal.h"
 #include "stdlib.h"
 #include "stdio.h"
 #include "string.h"
 #include "unistd.h"
 
 void handler_bad(int) {
   printf("1234");
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'printf' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: system call 'printf' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
 }
 
 void handler_good(int) {
   abort();
   _Exit(0);
   _exit(0);
   quick_exit(0);
   signal(0, SIG_DFL);
   memcpy((void*)10, (const void*)20, 1);
 }
 
 void test() {
   signal(SIGINT, handler_good);
   signal(SIGINT, handler_bad);
 }

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

 // RUN: %check_clang_tidy %s bugprone-signal-handler %t -- -- -isystem %S/Inputs/Headers
 
 #include "signal.h"
 #include "stdlib.h"
 #include "stdio.h"
 #include "system-other.h"
 
 // The function should be classified as system call even if there is
 // declaration the in source file.
 // FIXME: The detection works only if the first declaration is in system
 // header.
 int printf(const char *, ...);
 typedef void (*sighandler_t)(int);
 sighandler_t signal(int signum, sighandler_t handler);
 
-void handler_abort(int) {
-  abort();
-}
+void f_extern();
 
-void handler_other(int) {
+
+
+void handler_printf(int) {
   printf("1234");
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'printf' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
+  // CHECK-NOTES: :[[@LINE-1]]:3: warning: system call '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 handler_signal(int) {
-  // FIXME: It is only OK to call signal with the current signal number.
-  signal(0, SIG_DFL);
+void test_printf() {
+  signal(SIGINT, handler_printf);
 }
 
-void f_ok() {
-  abort();
+
+
+void handler_extern(int) {
+  f_extern();
+  // CHECK-NOTES: :[[@LINE-1]]:3: warning: function 'f_extern' may not be 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 f_bad() {
-  printf("1234");
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'printf' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
+void test_extern() {
+  signal(SIGINT, handler_extern);
 }
 
-void f_extern();
+
+
+void f_ok() {
+  abort();
+}
 
 void handler_ok(int) {
   f_ok();
 }
 
+void test_ok() {
+  signal(SIGINT, handler_ok);
+}
+
+
+
+void f_bad() {
+  printf("1234");
+  // CHECK-NOTES: :[[@LINE-1]]:3: warning: system call '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 handler_extern(int) {
-  f_extern();
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'f_extern' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
+void test_bad() {
+  signal(SIGINT, handler_bad);
+}
+
+
+
+void f_bad1() {
+  printf("1234");
+  // CHECK-NOTES: :[[@LINE-1]]:3: warning: system call '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+10]]:3: note: function 'f_bad1' called here from 'handler_bad1'
+  // CHECK-NOTES: :[[@LINE+13]]:18: note: function 'handler_bad1' registered here as signal handler
+}
+
+void f_bad2() {
+  f_bad1();
+}
+
+void handler_bad1(int) {
+  f_bad2();
+  f_bad1();
+}
+
+void test_bad1() {
+  signal(SIGINT, handler_bad1);
+}
+
+
+
+void handler_abort(int) {
+  abort();
+}
+
+void handler_signal(int) {
+  // FIXME: It is only OK to call signal with the current signal number.
+  signal(0, SIG_DFL);
 }
 
 void test() {
   signal(SIGINT, handler_abort);
   signal(SIGINT, handler_signal);
-  signal(SIGINT, handler_other);
-
-  signal(SIGINT, handler_ok);
-  signal(SIGINT, handler_bad);
-  signal(SIGINT, handler_extern);
 
   signal(SIGINT, _Exit);
   signal(SIGINT, other_call);
-  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'other_call' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
+  // CHECK-NOTES: :[[@LINE-1]]:18: warning: system call 'other_call' may not be asynchronous-safe; using it as signal handler may be dangerous [bugprone-signal-handler]
+  signal(SIGINT, f_extern);
+  // CHECK-NOTES: :[[@LINE-1]]:18: warning: function 'f_extern' may not be asynchronous-safe; using it as 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"
 
 // Function called "signal" that is not to be recognized by the checker.
 typedef void (*callback_t)(int);
 void signal(int, callback_t, int);
 
 namespace ns {
 void signal(int, callback_t);
 }
 
-struct S {
+struct Signal {
   static void signal(int, callback_t);
-  static void handler(int);
-
-  S() = default;
-  S(int) {
-    std::other_call();
-    // Should be fixed.
-    // CHECK-MESSAGES-NOT: :[[@LINE-2]]:5: warning: 'other_call' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
-  };
+
+
+
+struct S_operator {
   int operator-() const {
     std::other_call();
-    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'other_call' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
-  }
+    // CHECK-NOTES: :[[@LINE-1]]:5: warning: system call 'other_call' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
+    // CHECK-NOTES: :[[@LINE-2]]:5: note: function 'other_call' called here from 'operator-'
+    // CHECK-NOTES: :[[@LINE+7]]:3: note: function 'operator-' called here from 'handler_operator'
+    // CHECK-NOTES: :[[@LINE+10]]:23: note: function 'handler_operator' registered here as signal handler
+  }
+};
+
+extern "C" void handler_operator(int) {
+  S_operator S;
+  -S;
+}
+
+void test_operator() {
+  std::signal(SIGINT, handler_operator);
+
+
+
+struct S_constructor {
+  S_constructor() = default;
+  S_constructor(int) {
+    std::other_call();
+    // Should be fixed.
+    // CHECK-NOTES-NOT: :[[@LINE-2]]:5: warning: system call 'other_call' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
   };
+};
+
+extern "C" void handler_constructor(int) {
+  S_constructor S(0);
+}
+
+void test_constructor() {
+  std::signal(SIGINT, handler_constructor);
+}
 
-struct TestMemberInit {
+
+
+struct S_MemberInit {
   static int memberInit() {
     std::other_call();
     // Should be fixed.
-    // CHECK-MESSAGES-NOT: :[[@LINE-2]]:5: warning: 'other_call' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
+    // CHECK-NOTES-NOT: :[[@LINE-2]]:5: warning: system call 'other_call' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
   }
   int M = memberInit();
 };
 
+extern "C" void handler_MemberInit(int) {
+  S_MemberInit S;
+}
+
+void test_MemberInit() {
+  std::signal(SIGINT, handler_MemberInit);
+}
+
+
+
 int defaultInit() {
   std::other_call();
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'other_call' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
+  // CHECK-NOTES: :[[@LINE-1]]:3: warning: system call 'other_call' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
+  // CHECK-NOTES: :[[@LINE-2]]:3: note: function 'other_call' called here from 'defaultInit'
+  // CHECK-NOTES: :[[@LINE+4]]:30: note: function 'defaultInit' called here from 'handler_DefaultInit'
+  // CHECK-NOTES: :[[@LINE+11]]:23: note: function 'handler_DefaultInit' registered here as signal handler
 }
 
-void testDefaultInit(int I = defaultInit()) {
+void callDefaultInit(int I = defaultInit()) {
 }
 
-void S::handler(int) {
+extern "C" void handler_DefaultInit(int) {
+  callDefaultInit();
+}
+
+void test_DefaultInit() {
+  std::signal(SIGINT, handler_DefaultInit);
+}
+
+
+
+extern "C" void handler_UnsafeFunction(int) {
   std::other_call();
+  // CHECK-NOTES: :[[@LINE-1]]:3: warning: system call 'other_call' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
+  // CHECK-NOTES: :[[@LINE-2]]:3: note: function 'other_call' called here from 'handler_UnsafeFunction'
+  // CHECK-NOTES: :[[@LINE+4]]:23: note: function 'handler_UnsafeFunction' registered here as signal handler
 }
 
-extern "C" {
+void test_UnsafeFunction() {
+  std::signal(SIGINT, handler_UnsafeFunction);
+}
 
-void handler_abort(int) {
-  std::abort();
+
+
+extern "C" void handler_UnsafeOperator(int) {
+  std::SysStruct S;
+  S << 1;
+  // CHECK-NOTES: :[[@LINE-1]]:3: warning: system call 'operator<<' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
+  // CHECK-NOTES: :[[@LINE-2]]:3: note: function 'operator<<' called here from 'handler_UnsafeOperator'
+  // CHECK-NOTES: :[[@LINE+4]]:23: note: function 'handler_UnsafeOperator' registered here as signal handler
 }
 
-void handler__Exit(int) {
-  std::_Exit(0);
+void test_UnsafeOperator() {
+  std::signal(SIGINT, handler_UnsafeOperator);
 }
 
-void handler_quick_exit(int) {
-  std::quick_exit(0);
+
+
+extern "C" void handler_Lambda(int) {
+  int I = []() { std::other_call(); return 1; }();
+  // CHECK-NOTES: :[[@LINE-1]]:18: warning: system call 'other_call' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
+  // CHECK-NOTES: :[[@LINE-2]]:18: note: function 'other_call' called here from 'handler_Lambda'
+  // CHECK-NOTES: :[[@LINE+4]]:23: note: function 'handler_Lambda' registered here as signal handler
 }
 
-void handler_signal(int) {
-  // FIXME: It is only OK to call signal with the current signal number.
-  std::signal(0, SIG_DFL);
-  std::signal(0, SIG_IGN);
+void test_Lambda() {
+  std::signal(SIGINT, handler_Lambda);
 }
 
-void handler_bad1(int) {
+
+
+void handler_noexternc(int) {
   std::other_call();
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'other_call' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
 }
 
-void handler_bad2(int) {
-  std::SysStruct S;
-  S << 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'operator<<' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
-}
+struct StaticHandler {
+  static void handler(int);
+};
 
-void handler_bad3(int) {
-  S S1(0);
+void StaticHandler::handler(int) {
+  std::other_call();
 }
 
-void handler_bad4(int) {
-  S S1;
-  -S1;
+void test_noexternc() {
+  std::signal(SIGINT, handler_noexternc);
+  // CHECK-NOTES: :[[@LINE-1]]:23: warning: function 'handler_noexternc' should have C language linkage if used as signal handler [bugprone-signal-handler]
+  // CHECK-NOTES: :[[@LINE-15]]:1: note: handler function declared here
+  std::signal(SIGINT, StaticHandler::handler);
+  // CHECK-NOTES: :[[@LINE-1]]:23: warning: function 'handler' should have C language linkage if used as signal handler [bugprone-signal-handler]
+  // CHECK-NOTES: :[[@LINE-13]]:3: note: handler function declared here
 }
 
-void handler_bad5(int) {
-  TestMemberInit MI;
+
+
+extern "C" {
+
+void handler_abort(int) {
+  std::abort();
 }
 
-void handler_bad6(int) {
-  testDefaultInit();
+void handler__Exit(int) {
+  std::_Exit(0);
 }
 
-void handler_bad7(int) {
-  int I = []() { std::other_call(); return 2; }();
-  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'other_call' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
+void handler_quick_exit(int) {
+  std::quick_exit(0);
 }
+
+void handler_signal(int) {
+  // FIXME: It is only OK to call signal with the current signal number.
+  std::signal(0, SIG_DFL);
+  std::signal(0, SIG_IGN);
 }
 
-void handler_noexternc(int) {
-  std::other_call();
 }
 
 void test() {
+  // Register signal handlers with allowed functions.
   std::signal(SIGINT, handler_abort);
   std::signal(SIGINT, handler__Exit);
+  std::signal(SIGINT, handler_quick_exit);
   std::signal(SIGINT, handler_signal);
-  std::signal(SIGINT, handler_bad1);
-  std::signal(SIGINT, handler_bad2);
-  // test call of user-defined operator
-  std::signal(SIGINT, handler_bad3);
-  // test call of constructor
-  std::signal(SIGINT, handler_bad4);
-  // test call of default member initializer
-  std::signal(SIGINT, handler_bad5);
-  // test call of default argument initializer
-  std::signal(SIGINT, handler_bad6);
-  // test lambda call in handler
-  std::signal(SIGINT, handler_bad7);
 
-  std::signal(SIGINT, handler_noexternc);
-  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: function 'handler_noexternc' should have C language linkage if used as signal handler [bugprone-signal-handler]
-  // make a ExprWithCleanups
-  std::signal(-S(), handler_noexternc);
-  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: function 'handler_noexternc' should have C language linkage if used as signal handler [bugprone-signal-handler]
-  std::signal(SIGINT, S::handler);
-  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: function 'handler' should have C language linkage if used as signal handler [bugprone-signal-handler]
+  // Use a lambda that returns the signal handler.
+  // There is no warning for this case, no matter what the real handler will be.
+  std::signal(SIGINT, [](int) -> callback_t { return &handler_noexternc; }(1));
+
   std::signal(SIGINT, [](int) { std::other_call(); });
-  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: lambda expression is not allowed as signal handler [bugprone-signal-handler]
-  std::signal(SIGINT, [](int) -> callback_t { return &handler_bad1; }(1));
+  // CHECK-NOTES: :[[@LINE-1]]:23: warning: lambda expression is not allowed as signal handler [bugprone-signal-handler]
+
+  // make a ExprWithCleanups
+  std::signal(-S_operator(), [](int) { std::other_call(); });
+  // CHECK-NOTES: :[[@LINE-1]]:30: warning: lambda expression is not allowed as signal handler [bugprone-signal-handler]
 
-  // Do not find problems here.
+  // Do not find problems here (non-standard 'signal' function).
   signal(SIGINT, handler_abort, 1);
   ns::signal(SIGINT, handler_abort);
-  S::signal(SIGINT, handler_abort);
+  Signal::signal(SIGINT, handler_abort);
   system_other::signal(SIGINT, handler_abort);
 }