[gcc/devel/c++-modules] analyzer: fix global-sm-state issue affecting sm-signal

[Nathan Sidwell]

https://gcc.gnu.org/g:7fb3669edb4aa3c8313ddf8b914b86a1623e0954

commit 7fb3669edb4aa3c8313ddf8b914b86a1623e0954
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Sat Dec 21 08:49:03 2019 -0500

    analyzer: fix global-sm-state issue affecting sm-signal
    
    sm-signal.cc was failing to warn about the use of an fprintf call in a
    signal handler when the signal handler function was non-static.
    
    The root cause was a failure to copy global sm-state within
    sm_state_map::clone_with_remapping as called by
    program_state::can_merge_with_p, which led to the exploded node for
    the entrypoint to the handler in the "normal" state being erroneously
    reused for the "in_signal_handler" state, thus losing the global state,
    and thus failing to warn.
    
    This patch fixes the above, so that non-equal global sm-state values
    prevent merger of program_state, thus requiring separate exploded nodes
    for the "normal" and "in signal handler" states, and thus triggering
    the warning for the reproducer.
    
    gcc/analyzer/ChangeLog:
    	* program-state.cc (sm_state_map::clone_with_remapping): Copy
    	m_global_state.
    	(selftest::test_program_state_merging_2): New selftest.
    	(selftest::analyzer_program_state_cc_tests): Call it.
    
    gcc/testsuite/ChangeLog:
    	* gcc.dg/analyzer/signal-6.c: New test.

Diff:
---
 gcc/analyzer/ChangeLog                   |  7 +++++++
 gcc/analyzer/program-state.cc            | 35 ++++++++++++++++++++++++++++++++
 gcc/testsuite/ChangeLog                  |  4 ++++
 gcc/testsuite/gcc.dg/analyzer/signal-6.c | 20 ++++++++++++++++++
 4 files changed, 66 insertions(+)

diff --git a/gcc/analyzer/ChangeLog b/gcc/analyzer/ChangeLog
index d5dd0ca..7d3b109 100644
--- a/gcc/analyzer/ChangeLog
+++ b/gcc/analyzer/ChangeLog
@@ -1,5 +1,12 @@
 2020-01-14  David Malcolm  <dmalcolm@redhat.com>
 
+	* program-state.cc (sm_state_map::clone_with_remapping): Copy
+	m_global_state.
+	(selftest::test_program_state_merging_2): New selftest.
+	(selftest::analyzer_program_state_cc_tests): Call it.
+
+2020-01-14  David Malcolm  <dmalcolm@redhat.com>
+
 	* checker-path.h (checker_path::get_checker_event): New function.
 	(checker_path): Add DISABLE_COPY_AND_ASSIGN; make fields private.
 	* diagnostic-manager.cc
diff --git a/gcc/analyzer/program-state.cc b/gcc/analyzer/program-state.cc
index c9b595e..7dbdf9d 100644
--- a/gcc/analyzer/program-state.cc
+++ b/gcc/analyzer/program-state.cc
@@ -84,6 +84,7 @@ sm_state_map *
 sm_state_map::clone_with_remapping (const one_way_svalue_id_map &id_map) const
 {
   sm_state_map *result = new sm_state_map ();
+  result->m_global_state = m_global_state;
   for (typename map_t::iterator iter = m_map.begin ();
        iter != m_map.end ();
        ++iter)
@@ -1348,6 +1349,39 @@ test_program_state_merging ()
   ASSERT_EQ (s0, merged);
 }
 
+/* Verify that program_states with different global-state in an sm-state
+   can't be merged.  */
+
+static void
+test_program_state_merging_2 ()
+{
+  auto_delete_vec <state_machine> checkers;
+  checkers.safe_push (make_signal_state_machine (NULL));
+  extrinsic_state ext_state (checkers);
+
+  program_state s0 (ext_state);
+  {
+    sm_state_map *smap0 = s0.m_checker_states[0];
+    const state_machine::state_t TEST_STATE_0 = 0;
+    smap0->set_global_state (TEST_STATE_0);
+    ASSERT_EQ (smap0->get_global_state (), TEST_STATE_0);
+  }
+
+  program_state s1 (ext_state);
+  {
+    sm_state_map *smap1 = s1.m_checker_states[0];
+    const state_machine::state_t TEST_STATE_1 = 1;
+    smap1->set_global_state (TEST_STATE_1);
+    ASSERT_EQ (smap1->get_global_state (), TEST_STATE_1);
+  }
+
+  ASSERT_NE (s0, s1);
+
+  /* They ought to not be mergeable.  */
+  program_state merged (ext_state);
+  ASSERT_FALSE (s0.can_merge_with_p (s1, ext_state, &merged));
+}
+
 /* Run all of the selftests within this file.  */
 
 void
@@ -1355,6 +1389,7 @@ analyzer_program_state_cc_tests ()
 {
   test_sm_state_map ();
   test_program_state_merging ();
+  test_program_state_merging_2 ();
 }
 
 } // namespace selftest
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 135dae0..571b51a 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,5 +1,9 @@
 2020-01-14  David Malcolm  <dmalcolm@redhat.com>
 
+	* gcc.dg/analyzer/signal-6.c: New test.
+
+2020-01-14  David Malcolm  <dmalcolm@redhat.com>
+
 	* gcc.dg/analyzer/dot-output.c: Add test coverage for a BB with
 	no statements.
 
diff --git a/gcc/testsuite/gcc.dg/analyzer/signal-6.c b/gcc/testsuite/gcc.dg/analyzer/signal-6.c
new file mode 100644
index 0000000..f518451
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/signal-6.c
@@ -0,0 +1,20 @@
+#include <stdio.h>
+#include <signal.h>
+
+extern void body_of_program(void);
+
+/* Example of a non-static signal handler.  */
+
+void handler(int signum)
+{
+  fprintf(stderr, "LOG: %i", signum); /* { dg-warning "call to 'fprintf' from within signal handler" } */
+}
+
+int main(int argc, const char *argv)
+{
+  signal(SIGINT, handler); /* { dg-message "registering 'handler' as signal handler" } */
+
+  body_of_program();
+
+  return 0;
+}