[gcc(refs/vendors/redhat/heads/gcc-10-branch)] analyzer: avoid comparisons between uncomparable types (PR 93450)

[gcc(refs/vendors/redhat/heads/gcc-10-branch)] analyzer: avoid comparisons between uncomparable types (PR 93450)

[Jakub Jelinek]

https://gcc.gnu.org/g:d177c49cd31131c8cededb216da30877d8a3856d

commit d177c49cd31131c8cededb216da30877d8a3856d
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Wed Jan 29 20:24:42 2020 -0500

    analyzer: avoid comparisons between uncomparable types (PR 93450)
    
    PR analyzer/93450 reports an ICE trying to fold an EQ_EXPR comparison
    of (int)0 with (float)Inf.
    
    Most comparisons inside the analyzer come from gimple conditions, for
    which the necessary casts have already been added.
    
    This one is done inside constant_svalue::eval_condition as part of
    purging sm-state for an unknown function call, and fails to check
    the types being compared, leading to the ICE.
    
    sm_state_map::set_state calls region_model::eval_condition_without_cm in
    order to handle pointer equality (so that e.g. (void *)&r and (foo *)&r
    transition together), which leads to this code generating a bogus query
    to see if the two constants are equal.
    
    This patch fixes the ICE in two ways:
    
    - It avoids generating comparisons within
      constant_svalue::eval_condition unless the types are equal (thus for
      constants, but not for pointer values, which are handled by
      region_svalue).
    
    - It updates sm_state_map::set_state to bail immediately if the new
      state is the same as the old one, thus avoiding the above for the
      common case where an svalue_id has no sm-state (such as for the int
      and float constants in the reproducer), for which the above becomes a
      no-op.
    
    gcc/analyzer/ChangeLog:
    	PR analyzer/93450
    	* program-state.cc (sm_state_map::set_state): For the overload
    	taking an svalue_id, bail out if the set_state on the ec does
    	nothing.  Convert the latter's return type from void to bool,
    	returning true if anything changed.
    	(sm_state_map::impl_set_state): Convert the return type from void
    	to bool, returning true if the state changed.
    	* program-state.h (sm_state_map::set_state): Convert return type
    	from void to bool.
    	(sm_state_map::impl_set_state): Likewise.
    	* region-model.cc (constant_svalue::eval_condition): Only call
    	fold_build2 if the types are the same.
    
    gcc/testsuite/ChangeLog:
    	PR analyzer/93450
    	* gcc.dg/analyzer/torture/pr93450.c: New test.

Diff:
---
 gcc/analyzer/ChangeLog                          | 15 +++++++++++++++
 gcc/analyzer/program-state.cc                   | 23 ++++++++++++++++-------
 gcc/analyzer/program-state.h                    |  4 ++--
 gcc/analyzer/region-model.cc                    | 16 ++++++++++------
 gcc/testsuite/ChangeLog                         |  5 +++++
 gcc/testsuite/gcc.dg/analyzer/torture/pr93450.c | 25 +++++++++++++++++++++++++
 6 files changed, 73 insertions(+), 15 deletions(-)

diff --git a/gcc/analyzer/ChangeLog b/gcc/analyzer/ChangeLog
index 94a67ea..f1ac6e6 100644
--- a/gcc/analyzer/ChangeLog
+++ b/gcc/analyzer/ChangeLog
@@ -1,3 +1,18 @@
+2020-01-30  David Malcolm  <dmalcolm@redhat.com>
+
+	PR analyzer/93450
+	* program-state.cc (sm_state_map::set_state): For the overload
+	taking an svalue_id, bail out if the set_state on the ec does
+	nothing.  Convert the latter's return type from void to bool,
+	returning true if anything changed.
+	(sm_state_map::impl_set_state): Convert the return type from void
+	to bool, returning true if the state changed.
+	* program-state.h (sm_state_map::set_state): Convert return type
+	from void to bool.
+	(sm_state_map::impl_set_state): Likewise.
+	* region-model.cc (constant_svalue::eval_condition): Only call
+	fold_build2 if the types are the same.
+
 2020-01-29  Jakub Jelinek  <jakub@redhat.com>
 
 	* analyzer.h (PUSH_IGNORE_WFORMAT, POP_IGNORE_WFORMAT): Remove.
diff --git a/gcc/analyzer/program-state.cc b/gcc/analyzer/program-state.cc
index a9e300f..f41f105 100644
--- a/gcc/analyzer/program-state.cc
+++ b/gcc/analyzer/program-state.cc
@@ -259,7 +259,8 @@ sm_state_map::set_state (region_model *model,
   if (model == NULL)
     return;
   equiv_class &ec = model->get_constraints ()->get_equiv_class (sid);
-  set_state (ec, state, origin);
+  if (!set_state (ec, state, origin))
+    return;
 
   /* Also do it for all svalues that are equal via non-cm, so that
      e.g. (void *)&r and (foo *)&r transition together.  */
@@ -276,34 +277,42 @@ sm_state_map::set_state (region_model *model,
 }
 
 /* Set the state of EC to STATE, recording that the state came from
-   ORIGIN.  */
+   ORIGIN.
+   Return true if any states of svalue_ids within EC changed.  */
 
-void
+bool
 sm_state_map::set_state (const equiv_class &ec,
 			 state_machine::state_t state,
 			 svalue_id origin)
 {
   int i;
   svalue_id *sid;
+  bool any_changed = false;
   FOR_EACH_VEC_ELT (ec.m_vars, i, sid)
-    impl_set_state (*sid, state, origin);
+    any_changed |= impl_set_state (*sid, state, origin);
+  return any_changed;
 }
 
-/* Set state of PV to STATE, bypassing equivalence classes.  */
+/* Set state of SID to STATE, bypassing equivalence classes.
+   Return true if the state changed.  */
 
-void
+bool
 sm_state_map::impl_set_state (svalue_id sid, state_machine::state_t state,
 			      svalue_id origin)
 {
+  if (get_state (sid) == state)
+    return false;
+
   /* Special-case state 0 as the default value.  */
   if (state == 0)
     {
       if (m_map.get (sid))
 	m_map.remove (sid);
-      return;
+      return true;
     }
   gcc_assert (!sid.null_p ());
   m_map.put (sid, entry_t (state, origin));
+  return true;
 }
 
 /* Set the "global" state within this state map to STATE.  */
diff --git a/gcc/analyzer/program-state.h b/gcc/analyzer/program-state.h
index adc71a4..0a4e35f 100644
--- a/gcc/analyzer/program-state.h
+++ b/gcc/analyzer/program-state.h
@@ -161,10 +161,10 @@ public:
 		  svalue_id sid,
 		  state_machine::state_t state,
 		  svalue_id origin);
-  void set_state (const equiv_class &ec,
+  bool set_state (const equiv_class &ec,
 		  state_machine::state_t state,
 		  svalue_id origin);
-  void impl_set_state (svalue_id sid,
+  bool impl_set_state (svalue_id sid,
 		       state_machine::state_t state,
 		       svalue_id origin);
 
diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index a5b3dff..c838454 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -666,12 +666,16 @@ constant_svalue::eval_condition (constant_svalue *lhs,
   gcc_assert (CONSTANT_CLASS_P (lhs_const));
   gcc_assert (CONSTANT_CLASS_P (rhs_const));
 
-  tree comparison
-    = fold_build2 (op, boolean_type_node, lhs_const, rhs_const);
-  if (comparison == boolean_true_node)
-    return tristate (tristate::TS_TRUE);
-  if (comparison == boolean_false_node)
-    return tristate (tristate::TS_FALSE);
+  /* Check for comparable types.  */
+  if (TREE_TYPE (lhs_const) == TREE_TYPE (rhs_const))
+    {
+      tree comparison
+	= fold_build2 (op, boolean_type_node, lhs_const, rhs_const);
+      if (comparison == boolean_true_node)
+	return tristate (tristate::TS_TRUE);
+      if (comparison == boolean_false_node)
+	return tristate (tristate::TS_FALSE);
+    }
   return tristate::TS_UNKNOWN;
 }
 
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index ab61b48..a97bf32 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2020-01-30  David Malcolm  <dmalcolm@redhat.com>
+
+	PR analyzer/93450
+	* gcc.dg/analyzer/torture/pr93450.c: New test.
+
 2020-01-30  Jakub Jelinek  <jakub@redhat.com>
 
 	PR target/93494
diff --git a/gcc/testsuite/gcc.dg/analyzer/torture/pr93450.c b/gcc/testsuite/gcc.dg/analyzer/torture/pr93450.c
new file mode 100644
index 0000000..7f6cba0
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/torture/pr93450.c
@@ -0,0 +1,25 @@
+void
+ed (int);
+
+double
+bg (void)
+{
+  double kl = __builtin_huge_val ();
+
+  ed (0);
+
+  return kl;
+}
+
+static double __attribute__((noinline))
+get_hugeval (void)
+{
+  return __builtin_huge_val ();
+}
+
+int test_2 (int i)
+{
+  if (i < get_hugeval ())
+    return 42;
+  return 0;
+}