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 --git a/gcc/analyzer/ChangeLog b/gcc/analyzer/ChangeLog
index 94a67ea40f776cb9878e0ecd45fbcf24a0ad0c6b..f1ac6e6e63fb4e2a227e7b00ece29ebe1863c197 100644 (file)
--- 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 a9e300fba0fffea5e597991fb2ee29c0fdc33fe3..f41f105ce6b06acf40741e090d41f61260100c31 100644 (file)
--- 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 adc71a4eda2a4097aa80a159d11f374b318c2ad6..0a4e35f3d5d34b01b7a8d136ec275f3767ffff25 100644 (file)
--- 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 a5b3dffcc278ce6348031657b2fcc9efb5a46316..c838454a1ebcd6d72d0ed7bb9a088290e760e19f 100644 (file)
--- 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 ab61b48c7bddd33fe619e3f54b9bc9ccaa2a9db9..a97bf3250130f8350455e300e4287fcac44d3a96 100644 (file)
--- 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 (file)
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;
+}