From d60d63a00bb50ba6896939705c589578177b404d Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Tue, 29 Sep 2020 15:55:33 -0400
Subject: [PATCH] analyzer: fix signal-handler registration location [PR95188]

PR analyzer/95188 reports that diagnostics from
-Wanalyzer-unsafe-call-within-signal-handler use the wrong
source location when reporting the signal-handler registration
event in the diagnostic_path.  The diagnostics erroneously use the
location of the first stmt in the basic block containing the call
to "signal", rather than that of the call itself.

Fixed thusly.

gcc/analyzer/ChangeLog:
	PR analyzer/95188
	* engine.cc (stmt_requires_new_enode_p): Split enodes before
	"signal" calls.

gcc/testsuite/ChangeLog:
	PR analyzer/95188
	* gcc.dg/analyzer/signal-registration-loc.c: New test.
---
 gcc/analyzer/engine.cc                        | 22 +++++++++++++-----
 .../gcc.dg/analyzer/signal-registration-loc.c | 23 +++++++++++++++++++
 2 files changed, 39 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/analyzer/signal-registration-loc.c

diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc
index c15d1195a97..0e79254ad60 100644
--- a/gcc/analyzer/engine.cc
+++ b/gcc/analyzer/engine.cc
@@ -2677,13 +2677,23 @@ static bool
 stmt_requires_new_enode_p (const gimple *stmt,
 			   const gimple *prev_stmt)
 {
-  /* Stop consolidating at calls to
-     "__analyzer_dump_exploded_nodes", so they always appear at the
-     start of an exploded_node.  */
   if (const gcall *call = dyn_cast <const gcall *> (stmt))
-    if (is_special_named_call_p (call, "__analyzer_dump_exploded_nodes",
-			 1))
-      return true;
+    {
+      /* Stop consolidating at calls to
+	 "__analyzer_dump_exploded_nodes", so they always appear at the
+	 start of an exploded_node.  */
+      if (is_special_named_call_p (call, "__analyzer_dump_exploded_nodes",
+				   1))
+	return true;
+
+      /* sm-signal.cc injects an additional custom eedge at "signal" calls
+	 from the registration enode to the handler enode, separate from the
+	 regular next state, which defeats the "detect state change" logic
+	 in process_node.  Work around this via special-casing, to ensure
+	 we split the enode immediately before any "signal" call.  */
+      if (is_special_named_call_p (call, "signal", 2))
+	return true;
+    }
 
   /* If we had a PREV_STMT with an unknown location, and this stmt
      has a known location, then if a state change happens here, it
diff --git a/gcc/testsuite/gcc.dg/analyzer/signal-registration-loc.c b/gcc/testsuite/gcc.dg/analyzer/signal-registration-loc.c
new file mode 100644
index 00000000000..4bac1269b1e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/signal-registration-loc.c
@@ -0,0 +1,23 @@
+/* Ensure we use the correct location when reporting where the
+   signal handler was registered (PR analyzer/95188).  */
+
+/* { dg-require-effective-target signal } */
+
+#include <stdio.h>
+#include <signal.h>
+
+int g;
+extern int foo (void);
+
+static void
+handler (int n)
+{
+  fprintf (stderr, "got here: %i\n", g); /* { dg-warning "call to 'fprintf' from within signal handler" } */
+}
+
+int main (int argc, char *argv[])
+{
+  g = foo (); /* { dg-bogus "registering" } */
+  signal (SIGSEGV, handler); /* { dg-message "registering 'handler' as signal handler" } */
+  return 0;
+}
-- 
2.30.2