From 63ee77bfa7830c7e36b87890148f7ce8fc53aefc Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Tue, 21 Jul 2020 15:58:07 -0400
Subject: [PATCH 314/377] FIXME: don't store sm-state for NULL, fixing PR 93032

---
 gcc/analyzer/diagnostic-manager.cc            | 120 ++++++++++++++++++
 gcc/analyzer/program-state.cc                 |   2 +-
 gcc/analyzer/region-model2.cc                 |  15 +++
 gcc/analyzer/sm-file.cc                       |  14 ++
 gcc/analyzer/sm-malloc.cc                     |  36 +++++-
 gcc/analyzer/sm.h                             |   5 +
 gcc/analyzer/store2.cc                        |  14 +-
 .../gcc.dg/analyzer/attribute-nonnull.c       |  12 +-
 .../analyzer/pr93032-mztools-simplified.c     |   6 +-
 .../gcc.dg/analyzer/pr93032-mztools.c         |   6 +-
 10 files changed, 204 insertions(+), 26 deletions(-)
 rename FIXME-pr93032-mztools-simplified.c => gcc/testsuite/gcc.dg/analyzer/pr93032-mztools-simplified.c (72%)
 rename FIXME-pr93032-mztools.c => gcc/testsuite/gcc.dg/analyzer/pr93032-mztools.c (97%)

diff --git a/gcc/analyzer/diagnostic-manager.cc b/gcc/analyzer/diagnostic-manager.cc
index 43cdfad3fd9..28924d493dd 100644
--- a/gcc/analyzer/diagnostic-manager.cc
+++ b/gcc/analyzer/diagnostic-manager.cc
@@ -1024,6 +1024,106 @@ for_each_state_change2 (const program_state &src_state,
   return false;
 }
 
+/* An sm_context for adding state_change2_event on assignments to NULL,
+   where the default state isn't m_start.  Storing such state in the
+   sm_state_map would lead to bloat of the exploded_graph, so we want
+   to leave it as a default state, and inject state change events here
+   when we have a diagnostic.
+   Find transitions of constants, for handling on_zero_assignment.  */
+
+struct null_assignment_sm_context : public sm_context
+{
+  null_assignment_sm_context (int sm_idx,
+			      const state_machine &sm,
+			      const program_state *old_state,
+			      const program_state *new_state,
+			      const gimple *stmt,
+			      const program_point *point,
+			      checker_path *emission_path)
+  : sm_context (sm_idx, sm), m_old_state (old_state), m_new_state (new_state),
+    m_stmt (stmt), m_point (point), m_emission_path (emission_path)
+  {
+  }
+
+  tree get_fndecl_for_call (const gcall */*call*/) FINAL OVERRIDE
+  {
+    return NULL_TREE;
+  }
+
+  void on_transition (const supernode *node ATTRIBUTE_UNUSED,
+		      const gimple *stmt ATTRIBUTE_UNUSED,
+		      tree var,
+		      state_machine::state_t from,
+		      state_machine::state_t to,
+		      tree origin) FINAL OVERRIDE
+  {
+    if (from != 0)
+      return;
+
+    const svalue2 *var_old_sval
+      = m_old_state->m_region_model2->get_rvalue (var, NULL);
+    const svalue2 *var_new_sval
+      = m_new_state->m_region_model2->get_rvalue (var, NULL);
+    const supernode *supernode = m_point->get_supernode ();
+    int stack_depth = m_point->get_stack_depth ();
+
+    m_emission_path->add_event (new state_change2_event (supernode,
+							 m_stmt,
+							 stack_depth,
+							 m_sm,
+							 var_new_sval,
+							 from, to,
+							 NULL,
+							 *m_new_state));
+
+  }
+
+  void warn_for_state (const supernode *, const gimple *,
+		       tree, state_machine::state_t,
+		       pending_diagnostic *d) FINAL OVERRIDE
+  {
+    delete d;
+  }
+
+  tree get_diagnostic_tree (tree expr) FINAL OVERRIDE
+  {
+    return expr;
+  }
+
+  state_machine::state_t get_global_state () const FINAL OVERRIDE
+  {
+    return 0;
+  }
+
+  void set_global_state (state_machine::state_t) FINAL OVERRIDE
+  {
+    /* No-op.  */
+  }
+
+  void on_custom_transition (custom_transition *) FINAL OVERRIDE
+  {
+  }
+
+  tree is_zero_assignment (const gimple *stmt) FINAL OVERRIDE
+  {
+    const gassign *assign_stmt = dyn_cast <const gassign *> (stmt);
+    if (!assign_stmt)
+     return NULL_TREE;
+    if (const svalue2 *sval
+	= m_new_state->m_region_model2->get_gassign_result (assign_stmt, NULL))
+      if (tree cst = sval->maybe_get_constant ())
+	if (::zerop(cst))
+	  return gimple_assign_lhs (assign_stmt);
+    return NULL_TREE;
+  }
+
+  const program_state *m_old_state;
+  const program_state *m_new_state;
+  const program_point *m_point;
+  state_change_visitor2 *m_visitor;
+  const gimple *m_stmt;
+  checker_path *m_emission_path;
+};
 
 /* Subroutine of diagnostic_manager::build_emission_path.
    Add any events for EEDGE to EMISSION_PATH.  */
@@ -1123,6 +1223,26 @@ diagnostic_manager::add_events_for_eedge (const path_builder &pb,
 	    (new statement_event (stmt,
 				  dst_point.get_fndecl (),
 				  dst_stack_depth, dst_state));
+
+	/* Create state change events for assignment to NULL.  */
+	if (stmt->code == GIMPLE_ASSIGN)
+	  {
+	    const extrinsic_state &ext_state = pb.get_ext_state ();
+	    for (unsigned i = 0; i < ext_state.get_num_checkers (); i++)
+	      {
+		const state_machine &sm = ext_state.get_sm (i);
+		program_state state_after_stmt (dst_state);
+		state_after_stmt.m_region_model2->on_stmt (stmt, NULL);
+		null_assignment_sm_context sm_ctxt (i, sm,
+						    &dst_state,
+						    &state_after_stmt,
+						    stmt,
+						    &dst_point, emission_path);
+		sm.on_stmt (&sm_ctxt, dst_point.get_supernode (), stmt);
+		// TODO: what if it isn't the first stmt in the enode?
+		// TODO: what about phi nodes?
+	      }
+	  }
       }
       break;
     }
diff --git a/gcc/analyzer/program-state.cc b/gcc/analyzer/program-state.cc
index e71837e86ab..d1f043c5740 100644
--- a/gcc/analyzer/program-state.cc
+++ b/gcc/analyzer/program-state.cc
@@ -804,7 +804,7 @@ sm_state_map2::get_state (const svalue2 *sval,
 	      }
 	}
 
-  return 0;
+  return m_sm.get_default_state (sval);
 }
 
 /* Get the "origin" svalue2 for any state of SVAL.  */
diff --git a/gcc/analyzer/region-model2.cc b/gcc/analyzer/region-model2.cc
index 0d0a46ae93a..9a2574c29f3 100644
--- a/gcc/analyzer/region-model2.cc
+++ b/gcc/analyzer/region-model2.cc
@@ -286,6 +286,21 @@ svalue2::can_merge_p (const svalue2 *other,
       || (other->get_kind () == svalue2::SK_UNMERGEABLE))
     return NULL;
 
+  /* Reject attempts to merge NULL pointers with not-NULL-pointers.  */
+  if (POINTER_TYPE_P (get_type ()))
+    {
+      bool null0 = false;
+      bool null1 = false;
+      if (tree cst0 = maybe_get_constant ())
+	if (zerop (cst0))
+	  null0 = true;
+      if (tree cst1 = other->maybe_get_constant ())
+	if (zerop (cst1))
+	  null1 = true;
+      if (null0 != null1)
+	return NULL;
+    }
+
 #if WIDENING
   /* Widening.  */
   /* Merge: (new_cst, existing_cst) -> widen (existing, new).  */
diff --git a/gcc/analyzer/sm-file.cc b/gcc/analyzer/sm-file.cc
index 024bf965e9f..a41cd1b5b94 100644
--- a/gcc/analyzer/sm-file.cc
+++ b/gcc/analyzer/sm-file.cc
@@ -36,6 +36,9 @@ along with GCC; see the file COPYING3.  If not see
 #include "analyzer/pending-diagnostic.h"
 #include "analyzer/function-set.h"
 #include "analyzer/analyzer-selftests.h"
+#include "tristate.h"
+#include "selftest.h"
+#include "analyzer/region-model2.h"
 
 #if ENABLE_ANALYZER
 
@@ -52,6 +55,17 @@ public:
 
   bool inherited_state_p () const FINAL OVERRIDE { return false; }
 
+  state_machine::state_t
+  get_default_state (const svalue2 *sval) const FINAL OVERRIDE
+  {
+    if (tree cst = sval->maybe_get_constant ())
+      {
+	if (zerop (cst))
+	  return m_null;
+      }
+    return m_start;
+  }
+
   bool on_stmt (sm_context *sm_ctxt,
 		const supernode *node,
 		const gimple *stmt) const FINAL OVERRIDE;
diff --git a/gcc/analyzer/sm-malloc.cc b/gcc/analyzer/sm-malloc.cc
index cbc619733f6..3e39bdbf9fe 100644
--- a/gcc/analyzer/sm-malloc.cc
+++ b/gcc/analyzer/sm-malloc.cc
@@ -35,6 +35,9 @@ along with GCC; see the file COPYING3.  If not see
 #include "analyzer/analyzer-logging.h"
 #include "analyzer/sm.h"
 #include "analyzer/pending-diagnostic.h"
+#include "tristate.h"
+#include "selftest.h"
+#include "analyzer/region-model2.h"
 
 #if ENABLE_ANALYZER
 
@@ -53,6 +56,17 @@ public:
 
   bool inherited_state_p () const FINAL OVERRIDE { return false; }
 
+  state_machine::state_t
+  get_default_state (const svalue2 *sval) const FINAL OVERRIDE
+  {
+    if (tree cst = sval->maybe_get_constant ())
+      {
+	if (zerop (cst))
+	  return m_null;
+      }
+    return m_start;
+  }
+
   bool on_stmt (sm_context *sm_ctxt,
 		const supernode *node,
 		const gimple *stmt) const FINAL OVERRIDE;
@@ -427,9 +441,15 @@ public:
     auto_diagnostic_group d;
     diagnostic_metadata m;
     m.add_cwe (690);
-    bool warned = warning_meta (rich_loc, m, OPT_Wanalyzer_null_argument,
-				"use of NULL %qE where non-null expected",
-				m_arg);
+
+    bool warned;
+    if (zerop (m_arg))
+      warned = warning_meta (rich_loc, m, OPT_Wanalyzer_null_argument,
+			     "use of NULL where non-null expected");
+    else
+      warned = warning_meta (rich_loc, m, OPT_Wanalyzer_null_argument,
+			     "use of NULL %qE where non-null expected",
+			     m_arg);
     if (warned)
       inform_nonnull_attribute (m_fndecl, m_arg_idx);
     return warned;
@@ -437,9 +457,13 @@ public:
 
   label_text describe_final_event (const evdesc::final_event &ev) FINAL OVERRIDE
   {
-    return ev.formatted_print ("argument %u (%qE) NULL"
-			       " where non-null expected",
-			       m_arg_idx + 1, ev.m_expr);
+    if (zerop (ev.m_expr))
+      return ev.formatted_print ("argument %u NULL where non-null expected",
+				 m_arg_idx + 1);
+    else
+      return ev.formatted_print ("argument %u (%qE) NULL"
+				 " where non-null expected",
+				 m_arg_idx + 1, ev.m_expr);
   }
 
 private:
diff --git a/gcc/analyzer/sm.h b/gcc/analyzer/sm.h
index b37c27f5a69..26be70bd247 100644
--- a/gcc/analyzer/sm.h
+++ b/gcc/analyzer/sm.h
@@ -52,6 +52,11 @@ public:
      within a heap-allocated struct.  */
   virtual bool inherited_state_p () const = 0;
 
+  virtual state_machine::state_t get_default_state (const svalue2 *) const
+  {
+    return 0;
+  }
+
   const char *get_name () const { return m_name; }
 
   const char *get_state_name (state_t s) const;
diff --git a/gcc/analyzer/store2.cc b/gcc/analyzer/store2.cc
index 591aae47c40..5333c905f19 100644
--- a/gcc/analyzer/store2.cc
+++ b/gcc/analyzer/store2.cc
@@ -850,19 +850,13 @@ binding_cluster2::can_merge_p (const binding_cluster2 *cluster_a,
 	      out_cluster->m_map.put (key, merged_sval);
 	      continue;
 	    }
+	  /* Merger of the svalues failed.  Reject merger of the cluster.   */
+	  return false;
 	}
 
-      /* If we get here, then either one cluster binds this key and the other
-	 doesn't, or svalue2::can_merge_p failed.  */
+      /* If we get here, then one cluster binds this key and the other
+	 doesn't; merge them as "UNKNOWN".  */
       gcc_assert (sval_a || sval_b);
-
-      /* Don't merge unmergeable.  */
-      if (sval_a && sval_a->dyn_cast_unmergeable_svalue2 ())
-	return false;
-      if (sval_b && sval_b->dyn_cast_unmergeable_svalue2 ())
-	return false;
-
-      /* Otherwise, merge them as "UNKNOWN".  */
       tree type = sval_a ? sval_a->get_type () : sval_b->get_type ();
       const svalue2 *unknown_sval
 	= mgr->get_svalue2_manager ()->get_or_create_unknown_svalue2 (type);
diff --git a/gcc/testsuite/gcc.dg/analyzer/attribute-nonnull.c b/gcc/testsuite/gcc.dg/analyzer/attribute-nonnull.c
index e0bf1f4aa1c..70bb921e742 100644
--- a/gcc/testsuite/gcc.dg/analyzer/attribute-nonnull.c
+++ b/gcc/testsuite/gcc.dg/analyzer/attribute-nonnull.c
@@ -12,9 +12,10 @@ extern void bar(void *ptrA, void *ptrB, void *ptrC) /* { dg-message "argument 1
 void test_1 (void *p, void *q, void *r)
 {
   foo(p, q, r);
-  foo(NULL, q, r);
+  foo(NULL, q, r); /* { dg-warning "use of NULL where non-null expected" "warning" } */
+  /* { dg-message "argument 1 NULL where non-null expected" "note" { target *-*-* } .-1 } */
   foo(p, NULL, r);
-  foo(p, q, NULL);
+  foo(p, q, NULL); /* { dg-warning "use of NULL where non-null expected" } */
 }
 
 void test_1a (void *q, void *r)
@@ -27,9 +28,10 @@ void test_1a (void *q, void *r)
 void test_2 (void *p, void *q, void *r)
 {
   bar(p, q, r);
-  bar(NULL, q, r);
-  bar(p, NULL, r);
-  bar(p, q, NULL);
+  bar(NULL, q, r); /* { dg-warning "use of NULL where non-null expected" "warning" } */
+  bar(p, NULL, r); /* { dg-warning "use of NULL where non-null expected" "warning" } */
+  /* { dg-message "argument 2 NULL where non-null expected" "note" { target *-*-* } .-1 } */
+  bar(p, q, NULL); /* { dg-warning "use of NULL where non-null expected" "warning" } */
 }
 
 void test_3 (void *q, void *r)
diff --git a/FIXME-pr93032-mztools-simplified.c b/gcc/testsuite/gcc.dg/analyzer/pr93032-mztools-simplified.c
similarity index 72%
rename from FIXME-pr93032-mztools-simplified.c
rename to gcc/testsuite/gcc.dg/analyzer/pr93032-mztools-simplified.c
index 492991df5d3..85bf5d83d17 100644
--- a/FIXME-pr93032-mztools-simplified.c
+++ b/gcc/testsuite/gcc.dg/analyzer/pr93032-mztools-simplified.c
@@ -18,9 +18,11 @@ extern void unzRepair(const char* file, const char* fileOut, const char* fileOut
   FILE* fpZip = fopen(file, "rb");
   FILE* fpOut = fopen(fileOut, "wb");
   FILE* fpOutCD = fopen(fileOutTmp, "wb");
-  if (fpZip != NULL &&  fpOut != NULL) {
+  if (fpZip != NULL && fpOut != NULL) {
     fclose(fpOutCD);
     fclose(fpZip);
     fclose(fpOut);
   }
-}
+} /* { dg-warning "leak of FILE 'fpZip'" "leak of fpZip" } */
+  /* { dg-warning "leak of FILE 'fpOut'" "leak of fpOut" { target *-*-* } .-1 } */
+  /* { dg-warning "leak of FILE 'fpOutCD'" "leak of fpOutCD" { target *-*-* } .-2 } */
diff --git a/FIXME-pr93032-mztools.c b/gcc/testsuite/gcc.dg/analyzer/pr93032-mztools.c
similarity index 97%
rename from FIXME-pr93032-mztools.c
rename to gcc/testsuite/gcc.dg/analyzer/pr93032-mztools.c
index fcdf9d64146..5dd0abe8028 100644
--- a/FIXME-pr93032-mztools.c
+++ b/gcc/testsuite/gcc.dg/analyzer/pr93032-mztools.c
@@ -76,7 +76,7 @@ uLong* bytesRecovered;
   FILE* fpZip = fopen(file, "rb");
   FILE* fpOut = fopen(fileOut, "wb");
   FILE* fpOutCD = fopen(fileOutTmp, "wb");
-  if (fpZip != NULL &&  fpOut != NULL) {
+  if (fpZip != NULL && fpOut != NULL) {
     int entries = 0;
     uLong totalBytes = 0;
     char header[30];
@@ -325,5 +325,7 @@ uLong* bytesRecovered;
   } else {
     err = Z_STREAM_ERROR;
   }
-  return err;
+  return err; /* { dg-warning "leak of FILE 'fpZip'" "leak of fpZip" } */
+  /* { dg-warning "leak of FILE 'fpOut'" "leak of fpOut" { target *-*-* } .-1 } */
+  /* { dg-warning "leak of FILE 'fpOutCD'" "leak of fpOutCD" { target *-*-* } .-2 } */
 }
-- 
2.26.2

