analyzer: handle __builtin_expect [PR93993]

The false warning:
 pr93993.f90:19:0:

   19 |     allocate (tm) ! { dg-warning "dereference of possibly-NULL" }
      |
 Warning: dereference of possibly-NULL ‘_6’ [CWE-690] [-Wanalyzer-possible-null-dereference]

in the reproducer for PR analyzer/93993 is due to a BUILTIN_EXPECT in
the chain of SSA expressions between the malloc and the condition
guarding the edge: the analyzer didn't "know" about the relationship
between initial argument to BUILTIN_EXPECT and the return value.

This patch implements support for BUILTIN_EXPECT so that the return
value is known to be equal to the initial argument.  This adds
constraints when exploring the CFG edges, eliminating the above
false positive.

Doing so also eliminated the leak warning from the reproducer.  The
issue was that leaked_pvs was empty within
impl_region_model_context::on_state_leak, due to the leaking region
being a view, of type struct Pdtet_8 *, of a region of type
struct pdtet_8 *, which led region_model::get_representative_path_var to
return a NULL_TREE value.

Hence the patch also implements view support for
region_model::get_representative_path_var, restoring the leak
diagnostic, albeit changing the wording to:

  Warning: leak of ‘(struct Pdtet_8) qb’ [CWE-401] [-Wanalyzer-malloc-leak]

It's not clear to me if we should emit leaks at a fortran "end program"
(currently we suppress them for leaks at the end of main).

gcc/analyzer/ChangeLog:
	PR analyzer/93993
	* region-model.cc (region_model::on_call_pre): Handle
	BUILT_IN_EXPECT and its variants.
	(region_model::add_any_constraints_from_ssa_def_stmt): Split out
	gassign handling into add_any_constraints_from_gassign; add gcall
	handling.
	(region_model::add_any_constraints_from_gassign): New function,
	based on the above.  Add handling for NOP_EXPR.
	(region_model::add_any_constraints_from_gcall): New function.
	(region_model::get_representative_path_var): Handle views.
	* region-model.h
	(region_model::add_any_constraints_from_ssa_def_stmt): New decl.
	(region_model::add_any_constraints_from_gassign): New decl.

gcc/testsuite/ChangeLog:
	PR analyzer/93993
	* gcc.dg/analyzer/expect-1.c: New test.
	* gcc.dg/analyzer/malloc-4.c: New test.
	* gfortran.dg/analyzer/pr93993.f90: Remove xfail from dg-bogus.
	Move location of leak warning and update message.

diff --git a/gcc/analyzer/ChangeLog b/gcc/analyzer/ChangeLog
index 4f3e08e4dc47ce31c310010af71daf2e5c385d44..c4724cb090d069ace5ea2ab190ea3c266ecf6d39 100644 (file)
--- a/gcc/analyzer/ChangeLog
+++ b/gcc/analyzer/ChangeLog
@@ -1,3 +1,19 @@
+2020-03-04  David Malcolm  <dmalcolm@redhat.com>
+
+       PR analyzer/93993
+       * region-model.cc (region_model::on_call_pre): Handle
+       BUILT_IN_EXPECT and its variants.
+       (region_model::add_any_constraints_from_ssa_def_stmt): Split out
+       gassign handling into add_any_constraints_from_gassign; add gcall
+       handling.
+       (region_model::add_any_constraints_from_gassign): New function,
+       based on the above.  Add handling for NOP_EXPR.
+       (region_model::add_any_constraints_from_gcall): New function.
+       (region_model::get_representative_path_var): Handle views.
+       * region-model.h
+       (region_model::add_any_constraints_from_ssa_def_stmt): New decl.
+       (region_model::add_any_constraints_from_gassign): New decl.
+
 2020-03-04  David Malcolm  <dmalcolm@redhat.com>
 
        PR analyzer/93993

diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index b2179bd220aa4d34863540393095a73ec9012a6d..6813117968f83e312e6b9e50f07388feec50d04d 100644 (file)
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -4204,6 +4204,19 @@ region_model::on_call_pre (const gcall *call, region_model_context *ctxt)
            }
          return false;
        }
+      else if (gimple_call_builtin_p (call, BUILT_IN_EXPECT)
+              || gimple_call_builtin_p (call, BUILT_IN_EXPECT_WITH_PROBABILITY)
+              || gimple_call_internal_p (call, IFN_BUILTIN_EXPECT))
+       {
+         /* __builtin_expect's return value is its initial argument.  */
+         if (!lhs_rid.null_p ())
+           {
+             tree initial_arg = gimple_call_arg (call, 0);
+             svalue_id sid = get_rvalue (initial_arg, ctxt);
+             set_value (lhs_rid, sid, ctxt);
+           }
+         return false;
+       }
       else if (is_named_call_p (callee_fndecl, "strlen", call, 1))
        {
          region_id buf_rid = deref_rvalue (gimple_call_arg (call, 0), ctxt);
@@ -5447,28 +5460,46 @@ region_model::add_any_constraints_from_ssa_def_stmt (tree lhs,
   if (TREE_CODE (lhs) != SSA_NAME)
     return;
 
-  if (rhs != boolean_false_node)
+  if (!zerop (rhs))
     return;
 
   if (op != NE_EXPR && op != EQ_EXPR)
     return;
 
+  gimple *def_stmt = SSA_NAME_DEF_STMT (lhs);
+  if (const gassign *assign = dyn_cast<gassign *> (def_stmt))
+    add_any_constraints_from_gassign (op, rhs, assign, ctxt);
+  else if (gcall *call = dyn_cast<gcall *> (def_stmt))
+    add_any_constraints_from_gcall (op, rhs, call, ctxt);
+}
+
+/* Add any constraints for an SSA_NAME defined by ASSIGN
+   where the result OP RHS.  */
+
+void
+region_model::add_any_constraints_from_gassign (enum tree_code op,
+                                               tree rhs,
+                                               const gassign *assign,
+                                               region_model_context *ctxt)
+{
   /* We have either
      - "LHS != false" (i.e. LHS is true), or
      - "LHS == false" (i.e. LHS is false).  */
   bool is_true = op == NE_EXPR;
 
-  gimple *def_stmt = SSA_NAME_DEF_STMT (lhs);
-  gassign *assign = dyn_cast<gassign *> (def_stmt);
-  if (!assign)
-    return;
-
   enum tree_code rhs_code = gimple_assign_rhs_code (assign);
 
   switch (rhs_code)
     {
     default:
       break;
+
+    case NOP_EXPR:
+      {
+       add_constraint (gimple_assign_rhs1 (assign), op, rhs, ctxt);
+      }
+      break;
+
     case BIT_AND_EXPR:
       {
        if (is_true)
@@ -5514,6 +5545,24 @@ region_model::add_any_constraints_from_ssa_def_stmt (tree lhs,
     }
 }
 
+/* Add any constraints for an SSA_NAME defined by CALL
+   where the result OP RHS.  */
+
+void
+region_model::add_any_constraints_from_gcall (enum tree_code op,
+                                             tree rhs,
+                                             const gcall *call,
+                                             region_model_context *ctxt)
+{
+  if (gimple_call_builtin_p (call, BUILT_IN_EXPECT)
+      || gimple_call_builtin_p (call, BUILT_IN_EXPECT_WITH_PROBABILITY)
+      || gimple_call_internal_p (call, IFN_BUILTIN_EXPECT))
+    {
+      /* __builtin_expect's return value is its initial argument.  */
+      add_constraint (gimple_call_arg (call, 0), op, rhs, ctxt);
+    }
+}
+
 /* Determine what is known about the condition "LHS OP RHS" within
    this model.
    Use CTXT for reporting any diagnostics associated with the accesses.  */
@@ -5608,6 +5657,16 @@ region_model::get_representative_path_var (region_id rid) const
   region *parent_reg = get_region (parent_rid);
   if (parent_reg)
     {
+      if (reg->is_view_p ())
+       {
+         path_var parent_pv = get_representative_path_var (parent_rid);
+         if (parent_pv.m_tree && reg->get_type ())
+           return path_var (build1 (NOP_EXPR,
+                                    TREE_TYPE (reg->get_type ()),
+                                    parent_pv.m_tree),
+                            parent_pv.m_stack_depth);
+       }
+
       if (parent_reg->get_kind () == RK_STRUCT)
        {
          map_region *parent_map_region = (map_region *)parent_reg;

diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h
index f3cf45566d1180db962dc6d2c1724fbebb492e07..6d49f00cfe386859d85673449d066a3f52ee7d23 100644 (file)
--- a/gcc/analyzer/region-model.h
+++ b/gcc/analyzer/region-model.h
@@ -1850,6 +1850,14 @@ class region_model
                                              enum tree_code op,
                                              tree rhs,
                                              region_model_context *ctxt);
+  void add_any_constraints_from_gassign (enum tree_code op,
+                                        tree rhs,
+                                        const gassign *assign,
+                                        region_model_context *ctxt);
+  void add_any_constraints_from_gcall (enum tree_code op,
+                                      tree rhs,
+                                      const gcall *call,
+                                      region_model_context *ctxt);
 
   void update_for_call_superedge (const call_superedge &call_edge,
                                  region_model_context *ctxt);

diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index d44d3c7cbe50a9b6897bd47929e48f63288868bb..af4470d6303ffc814df209483f7bcc421e0c8d82 100644 (file)
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,11 @@
+2020-03-04  David Malcolm  <dmalcolm@redhat.com>
+
+       PR analyzer/93993
+       * gcc.dg/analyzer/expect-1.c: New test.
+       * gcc.dg/analyzer/malloc-4.c: New test.
+       * gfortran.dg/analyzer/pr93993.f90: Remove xfail from dg-bogus.
+       Move location of leak warning and update message.
+
 2020-03-04  David Malcolm  <dmalcolm@redhat.com>
 
        PR analyzer/93993

diff --git a/gcc/testsuite/gcc.dg/analyzer/expect-1.c b/gcc/testsuite/gcc.dg/analyzer/expect-1.c
new file mode 100644 (file)
index 0000000..e538f77
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/expect-1.c
@@ -0,0 +1,32 @@
+#define NULL ((void*)0)
+
+void *test_1 (void)
+{
+  int *p = (int *)__builtin_malloc (sizeof (int));
+  if (__builtin_expect (p != NULL, 1))
+    {
+      *p = 42;
+      return p;
+    }
+  return NULL;    
+}
+
+void *test_2 (void)
+{
+  int *p = (int *)__builtin_malloc (sizeof (int));
+  if (__builtin_expect (p == NULL, 1))
+    return NULL;
+
+  *p = 42;
+  return p;
+}
+
+void *test_3 (void)
+{
+  int *p = (int *)__builtin_malloc (sizeof (int));
+  if (__builtin_expect_with_probability (p == NULL, 1, 0.5f))
+    return NULL;
+
+  *p = 42;
+  return p;
+}

diff --git a/gcc/testsuite/gcc.dg/analyzer/malloc-4.c b/gcc/testsuite/gcc.dg/analyzer/malloc-4.c
new file mode 100644 (file)
index 0000000..94d2825
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/malloc-4.c
@@ -0,0 +1,20 @@
+/* { dg-additional-options "-Wno-incompatible-pointer-types" } */
+
+#include <stdlib.h>
+
+struct foo;
+struct bar;
+void *hv (struct foo **tm)
+{
+  void *p = __builtin_malloc (4);
+  *tm = p;
+  if (!p)
+    abort ();
+  return p; /* { dg-warning "leak of 'tm'" } */
+}
+
+void a5 (void)
+{
+  struct bar *qb = NULL;
+  hv (&qb);
+} /* { dg-warning "leak of '\\(struct foo\\)qb'" } */

diff --git a/gcc/testsuite/gfortran.dg/analyzer/pr93993.f90 b/gcc/testsuite/gfortran.dg/analyzer/pr93993.f90
index 8d5261c0eb904d882de952fdf1269cdf2c9e3667..230b99e4fcde88d05b96d356ecf795132c4dc5ef 100644 (file)
--- a/gcc/testsuite/gfortran.dg/analyzer/pr93993.f90
+++ b/gcc/testsuite/gfortran.dg/analyzer/pr93993.f90
@@ -16,9 +16,9 @@ contains
     type (et(real_kind=za)), allocatable, target :: tm
     type (et(real_kind=za)), pointer :: ce
 
-    allocate (tm) ! { dg-bogus "dereference of possibly-NULL" "" { xfail *-*-* } }
+    allocate (tm) ! { dg-bogus "dereference of possibly-NULL" }
     ce => tm
-  end function hv ! { dg-warning "leak of 'tm'" }
+  end function hv
 
 end module gg
 
@@ -30,4 +30,4 @@ program a5
   type (et(real_kind=za)), pointer :: vt
 
   vt => hv (qb)
-end program a5
+end program a5 ! { dg-warning "leak of '.*qb'" }