[gcc(refs/users/marxin/heads/marxin-gcc-benchmark-branch)] analyzer: prevent ICE on isnan (PR 93290)

[Martin Liska]

https://gcc.gnu.org/g:07c86323a199ca15177d99ad6c488b8f5fb5c729

commit 07c86323a199ca15177d99ad6c488b8f5fb5c729
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Thu Jan 16 09:46:30 2020 -0500

    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.

Diff:
---
 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(-)

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  <dmalcolm@redhat.com>
+
+	* doc/analyzer.texi (Limitations): Add note about NaN.
+
 2020-01-17  Mihail-Calin Ionescu <mihail.ionescu@arm.com>
 	    Sudakshina Das  <sudi.das@arm.com>
 
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  <dmalcolm@redhat.com>
+
+	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  <dmalcolm@redhat.com>
 
 	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  <dmalcolm@redhat.com>
+
+	PR analyzer/93290
+	* gcc.dg/analyzer/pr93290.c: New test.
+
 2020-01-17  Paolo Carlini  <paolo.carlini@oracle.com>
 
 	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 <math.h>
+
+int test_1 (void)
+{
+  float foo = 42.;
+  if (isnan (foo))
+    return 1;
+  return 0;
+}