[committed] analyzer: fix "when 'strchr' returns non-NULL" message

[committed] analyzer: fix "when 'strchr' returns non-NULL" message

[David Malcolm]

Tweak analyzer handling of strchr, so that we show the
  when 'strchr' returns non-NULL
message for that execution path.

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to trunk as r13-3768-g55e042407ef307.

gcc/analyzer/ChangeLog:
	* region-model-impl-calls.cc (region_model::impl_call_strchr):
	Move to on_call_post.  Handle both outcomes using bifurcation,
	rather than just the "not found" case.
	* region-model.cc (region_model::on_call_pre): Move
	BUILT_IN_STRCHR and "strchr" to...
	(region_model::on_call_post): ...here.

gcc/testsuite/ChangeLog:
	* gcc.dg/analyzer/strchr-1.c (test_literal): Detect writing to a
	string literal.  Verify that we emit the "when '__builtin_strchr'
	returns non-NULL" message.

Signed-off-by: David Malcolm <dmalcolm@redhat.com>
---
 gcc/analyzer/region-model-impl-calls.cc  | 14 +++++++-------
 gcc/analyzer/region-model.cc             | 14 ++++++++++++--
 gcc/testsuite/gcc.dg/analyzer/strchr-1.c |  3 ++-
 3 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/gcc/analyzer/region-model-impl-calls.cc b/gcc/analyzer/region-model-impl-calls.cc
index 30fa765c4b4..46dbbb53bdc 100644
--- a/gcc/analyzer/region-model-impl-calls.cc
+++ b/gcc/analyzer/region-model-impl-calls.cc
@@ -1013,7 +1013,7 @@ region_model::impl_call_realloc (const call_details &cd)
     }
 }
 
-/* Handle the on_call_pre part of "strchr" and "__builtin_strchr".  */
+/* Handle the on_call_post part of "strchr" and "__builtin_strchr".  */
 
 void
 region_model::impl_call_strchr (const call_details &cd)
@@ -1075,13 +1075,13 @@ region_model::impl_call_strchr (const call_details &cd)
     bool m_found;
   };
 
-  /* Bifurcate state, creating a "not found" out-edge.  */
+  /* Body of region_model::impl_call_strchr.  */
   if (cd.get_ctxt ())
-    cd.get_ctxt ()->bifurcate (make_unique<strchr_call_info> (cd, false));
-
-  /* The "unbifurcated" state is the "found" case.  */
-  strchr_call_info found (cd, true);
-  found.update_model (this, NULL, cd.get_ctxt ());
+    {
+      cd.get_ctxt ()->bifurcate (make_unique<strchr_call_info> (cd, false));
+      cd.get_ctxt ()->bifurcate (make_unique<strchr_call_info> (cd, true));
+      cd.get_ctxt ()->terminate_path ();
+    }
 }
 
 /* Handle the on_call_pre part of "strcpy" and "__builtin_strcpy_chk".  */
diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index edf3412a817..e182d2e0e1a 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -2223,7 +2223,7 @@ region_model::on_call_pre (const gcall *call, region_model_context *ctxt,
 	  case BUILT_IN_REALLOC:
 	    return false;
 	  case BUILT_IN_STRCHR:
-	    impl_call_strchr (cd);
+	    /* Handle in "on_call_post".  */
 	    return false;
 	  case BUILT_IN_STRCPY:
 	  case BUILT_IN_STRCPY_CHK:
@@ -2341,7 +2341,7 @@ region_model::on_call_pre (const gcall *call, region_model_context *ctxt,
       else if (is_named_call_p (callee_fndecl, "strchr", call, 2)
 	       && POINTER_TYPE_P (cd.get_arg_type (0)))
 	{
-	  impl_call_strchr (cd);
+	  /* Handle in "on_call_post".  */
 	  return false;
 	}
       else if (is_named_call_p (callee_fndecl, "strlen", call, 1)
@@ -2418,6 +2418,12 @@ region_model::on_call_post (const gcall *call,
 	  impl_call_pipe (cd);
 	  return;
 	}
+      else if (is_named_call_p (callee_fndecl, "strchr", call, 2)
+	       && POINTER_TYPE_P (cd.get_arg_type (0)))
+	{
+	  impl_call_strchr (cd);
+	  return;
+	}
       /* Was this fndecl referenced by
 	 __attribute__((malloc(FOO)))?  */
       if (lookup_attribute ("*dealloc", DECL_ATTRIBUTES (callee_fndecl)))
@@ -2435,6 +2441,10 @@ region_model::on_call_post (const gcall *call,
 	    impl_call_realloc (cd);
 	    return;
 
+	  case BUILT_IN_STRCHR:
+	    impl_call_strchr (cd);
+	    return;
+
 	  case BUILT_IN_VA_END:
 	    impl_call_va_end (cd);
 	    return;
diff --git a/gcc/testsuite/gcc.dg/analyzer/strchr-1.c b/gcc/testsuite/gcc.dg/analyzer/strchr-1.c
index dfe1bc9ea1a..bfa48916ca2 100644
--- a/gcc/testsuite/gcc.dg/analyzer/strchr-1.c
+++ b/gcc/testsuite/gcc.dg/analyzer/strchr-1.c
@@ -3,12 +3,13 @@
 
 const char* test_literal (int x)
 {
-  char *p = __builtin_strchr ("123", x);
+  char *p = __builtin_strchr ("123", x); /* { dg-message "when '__builtin_strchr' returns non-NULL" } */
   if (p)
     {
       __analyzer_eval (*p == x); /* { dg-message "UNKNOWN" } */
       /* TODO: this ought to be TRUE, but it's unclear that it's
 	 worth stashing this constraint.  */
+      *p = 'A'; /* { dg-warning "write to string literal" } */
     }
   return p;
 }
-- 
2.26.3