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

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

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>

diff --git a/gcc/analyzer/region-model-impl-calls.cc b/gcc/analyzer/region-model-impl-calls.cc
index 30fa765..46dbbb5 100644 (file)
--- 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 edf3412..e182d2e 100644 (file)
--- 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 dfe1bc9..bfa4891 100644 (file)
--- 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;
 }