From 07c86323a199ca15177d99ad6c488b8f5fb5c729 Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Thu, 16 Jan 2020 09:46:30 -0500 Subject: [PATCH] analyzer: prevent ICE on isnan (PR 93290) PR analyzer/93290 reports an ICE on calls to isnan(). The root cause is that an UNORDERED_EXPR is passed to region_model::eval_condition_without_cm, and there's a stray gcc_unreachable () in the case where we're comparing an svalue against itself. I attempted a more involved patch that properly handled NaN in general but it seems I've baked the assumption of reflexivity too deeply into the constraint_manager code. For now, this patch avoids the ICE and documents the limitation. gcc/analyzer/ChangeLog: PR analyzer/93290 * region-model.cc (region_model::eval_condition_without_cm): Avoid gcc_unreachable for unexpected operations for the case where we're comparing an svalue against itself. gcc/ChangeLog * doc/analyzer.texi (Limitations): Add note about NaN. gcc/testsuite/ChangeLog: PR analyzer/93290 * gcc.dg/analyzer/pr93290.c: New test. --- gcc/ChangeLog | 4 ++++ gcc/analyzer/ChangeLog | 7 +++++++ gcc/analyzer/region-model.cc | 10 ++++++---- gcc/doc/analyzer.texi | 3 +++ gcc/testsuite/ChangeLog | 5 +++++ gcc/testsuite/gcc.dg/analyzer/pr93290.c | 9 +++++++++ 6 files changed, 34 insertions(+), 4 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/analyzer/pr93290.c diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 7986c68e533..d837e95509a 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,7 @@ +2020-01-17 David Malcolm + + * doc/analyzer.texi (Limitations): Add note about NaN. + 2020-01-17 Mihail-Calin Ionescu Sudakshina Das diff --git a/gcc/analyzer/ChangeLog b/gcc/analyzer/ChangeLog index e8090184216..3e6e21b2740 100644 --- a/gcc/analyzer/ChangeLog +++ b/gcc/analyzer/ChangeLog @@ -1,3 +1,10 @@ +2020-01-17 David Malcolm + + PR analyzer/93290 + * region-model.cc (region_model::eval_condition_without_cm): Avoid + gcc_unreachable for unexpected operations for the case where + we're comparing an svalue against itself. + 2020-01-17 David Malcolm PR analyzer/93281 diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc index f67572e2d45..1e0be312e03 100644 --- a/gcc/analyzer/region-model.cc +++ b/gcc/analyzer/region-model.cc @@ -5189,13 +5189,11 @@ region_model::eval_condition_without_cm (svalue_id lhs_sid, { if (lhs == rhs) { - /* If we have the same svalue, then we have equality. + /* If we have the same svalue, then we have equality + (apart from NaN-handling). TODO: should this definitely be the case for poisoned values? */ switch (op) { - default: - gcc_unreachable (); - case EQ_EXPR: case GE_EXPR: case LE_EXPR: @@ -5205,6 +5203,10 @@ region_model::eval_condition_without_cm (svalue_id lhs_sid, case GT_EXPR: case LT_EXPR: return tristate::TS_FALSE; + + default: + /* For other ops, use the logic below. */ + break; } } diff --git a/gcc/doc/analyzer.texi b/gcc/doc/analyzer.texi index b4e9b01da2e..81acdd8998b 100644 --- a/gcc/doc/analyzer.texi +++ b/gcc/doc/analyzer.texi @@ -388,6 +388,9 @@ The implementation of call summaries is currently very simplistic. @item 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. +@item The region model code creates lots of little mutable objects at each @code{region_model} (and thus per @code{exploded_node}) rather than sharing immutable objects and having the mutable state in the diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index d3a89a3ac72..95e4e344ee0 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2020-01-17 David Malcolm + + PR analyzer/93290 + * gcc.dg/analyzer/pr93290.c: New test. + 2020-01-17 Paolo Carlini PR c++/92542 diff --git a/gcc/testsuite/gcc.dg/analyzer/pr93290.c b/gcc/testsuite/gcc.dg/analyzer/pr93290.c new file mode 100644 index 00000000000..fa35629d955 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/pr93290.c @@ -0,0 +1,9 @@ +#include + +int test_1 (void) +{ + float foo = 42.; + if (isnan (foo)) + return 1; + return 0; +} -- 2.30.2