[committed] analyzer: prevent ICE on isnan (PR 93290)

Message ID 20200117214311.6604-1-dmalcolm@redhat.com
State New
Headers show
Series
  • [committed] analyzer: prevent ICE on isnan (PR 93290)
Related show

Commit Message

David Malcolm Jan. 17, 2020, 9:43 p.m.
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.

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.

Pushed to master as 07c86323a199ca15177d99ad6c488b8f5fb5c729.

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/analyzer/region-model.cc            | 10 ++++++----
 gcc/doc/analyzer.texi                   |  3 +++
 gcc/testsuite/gcc.dg/analyzer/pr93290.c |  9 +++++++++
 3 files changed, 18 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/analyzer/pr93290.c

-- 
2.21.0

Patch

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/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;
+}