From e978955dd720d5cc0e5141a1e9bbbbb943a3cc41 Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Thu, 30 Jan 2020 12:35:46 -0500 Subject: [PATCH] analyzer: fix ICE in __builtin_isnan (PR 93356) 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 | 6 ++++++ gcc/analyzer/ChangeLog | 8 +++++++ gcc/analyzer/region-model.cc | 25 ++++++++++++++++++++++ gcc/doc/analyzer.texi | 2 ++ gcc/testsuite/ChangeLog | 14 +++++++++++- .../gcc.dg/analyzer/conditionals-notrans.c | 6 ++++++ gcc/testsuite/gcc.dg/analyzer/conditionals-trans.c | 9 +++++++- gcc/testsuite/gcc.dg/analyzer/data-model-1.c | 9 +++++--- gcc/testsuite/gcc.dg/analyzer/torture/pr93356.c | 6 ++++++ 9 files changed, 80 insertions(+), 5 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/analyzer/torture/pr93356.c diff --git a/gcc/ChangeLog b/gcc/ChangeLog index a45d090..4e312cb 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,9 @@ +2020-01-30 David Malcolm + + PR analyzer/93356 + * doc/analyzer.texi (Limitations): Note that constraints on + floating-point values are currently ignored. + 2020-01-30 Jakub Jelinek PR lto/93384 diff --git a/gcc/analyzer/ChangeLog b/gcc/analyzer/ChangeLog index f1ac6e6..1c981e4 100644 --- a/gcc/analyzer/ChangeLog +++ b/gcc/analyzer/ChangeLog @@ -1,5 +1,13 @@ 2020-01-30 David Malcolm + 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 + 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 diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc index c838454..a15088a 100644 --- a/gcc/analyzer/region-model.cc +++ b/gcc/analyzer/region-model.cc @@ -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)); } diff --git a/gcc/doc/analyzer.texi b/gcc/doc/analyzer.texi index 81acdd8..1fe4bce 100644 --- a/gcc/doc/analyzer.texi +++ b/gcc/doc/analyzer.texi @@ -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 diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 90eb0b2..621d428 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,4 +1,16 @@ -2020-01-30 Jeff Law + + 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 PR c/88660 * gcc.dg/pr88660.c: New test diff --git a/gcc/testsuite/gcc.dg/analyzer/conditionals-notrans.c b/gcc/testsuite/gcc.dg/analyzer/conditionals-notrans.c index 3b6e28c..a00127b 100644 --- a/gcc/testsuite/gcc.dg/analyzer/conditionals-notrans.c +++ b/gcc/testsuite/gcc.dg/analyzer/conditionals-notrans.c @@ -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" } */ +} diff --git a/gcc/testsuite/gcc.dg/analyzer/conditionals-trans.c b/gcc/testsuite/gcc.dg/analyzer/conditionals-trans.c index ab34618..f032789 100644 --- a/gcc/testsuite/gcc.dg/analyzer/conditionals-trans.c +++ b/gcc/testsuite/gcc.dg/analyzer/conditionals-trans.c @@ -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" } */ } diff --git a/gcc/testsuite/gcc.dg/analyzer/data-model-1.c b/gcc/testsuite/gcc.dg/analyzer/data-model-1.c index 91685f5..3f92594 100644 --- a/gcc/testsuite/gcc.dg/analyzer/data-model-1.c +++ b/gcc/testsuite/gcc.dg/analyzer/data-model-1.c @@ -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 index 0000000..5db20d8 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/torture/pr93356.c @@ -0,0 +1,6 @@ +void +test (double d) +{ + if (__builtin_isnan (d)) + return; +} -- 2.7.4