analyzer: stricter handling of non-pure builtins [PR96798]
authorDavid Malcolm <dmalcolm@redhat.com>
Mon, 31 Aug 2020 19:55:45 +0000 (15:55 -0400)
committerDavid Malcolm <dmalcolm@redhat.com>
Fri, 11 Sep 2020 01:08:09 +0000 (21:08 -0400)
Amongst other things PR analyzer/96798 notes that
region_model::on_call_pre treats any builtin that hasn't been coded
yet as a no-op (albeit with an unknown return value), which is wrong
for non-pure builtins.

This patch updates that function's handling of such builtins so that it
instead conservatively assumes that any escaped/reachable regions can
be affected by the call, and implements enough handling of specific
builtins to avoid regressing the testsuite (I hope).

gcc/analyzer/ChangeLog:
PR analyzer/96798
* region-model-impl-calls.cc (region_model::impl_call_memcpy):
New.
(region_model::impl_call_strcpy): New.
* region-model.cc (region_model::on_call_pre): Flag unhandled
builtins that are non-pure as having unknown side-effects.
Implement BUILT_IN_MEMCPY, BUILT_IN_MEMCPY_CHK, BUILT_IN_STRCPY,
BUILT_IN_STRCPY_CHK, BUILT_IN_FPRINTF, BUILT_IN_FPRINTF_UNLOCKED,
BUILT_IN_PUTC, BUILT_IN_PUTC_UNLOCKED, BUILT_IN_FPUTC,
BUILT_IN_FPUTC_UNLOCKED, BUILT_IN_FPUTS, BUILT_IN_FPUTS_UNLOCKED,
BUILT_IN_FWRITE, BUILT_IN_FWRITE_UNLOCKED, BUILT_IN_PRINTF,
BUILT_IN_PRINTF_UNLOCKED, BUILT_IN_PUTCHAR,
BUILT_IN_PUTCHAR_UNLOCKED, BUILT_IN_PUTS, BUILT_IN_PUTS_UNLOCKED,
BUILT_IN_VFPRINTF, BUILT_IN_VPRINTF.
* region-model.h (region_model::impl_call_memcpy): New decl.
(region_model::impl_call_strcpy): New decl.

gcc/testsuite/ChangeLog:
PR analyzer/96798
* gcc.dg/analyzer/memcpy-1.c: New test.
* gcc.dg/analyzer/strcpy-1.c: New test.

gcc/analyzer/region-model-impl-calls.cc
gcc/analyzer/region-model.cc
gcc/analyzer/region-model.h
gcc/testsuite/gcc.dg/analyzer/memcpy-1.c [new file with mode: 0644]
gcc/testsuite/gcc.dg/analyzer/strcpy-1.c [new file with mode: 0644]

index fa85035..6582ffb 100644 (file)
@@ -276,6 +276,30 @@ region_model::impl_call_malloc (const call_details &cd)
   return true;
 }
 
+/* Handle the on_call_pre part of "memcpy" and "__builtin_memcpy".  */
+
+void
+region_model::impl_call_memcpy (const call_details &cd)
+{
+  const svalue *dest_sval = cd.get_arg_svalue (0);
+  const svalue *num_bytes_sval = cd.get_arg_svalue (2);
+
+  const region *dest_reg = deref_rvalue (dest_sval, cd.get_arg_tree (0),
+                                        cd.get_ctxt ());
+
+  cd.maybe_set_lhs (dest_sval);
+
+  if (tree num_bytes = num_bytes_sval->maybe_get_constant ())
+    {
+      /* "memcpy" of zero size is a no-op.  */
+      if (zerop (num_bytes))
+       return;
+    }
+
+  /* Otherwise, mark region's contents as unknown.  */
+  mark_region_as_unknown (dest_reg);
+}
+
 /* Handle the on_call_pre part of "memset" and "__builtin_memset".  */
 
 bool
@@ -353,6 +377,21 @@ region_model::impl_call_operator_delete (const call_details &cd)
   return false;
 }
 
+/* Handle the on_call_pre part of "strcpy" and "__builtin_strcpy_chk".  */
+
+void
+region_model::impl_call_strcpy (const call_details &cd)
+{
+  const svalue *dest_sval = cd.get_arg_svalue (0);
+  const region *dest_reg = deref_rvalue (dest_sval, cd.get_arg_tree (0),
+                                        cd.get_ctxt ());
+
+  cd.maybe_set_lhs (dest_sval);
+
+  /* For now, just mark region's contents as unknown.  */
+  mark_region_as_unknown (dest_reg);
+}
+
 /* Handle the on_call_pre part of "strlen".
    Return true if the LHS is updated.  */
 
index 03c7daf..75f4eae 100644 (file)
@@ -658,6 +658,8 @@ region_model::on_call_pre (const gcall *call, region_model_context *ctxt)
        switch (DECL_UNCHECKED_FUNCTION_CODE (callee_fndecl))
          {
          default:
+           if (!DECL_PURE_P (callee_fndecl))
+             unknown_side_effects = true;
            break;
          case BUILT_IN_ALLOCA:
          case BUILT_IN_ALLOCA_WITH_ALIGN:
@@ -672,20 +674,54 @@ region_model::on_call_pre (const gcall *call, region_model_context *ctxt)
            break;
          case BUILT_IN_MALLOC:
            return impl_call_malloc (cd);
+         case BUILT_IN_MEMCPY:
+         case BUILT_IN_MEMCPY_CHK:
+           impl_call_memcpy (cd);
+           return false;
          case BUILT_IN_MEMSET:
          case BUILT_IN_MEMSET_CHK:
            impl_call_memset (cd);
            return false;
            break;
+         case BUILT_IN_STRCPY:
+         case BUILT_IN_STRCPY_CHK:
+           impl_call_strcpy (cd);
+           return false;
          case BUILT_IN_STRLEN:
            if (impl_call_strlen (cd))
              return false;
            break;
+
+         /* Stdio builtins.  */
+         case BUILT_IN_FPRINTF:
+         case BUILT_IN_FPRINTF_UNLOCKED:
+         case BUILT_IN_PUTC:
+         case BUILT_IN_PUTC_UNLOCKED:
+         case BUILT_IN_FPUTC:
+         case BUILT_IN_FPUTC_UNLOCKED:
+         case BUILT_IN_FPUTS:
+         case BUILT_IN_FPUTS_UNLOCKED:
+         case BUILT_IN_FWRITE:
+         case BUILT_IN_FWRITE_UNLOCKED:
+         case BUILT_IN_PRINTF:
+         case BUILT_IN_PRINTF_UNLOCKED:
+         case BUILT_IN_PUTCHAR:
+         case BUILT_IN_PUTCHAR_UNLOCKED:
+         case BUILT_IN_PUTS:
+         case BUILT_IN_PUTS_UNLOCKED:
+         case BUILT_IN_VFPRINTF:
+         case BUILT_IN_VPRINTF:
+           /* These stdio builtins have external effects that are out
+              of scope for the analyzer: we only want to model the effects
+              on the return value.  */
+           break;
          }
       else if (gimple_call_internal_p (call))
        switch (gimple_call_internal_fn (call))
          {
          default:
+           if (!DECL_PURE_P (callee_fndecl))
+             unknown_side_effects = true;
            break;
          case IFN_BUILTIN_EXPECT:
            return impl_call_builtin_expect (cd);
index 4707293..1bb9798 100644 (file)
@@ -2554,7 +2554,9 @@ class region_model
   bool impl_call_calloc (const call_details &cd);
   void impl_call_free (const call_details &cd);
   bool impl_call_malloc (const call_details &cd);
+  void impl_call_memcpy (const call_details &cd);
   bool impl_call_memset (const call_details &cd);
+  void impl_call_strcpy (const call_details &cd);
   bool impl_call_strlen (const call_details &cd);
   bool impl_call_operator_new (const call_details &cd);
   bool impl_call_operator_delete (const call_details &cd);
diff --git a/gcc/testsuite/gcc.dg/analyzer/memcpy-1.c b/gcc/testsuite/gcc.dg/analyzer/memcpy-1.c
new file mode 100644 (file)
index 0000000..f120eac
--- /dev/null
@@ -0,0 +1,43 @@
+#include <string.h>
+#include "analyzer-decls.h"
+
+void *test_1 (void *dst, void *src, size_t n)
+{
+  void *result = memcpy (dst, src, n);
+  __analyzer_eval (result == dst); /* { dg-warning "TRUE" } */
+  return result;
+}
+
+void *test_1a (void *dst, void *src, size_t n)
+{
+  void *result = __memcpy_chk (dst, src, n, -1);
+  __analyzer_eval (result == dst); /* { dg-warning "TRUE" } */
+  return result;
+}
+
+void test_2 (int i)
+{
+  int j;
+  memcpy (&j, &i, sizeof (int));
+  __analyzer_eval (i == j); /* { dg-warning "TRUE" } */
+}
+
+void test_2a (int i)
+{
+  int j;
+  __memcpy_chk (&j, &i, sizeof (int), sizeof (int));
+  __analyzer_eval (i == j);  /* { dg-warning "TRUE" } */
+}
+
+void test_3 (void *src, size_t n)
+{
+  char buf[40], other[40];
+  buf[0] = 'a';
+  other[0] = 'b';
+  __analyzer_eval (buf[0] == 'a');    /* { dg-warning "TRUE" } */
+  __analyzer_eval (other[0] == 'b');  /* { dg-warning "TRUE" } */
+
+  memcpy (buf, src, n);
+  __analyzer_eval (buf[0] == 'a');    /* { dg-warning "UNKNOWN" } */
+  __analyzer_eval (other[0] == 'b');  /* { dg-warning "TRUE" } */
+}
diff --git a/gcc/testsuite/gcc.dg/analyzer/strcpy-1.c b/gcc/testsuite/gcc.dg/analyzer/strcpy-1.c
new file mode 100644 (file)
index 0000000..ed5bab9
--- /dev/null
@@ -0,0 +1,18 @@
+#include <string.h>
+#include "analyzer-decls.h"
+
+char *
+test_1 (char *dst, char *src)
+{
+  char *result = strcpy (dst, src);
+  __analyzer_eval (result == dst); /* { dg-warning "TRUE" } */
+  return result;
+}
+
+char *
+test_1a (char *dst, char *src)
+{
+  char *result = __strcpy_chk (dst, src, -1);
+  __analyzer_eval (result == dst); /* { dg-warning "TRUE" } */
+  return result;
+}