[gcc(refs/users/marxin/heads/marxin-gcc-benchmark-branch)] analyzer: fix qsort issue with array_region keys (PR 93352)

Martin Liska marxin@gcc.gnu.org
Mon Mar 30 10:20:16 GMT 2020


https://gcc.gnu.org/g:4f01e5778689977c9569477947b8062d8d866667

commit 4f01e5778689977c9569477947b8062d8d866667
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Tue Jan 21 12:42:36 2020 -0500

    analyzer: fix qsort issue with array_region keys (PR 93352)
    
    PR analyzer/93352 reports a qsort failure
      "comparator not anti-symmetric: -2147483648, -2147483648)"
    within the analyzer on code involving an array access of [0x7fffffff + 1].
    
    The issue is that array_region (which uses int for keys into known
    values in the array) uses subtraction to implement int_cmp for sorting
    the keys, which isn't going to work for boundary values.
    
    Potentially a wider type should be used, but for now this patch fixes
    the ICE by using explicit comparisons rather than subtraction to
    implement the qsort callback.
    
    gcc/analyzer/ChangeLog:
            PR analyzer/93352
            * region-model.cc (int_cmp): Rename to...
            (array_region::key_cmp): ...this, using key_t rather than int.
            Rewrite in terms of comparisons rather than subtraction to
            ensure qsort is anti-symmetric when handling extreme values.
            (array_region::walk_for_canonicalization): Update for above
            renaming.
            * region-model.h (array_region::key_cmp): New decl.
    
    gcc/testsuite/ChangeLog:
            PR analyzer/93352
            * gcc.dg/analyzer/pr93352.c: New test.

Diff:
---
 gcc/analyzer/ChangeLog                  | 11 +++++++++++
 gcc/analyzer/region-model.cc            | 19 ++++++++++++-------
 gcc/analyzer/region-model.h             |  2 ++
 gcc/testsuite/ChangeLog                 |  5 +++++
 gcc/testsuite/gcc.dg/analyzer/pr93352.c | 12 ++++++++++++
 5 files changed, 42 insertions(+), 7 deletions(-)

diff --git a/gcc/analyzer/ChangeLog b/gcc/analyzer/ChangeLog
index 3e6e21b2740..b49f43bf688 100644
--- a/gcc/analyzer/ChangeLog
+++ b/gcc/analyzer/ChangeLog
@@ -1,3 +1,14 @@
+2020-01-21  David Malcolm  <dmalcolm@redhat.com>
+
+	PR analyzer/93352
+	* region-model.cc (int_cmp): Rename to...
+	(array_region::key_cmp): ...this, using key_t rather than int.
+	Rewrite in terms of comparisons rather than subtraction to
+	ensure qsort is anti-symmetric when handling extreme values.
+	(array_region::walk_for_canonicalization): Update for above
+	renaming.
+	* region-model.h (array_region::key_cmp): New decl.
+
 2020-01-17  David Malcolm  <dmalcolm@redhat.com>
 
 	PR analyzer/93290
diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index 1e0be312e03..9474c6737d5 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -2387,15 +2387,20 @@ array_region::get_key_for_child_region (region_id child_rid, key_t *out) const
   return false;
 }
 
-/* qsort comparator for int.  */
+/* qsort comparator for array_region's keys.  */
 
-static int
-int_cmp (const void *p1, const void *p2)
+int
+array_region::key_cmp (const void *p1, const void *p2)
 {
-  int i1 = *(const int *)p1;
-  int i2 = *(const int *)p2;
+  key_t i1 = *(const key_t *)p1;
+  key_t i2 = *(const key_t *)p2;
 
-  return i1 - i2;
+  if (i1 > i2)
+    return 1;
+  else if (i1 < i2)
+    return -1;
+  else
+    return 0;
 }
 
 /* Implementation of region::walk_for_canonicalization vfunc for
@@ -2412,7 +2417,7 @@ array_region::walk_for_canonicalization (canonicalization *c) const
       int key_a = (*iter).first;
       keys.quick_push (key_a);
     }
-  keys.qsort (int_cmp);
+  keys.qsort (key_cmp);
 
   unsigned i;
   int key;
diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h
index eea808e61eb..c9c8265e619 100644
--- a/gcc/analyzer/region-model.h
+++ b/gcc/analyzer/region-model.h
@@ -1319,6 +1319,8 @@ public:
   static key_t key_from_constant (tree cst);
 
  private:
+  static int key_cmp (const void *, const void *);
+
   /* Mapping from tree to child region.  */
   map_t m_map;
 };
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 2fffb91bd15..968145362c2 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2020-01-21  David Malcolm  <dmalcolm@redhat.com>
+
+	PR analyzer/93352
+	* gcc.dg/analyzer/pr93352.c: New test.
+
 2020-01-22  Hans-Peter Nilsson  <hp@axis.com>
 
 	* gcc.target/cris/asm-v8.S, gcc.target/cris/inasm-v8.c,
diff --git a/gcc/testsuite/gcc.dg/analyzer/pr93352.c b/gcc/testsuite/gcc.dg/analyzer/pr93352.c
new file mode 100644
index 00000000000..ccb96d0eaed
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/pr93352.c
@@ -0,0 +1,12 @@
+/* { dg-additional-options "-Wno-overflow" } */
+
+struct yc {
+  int c0;
+  char di[];
+};
+
+void
+qt (struct yc *ab)
+{
+  ab->di[0x7fffffff + 1] = ab->di[0];
+}


More information about the Gcc-cvs mailing list