analyzer: fix ICE in __builtin_isnan (PR 93356)
authorDavid Malcolm <dmalcolm@redhat.com>
Thu, 30 Jan 2020 17:35:46 +0000 (12:35 -0500)
committerDavid Malcolm <dmalcolm@redhat.com>
Fri, 31 Jan 2020 00:00:41 +0000 (19:00 -0500)
PR analyzer/93356 reports an ICE handling __builtin_isnan due to a
failing assertion:
  674     gcc_assert (lhs_ec_id != rhs_ec_id);
with op=UNORDERED_EXPR.
when attempting to add an UNORDERED_EXPR constraint.

This is an overzealous assertion, but underlying it are various forms of
sloppiness regarding NaN within the analyzer:

  (a) the assumption in the constraint_manager that equivalence classes
  are reflexive (X == X), which isn't the case for NaN.

  (b) Hardcoding the "honor_nans" param to false when calling
  invert_tree_comparison throughout the analyzer.

  (c) Ignoring ORDERED_EXPR, UNORDERED_EXPR, and the UN-prefixed
  comparison codes.

I wrote a patch for this which tracks the NaN-ness of floating-point
values and uses this to address all of the above.

However, to minimize changes in gcc 10 stage 4, here's a simpler patch
which rejects attempts to query or add constraints on floating-point
values, instead treating any floating-point comparison as "unknown", and
silently dropping the constraints at edges.

gcc/analyzer/ChangeLog:
PR analyzer/93356
* region-model.cc (region_model::eval_condition): In both
overloads, bail out immediately on floating-point types.
(region_model::eval_condition_without_cm): Likewise.
(region_model::add_constraint): Likewise.

gcc/testsuite/ChangeLog:
PR analyzer/93356
* gcc.dg/analyzer/conditionals-notrans.c (test_float_selfcmp):
Add.
* gcc.dg/analyzer/conditionals-trans.c: Mark floating point
comparison test as failing.
(test_float_selfcmp): Add.
* gcc.dg/analyzer/data-model-1.c: Mark floating point comparison
tests as failing.
* gcc.dg/analyzer/torture/pr93356.c: New test.

gcc/ChangeLog:
PR analyzer/93356
* doc/analyzer.texi (Limitations): Note that constraints on
floating-point values are currently ignored.

gcc/ChangeLog
gcc/analyzer/ChangeLog
gcc/analyzer/region-model.cc
gcc/doc/analyzer.texi
gcc/testsuite/ChangeLog
gcc/testsuite/gcc.dg/analyzer/conditionals-notrans.c
gcc/testsuite/gcc.dg/analyzer/conditionals-trans.c
gcc/testsuite/gcc.dg/analyzer/data-model-1.c
gcc/testsuite/gcc.dg/analyzer/torture/pr93356.c [new file with mode: 0644]

index a45d0907e9d116b25970ab43be7932ea50deb28f..4e312cb8c3be55b088cc768042e389dc990f66f6 100644 (file)
@@ -1,3 +1,9 @@
+2020-01-30  David Malcolm  <dmalcolm@redhat.com>
+
+       PR analyzer/93356
+       * doc/analyzer.texi (Limitations): Note that constraints on
+       floating-point values are currently ignored.
+
 2020-01-30  Jakub Jelinek  <jakub@redhat.com>
 
        PR lto/93384
index f1ac6e6e63fb4e2a227e7b00ece29ebe1863c197..1c981e4266b66212ac945c9c49c7617d134f535f 100644 (file)
@@ -1,3 +1,11 @@
+2020-01-30  David Malcolm  <dmalcolm@redhat.com>
+
+       PR analyzer/93356
+       * region-model.cc (region_model::eval_condition): In both
+       overloads, bail out immediately on floating-point types.
+       (region_model::eval_condition_without_cm): Likewise.
+       (region_model::add_constraint): Likewise.
+
 2020-01-30  David Malcolm  <dmalcolm@redhat.com>
 
        PR analyzer/93450
index c838454a1ebcd6d72d0ed7bb9a088290e760e19f..a15088a2e3cfe417347fceed5e3bea0bdbc3634b 100644 (file)
@@ -5144,6 +5144,15 @@ region_model::eval_condition (svalue_id lhs_sid,
                              enum tree_code op,
                              svalue_id rhs_sid) const
 {
+  svalue *lhs = get_svalue (lhs_sid);
+  svalue *rhs = get_svalue (rhs_sid);
+
+  /* For now, make no attempt to capture constraints on floating-point
+     values.  */
+  if ((lhs->get_type () && FLOAT_TYPE_P (lhs->get_type ()))
+      || (rhs->get_type () && FLOAT_TYPE_P (rhs->get_type ())))
+    return tristate::unknown ();
+
   tristate ts = eval_condition_without_cm (lhs_sid, op, rhs_sid);
 
   if (ts.is_known ())
@@ -5173,6 +5182,12 @@ region_model::eval_condition_without_cm (svalue_id lhs_sid,
   /* See what we know based on the values.  */
   if (lhs && rhs)
     {
+      /* For now, make no attempt to capture constraints on floating-point
+        values.  */
+      if ((lhs->get_type () && FLOAT_TYPE_P (lhs->get_type ()))
+         || (rhs->get_type () && FLOAT_TYPE_P (rhs->get_type ())))
+       return tristate::unknown ();
+
       if (lhs == rhs)
        {
          /* If we have the same svalue, then we have equality
@@ -5252,6 +5267,11 @@ bool
 region_model::add_constraint (tree lhs, enum tree_code op, tree rhs,
                              region_model_context *ctxt)
 {
+  /* For now, make no attempt to capture constraints on floating-point
+     values.  */
+  if (FLOAT_TYPE_P (TREE_TYPE (lhs)) || FLOAT_TYPE_P (TREE_TYPE (rhs)))
+    return true;
+
   svalue_id lhs_sid = get_rvalue (lhs, ctxt);
   svalue_id rhs_sid = get_rvalue (rhs, ctxt);
 
@@ -5385,6 +5405,11 @@ region_model::eval_condition (tree lhs,
                              tree rhs,
                              region_model_context *ctxt)
 {
+  /* For now, make no attempt to model constraints on floating-point
+     values.  */
+  if (FLOAT_TYPE_P (TREE_TYPE (lhs)) || FLOAT_TYPE_P (TREE_TYPE (rhs)))
+    return tristate::unknown ();
+
   return eval_condition (get_rvalue (lhs, ctxt), op, get_rvalue (rhs, ctxt));
 }
 
index 81acdd8998bd805a631e65272ce707036eeb70aa..1fe4bcefd1bc8a4afa6fde9591ff3e8ff23a1453 100644 (file)
@@ -390,6 +390,8 @@ Lack of function pointer analysis
 @item
 The constraint-handling code assumes reflexivity in some places
 (that values are equal to themselves), which is not the case for NaN.
+As a simple workaround, constraints on floating-point values are
+currently ignored.
 @item
 The region model code creates lots of little mutable objects at each
 @code{region_model} (and thus per @code{exploded_node}) rather than
index 90eb0b2ffba6a6552bde63ae84d892df384cf0ce..621d4283ae3c15f1ece6519bcd52ae571bc93dfb 100644 (file)
@@ -1,4 +1,16 @@
-2020-01-30  Jeff Law  <law@redhat.com
+2020-01-30  David Malcolm  <dmalcolm@redhat.com>
+
+       PR analyzer/93356
+       * gcc.dg/analyzer/conditionals-notrans.c (test_float_selfcmp):
+       Add.
+       * gcc.dg/analyzer/conditionals-trans.c: Mark floating point
+       comparison test as failing.
+       (test_float_selfcmp): Add.
+       * gcc.dg/analyzer/data-model-1.c: Mark floating point comparison
+       tests as failing.
+       * gcc.dg/analyzer/torture/pr93356.c: New test.
+
+2020-01-30  Jeff Law  <law@redhat.com>
 
        PR c/88660
        * gcc.dg/pr88660.c: New test
index 3b6e28cf539014123cdc06cadb6eb77373dfd74f..a00127b1a7f54e48f7b2b43b8997af482ac3f915 100644 (file)
@@ -157,3 +157,9 @@ void test_range_float_ge_le (float f)
       __analyzer_eval (f == 4); /* { dg-warning "TRUE" "desired" { xfail *-*-* } } */
       /* { dg-bogus "UNKNOWN" "status quo" { xfail *-*-* } .-1 } */
 }
+
+void test_float_selfcmp (float f)
+{
+  __analyzer_eval (f == f); /* { dg-warning "UNKNOWN" } */
+  __analyzer_eval (f != f); /* { dg-warning "UNKNOWN" } */
+}
index ab34618c4110356fe07ac5c4202d7945ecd3f8b4..f032789e6c4ab8f3000a1611d4892b12bae1dc9a 100644 (file)
@@ -140,5 +140,12 @@ void test_range_float_ge_le (float f)
 {
   if (f >= 4)
     if (f <= 4)
-      __analyzer_eval (f == 4); /* { dg-warning "TRUE" } */
+      __analyzer_eval (f == 4); /* { dg-warning "TRUE" "PR 93356" { xfail *-*-* } } */
+  /* { dg-warning "UNKNOWN" "disabled float comparisons" { target *-*-* } .-1 } */
+}
+
+void test_float_selfcmp (float f)
+{
+  __analyzer_eval (f == f); /* { dg-warning "UNKNOWN" } */
+  __analyzer_eval (f != f); /* { dg-warning "UNKNOWN" } */
 }
index 91685f578a4f7b84adddc38659b379905b03818f..3f925941f8743be35c5d5d7a6d2a53bab66a1ddd 100644 (file)
@@ -209,14 +209,16 @@ void test_13 (struct outer *o)
 {
   __analyzer_eval (o->mid.in.f == 0.f); /* { dg-warning "UNKNOWN" } */
   o->mid.in.f = 0.f;
-  __analyzer_eval (o->mid.in.f == 0.f); /* { dg-warning "TRUE" } */
+  __analyzer_eval (o->mid.in.f == 0.f); /* { dg-warning "TRUE" "PR 93356" { xfail *-*-* } } */
+  /* { dg-warning "UNKNOWN" "disabled float comparisons" { target *-*-* } .-1 } */
 }
 
 void test_14 (struct outer o)
 {
   __analyzer_eval (o.mid.in.f == 0.f); /* { dg-warning "UNKNOWN" } */
   o.mid.in.f = 0.f;
-  __analyzer_eval (o.mid.in.f == 0.f); /* { dg-warning "TRUE" } */
+  __analyzer_eval (o.mid.in.f == 0.f); /* { dg-warning "TRUE" "PR 93356" { xfail *-*-* } } */
+  /* { dg-warning "UNKNOWN" "disabled float comparisons" { target *-*-* } .-1 } */
 }
 
 void test_15 (const char *str)
@@ -947,7 +949,8 @@ void test_42 (void)
   float f;
   i = 42;
   f = i;
-  __analyzer_eval (f == 42.0); /* { dg-warning "TRUE" } */
+  __analyzer_eval (f == 42.0); /* { dg-warning "TRUE" "PR 93356" { xfail *-*-* } } */
+  /* { dg-warning "UNKNOWN" "disabled float comparisons" { target *-*-* } .-1 } */
 }
 
 void test_43 (void)
diff --git a/gcc/testsuite/gcc.dg/analyzer/torture/pr93356.c b/gcc/testsuite/gcc.dg/analyzer/torture/pr93356.c
new file mode 100644 (file)
index 0000000..5db20d8
--- /dev/null
@@ -0,0 +1,6 @@
+void
+test (double d)
+{
+  if (__builtin_isnan (d))
+    return;
+}