From a451911250931b350d7ce1b95f145a3d1731c77f Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Fri, 22 Mar 2019 16:43:53 -0400
Subject: [PATCH 138/169] FIXME: implement custom messages for final events

---
 gcc/analyzer/engine.cc                         | 13 ++++++++++---
 gcc/analyzer/hardcoded-state-machine-malloc.cc | 21 +++++++++++++++++----
 gcc/analyzer/hardcoded-state-machine.h         | 10 ++++++++++
 gcc/testsuite/gcc.dg/analyzer/malloc-ipa-10.c  |  2 +-
 gcc/testsuite/gcc.dg/analyzer/malloc-ipa-2.c   |  2 +-
 gcc/testsuite/gcc.dg/analyzer/malloc-ipa-8.c   |  7 ++++---
 gcc/testsuite/gcc.dg/analyzer/malloc-paths-1.c |  4 ++--
 gcc/testsuite/gcc.dg/analyzer/malloc-paths-2.c |  2 +-
 gcc/testsuite/gcc.dg/analyzer/malloc-paths-5.c |  4 ++--
 libcpp/include/line-map.h                      |  9 +++++++++
 10 files changed, 57 insertions(+), 17 deletions(-)

diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc
index 12fa87d..db751ab 100644
--- a/gcc/analyzer/engine.cc
+++ b/gcc/analyzer/engine.cc
@@ -3189,15 +3189,22 @@ engine::maybe_emit (const candidate_diagnostic &cd)
 
   prune_path (expanded_path, cd.m_var, cd.m_state);
 
-  char *msg = print ("here (%qE is in state %qs)", cd.m_var,
-		     m_sm.get_state_name (cd.m_state));
+  char *msg = NULL;
+  label_text ev_desc = cd.m_pd->describe_final_event (cd.m_var, cd.m_state);
+  if (ev_desc.m_buffer)
+    msg = ev_desc.take_or_copy ();
+  else
+    {
+      msg = print ("here (%qE is in state %qs)", cd.m_var,
+		   m_sm.get_state_name (cd.m_state));
+      // FIXME: ideally would show the full message here, too?
+    }
 
   checker_event *end_of_path
    = new warning_event (cd.m_stmt->location,
 			cd.m_node->m_fun->decl, path.m_stack_depth,
 			msg);
   expanded_path->add_event (end_of_path);
-  // FIXME: ideally would show the full message here, too?
 
   expanded_path->set_pending_diagnostic (cd.m_pd);
 
diff --git a/gcc/analyzer/hardcoded-state-machine-malloc.cc b/gcc/analyzer/hardcoded-state-machine-malloc.cc
index 6365eeb..7a8eb52 100644
--- a/gcc/analyzer/hardcoded-state-machine-malloc.cc
+++ b/gcc/analyzer/hardcoded-state-machine-malloc.cc
@@ -66,12 +66,13 @@ public:
     warning_at (rich_loc, m, "double-free of %qE", m_arg);
   }
 
-  label_text describe_critical_state_change (tree expr,
-					     state_machine::state_t old_state,
-					     state_machine::state_t new_state)
+  label_text describe_critical_state_change
+    (tree expr,
+     state_machine::state_t old_state ATTRIBUTE_UNUSED,
+     state_machine::state_t new_state)
     FINAL OVERRIDE
   {
-    if (old_state == m_sm.m_start && new_state == m_sm.m_free)
+    if (new_state == m_sm.m_free)
       {
 	pretty_printer pp;
 	pp_printf (&pp, "first %qs here", "free");
@@ -81,6 +82,18 @@ public:
     return label_text ();
   }
 
+  /* FIXME.  */
+  virtual label_text describe_final_event
+    (/*diagnostic_context *ctxt, */
+     tree expr ATTRIBUTE_UNUSED,
+     state_machine::state_t state ATTRIBUTE_UNUSED)
+  {
+    pretty_printer pp;
+    pp_printf (&pp, "second %qs here", "free");
+    return label_text::take (xstrdup (pp_formatted_text (&pp)));
+    // TODO: using appropriate colorization etc
+  }
+
   /* FIXME: maybe also something like this.  */
 #if 0
   label_test label_critical_state (int event_idx)
diff --git a/gcc/analyzer/hardcoded-state-machine.h b/gcc/analyzer/hardcoded-state-machine.h
index 7078011..c9bda73 100644
--- a/gcc/analyzer/hardcoded-state-machine.h
+++ b/gcc/analyzer/hardcoded-state-machine.h
@@ -131,6 +131,16 @@ class pending_diagnostic
     /* Default no-op implementation.  */
     return label_text ();
   }
+
+  /* FIXME.  */
+  virtual label_text describe_final_event
+    (/*diagnostic_context *ctxt, */
+     tree expr ATTRIBUTE_UNUSED,
+     state_machine::state_t state ATTRIBUTE_UNUSED)
+  {
+    /* Default no-op implementation.  */
+    return label_text ();
+  }
 };
 
 #if 0
diff --git a/gcc/testsuite/gcc.dg/analyzer/malloc-ipa-10.c b/gcc/testsuite/gcc.dg/analyzer/malloc-ipa-10.c
index e59faa2..a88263f 100644
--- a/gcc/testsuite/gcc.dg/analyzer/malloc-ipa-10.c
+++ b/gcc/testsuite/gcc.dg/analyzer/malloc-ipa-10.c
@@ -6,7 +6,7 @@ calls_free (void *victim) /* { dg-message "\\(3\\) entry to 'calls_free'" } */
 {
   free (victim); /* { dg-warning "double-free of 'victim'" } */
   /* { dg-message "\\(4\\) first 'free' here" "" { target *-*-* } .-1 } */
-  /* { dg-message "\\(8\\) here \\('victim' is in state 'free'\\)" "" { target *-*-* } .-2 } */
+  /* { dg-message "\\(8\\) second 'free' here" "" { target *-*-* } .-2 } */
 
   /* TODO: would this be better emitted at the callsite,
      for such a simple wrapper?  */
diff --git a/gcc/testsuite/gcc.dg/analyzer/malloc-ipa-2.c b/gcc/testsuite/gcc.dg/analyzer/malloc-ipa-2.c
index 508cfdc..f42042d 100644
--- a/gcc/testsuite/gcc.dg/analyzer/malloc-ipa-2.c
+++ b/gcc/testsuite/gcc.dg/analyzer/malloc-ipa-2.c
@@ -8,7 +8,7 @@ calls_free (void *victim) /* { dg-message "\\(3\\) entry to 'calls_free'" } */
 {
   free (victim); /* { dg-warning "double-free of 'victim'" } */
   /* { dg-message "\\(4\\) first 'free' here" "" { target *-*-* } .-1 } */
-  /* { dg-message "\\(8\\) here \\('victim' is in state 'free'\\)" "" { target *-*-* } .-2 } */
+  /* { dg-message "\\(8\\) second 'free' here" "" { target *-*-* } .-2 } */
 
   /* TODO: would this be better emitted at the callsite,
      for such a simple wrapper?  */
diff --git a/gcc/testsuite/gcc.dg/analyzer/malloc-ipa-8.c b/gcc/testsuite/gcc.dg/analyzer/malloc-ipa-8.c
index d222e04..3bd9a98 100644
--- a/gcc/testsuite/gcc.dg/analyzer/malloc-ipa-8.c
+++ b/gcc/testsuite/gcc.dg/analyzer/malloc-ipa-8.c
@@ -18,10 +18,11 @@ void wrapped_free (void *ptr) /* { dg-warning "summary: add: 'ptr': 'start' -> '
 {
   free (ptr); /* { dg-warning "double-free of 'ptr' \\\[CWE-415\\]" } */
   /* { dg-message "\\(11\\) first 'free' here" "" { target *-*-* } .-1 } */
-  /* { dg-message "\\(12\\) state of 'ptr': 'unchecked' -> 'free'" "" { target *-*-* } .-2 } */
+  /* { dg-message "\\(12\\) first 'free' here" "" { target *-*-* } .-2 } */
   /* FIXME: unchecked -> free is correct, but start -> free should be pruned
-     from the path.  */
-  /* { dg-message "\\(18\\) here \\('ptr' is in state 'free'\\)" "" { target *-*-* } .-5 } */
+     from the path.
+     FIXME: do we need a way of turning off the friendlier messages?  */
+  /* { dg-message "\\(18\\) second 'free' here" "" { target *-*-* } .-6 } */
 }
 
 typedef struct boxed_int
diff --git a/gcc/testsuite/gcc.dg/analyzer/malloc-paths-1.c b/gcc/testsuite/gcc.dg/analyzer/malloc-paths-1.c
index 232b2d6..0e743b6 100644
--- a/gcc/testsuite/gcc.dg/analyzer/malloc-paths-1.c
+++ b/gcc/testsuite/gcc.dg/analyzer/malloc-paths-1.c
@@ -10,6 +10,6 @@ void test_1 (void)
 
   /* { dg-warning "double-free of 'ptr'"  "" { target *-*-* } second_free } */
   /* { dg-message "\\(1\\) state of 'ptr': 'start' -> 'unchecked'" "" { target *-*-* } malloc } */
-  /* { dg-message "\\(2\\) state of 'ptr': 'unchecked' -> 'free'" "" { target *-*-* } first_free } */
-  /* { dg-message "\\(3\\) here \\('ptr' is in state 'free'\\)" "" { target *-*-* } second_free } */
+  /* { dg-message "\\(2\\) first 'free' here" "" { target *-*-* } first_free } */
+  /* { dg-message "\\(3\\) second 'free' here" "" { target *-*-* } second_free } */
 }
diff --git a/gcc/testsuite/gcc.dg/analyzer/malloc-paths-2.c b/gcc/testsuite/gcc.dg/analyzer/malloc-paths-2.c
index c0cd5a8..eda86c2 100644
--- a/gcc/testsuite/gcc.dg/analyzer/malloc-paths-2.c
+++ b/gcc/testsuite/gcc.dg/analyzer/malloc-paths-2.c
@@ -9,5 +9,5 @@ void test_2 (void *ptr)
 
   /* { dg-warning "double-free of 'ptr'"  "" { target *-*-* } second_free } */
   /* { dg-message "\\(1\\) first 'free' here" "" { target *-*-* } first_free } */
-  /* { dg-message "\\(2\\) here \\('ptr' is in state 'free'\\)" "" { target *-*-* } second_free } */
+  /* { dg-message "\\(2\\) second 'free' here" "" { target *-*-* } second_free } */
 }
diff --git a/gcc/testsuite/gcc.dg/analyzer/malloc-paths-5.c b/gcc/testsuite/gcc.dg/analyzer/malloc-paths-5.c
index 5023cca..081c83c 100644
--- a/gcc/testsuite/gcc.dg/analyzer/malloc-paths-5.c
+++ b/gcc/testsuite/gcc.dg/analyzer/malloc-paths-5.c
@@ -32,8 +32,8 @@ int test (const char *filename, int flag)
   /* { dg-warning "double-free of 'p'" "" { target *-*-* } second_free_of_p } */
   /* { dg-message "\\(1\\) state of 'p': 'start' -> 'unchecked'" "" { target *-*-* } malloc_of_p } */
   /* { dg-message "\\(2\\) state of 'p': 'unchecked' -> 'nonnull'" "" { target *-*-* } test_of_p } */
-  /* { dg-message "\\(3\\) state of 'p': 'nonnull' -> 'free'" "" { target *-*-* } first_free_of_p } */
-  /* { dg-message "\\(4\\) here \\('p' is in state 'free'\\)" "" { target *-*-* } second_free_of_p } */
+  /* { dg-message "\\(3\\) first 'free' here" "" { target *-*-* } first_free_of_p } */
+  /* { dg-message "\\(4\\) second 'free' here" "" { target *-*-* } second_free_of_p } */
 
   /* We dont' care about the state changes to q.  */
   /* TODO: we do care about the test of q.  */
diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h
index 6b23db3..107fb46 100644
--- a/libcpp/include/line-map.h
+++ b/libcpp/include/line-map.h
@@ -1812,6 +1812,15 @@ struct label_text
     return label_text (buffer, true);
   }
 
+  /* Take ownership of the buffer, copying if necessary.  */
+  char *take_or_copy ()
+  {
+    if (m_caller_owned)
+      return m_buffer;
+    else
+      return xstrdup (m_buffer);
+  }
+
   char *m_buffer;
   bool m_caller_owned;
 
-- 
1.8.5.3

