Check for overflow in tree-switch-conversion (PR middle-end/90478).

Message ID 75068b7a-f6f0-7a59-0fb7-d78fafa92d25@suse.cz
State New
Headers show
Series
  • Check for overflow in tree-switch-conversion (PR middle-end/90478).
Related show

Commit Message

Martin Liška May 15, 2019, 10:20 a.m.
Hi.

The patch is about handling of overflow in jump_table_cluster::can_be_handled.
I calculate 100 * range and compare it to range. If it's smaller, then
overflow happens.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready to be installed?
Thanks,
Martin

gcc/ChangeLog:

2019-05-15  Martin Liska  <mliska@suse.cz>

	PR middle-end/90478
	* tree-switch-conversion.c (jump_table_cluster::can_be_handled):
	Check for overflow.

gcc/testsuite/ChangeLog:

2019-05-15  Martin Liska  <mliska@suse.cz>

	PR middle-end/90478
	* gcc.dg/tree-ssa/pr90478-2.c: New test.
	* gcc.dg/tree-ssa/pr90478.c: New test.
---
 gcc/testsuite/gcc.dg/tree-ssa/pr90478-2.c | 17 +++++++++++++++++
 gcc/testsuite/gcc.dg/tree-ssa/pr90478.c   | 18 ++++++++++++++++++
 gcc/tree-switch-conversion.c              |  6 +++++-
 3 files changed, 40 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr90478-2.c
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr90478.c

Comments

Richard Biener May 15, 2019, 11:21 a.m. | #1
On Wed, May 15, 2019 at 12:21 PM Martin Liška <mliska@suse.cz> wrote:
>

> Hi.

>

> The patch is about handling of overflow in jump_table_cluster::can_be_handled.

> I calculate 100 * range and compare it to range. If it's smaller, then

> overflow happens.

>

> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

>

> Ready to be installed?


Looks good, but can the RHS overflow as well?  I suppose changing
the compute/compare to uint64_t would fix the overflow but downstream
the issue would re-appear?

Richard.

> Thanks,

> Martin

>

> gcc/ChangeLog:

>

> 2019-05-15  Martin Liska  <mliska@suse.cz>

>

>         PR middle-end/90478

>         * tree-switch-conversion.c (jump_table_cluster::can_be_handled):

>         Check for overflow.

>

> gcc/testsuite/ChangeLog:

>

> 2019-05-15  Martin Liska  <mliska@suse.cz>

>

>         PR middle-end/90478

>         * gcc.dg/tree-ssa/pr90478-2.c: New test.

>         * gcc.dg/tree-ssa/pr90478.c: New test.

> ---

>  gcc/testsuite/gcc.dg/tree-ssa/pr90478-2.c | 17 +++++++++++++++++

>  gcc/testsuite/gcc.dg/tree-ssa/pr90478.c   | 18 ++++++++++++++++++

>  gcc/tree-switch-conversion.c              |  6 +++++-

>  3 files changed, 40 insertions(+), 1 deletion(-)

>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr90478-2.c

>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr90478.c

>

>
Martin Liška May 15, 2019, 11:30 a.m. | #2
On 5/15/19 1:21 PM, Richard Biener wrote:
> On Wed, May 15, 2019 at 12:21 PM Martin Liška <mliska@suse.cz> wrote:

>>

>> Hi.

>>

>> The patch is about handling of overflow in jump_table_cluster::can_be_handled.

>> I calculate 100 * range and compare it to range. If it's smaller, then

>> overflow happens.

>>

>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

>>

>> Ready to be installed?

> 

> Looks good, but can the RHS overflow as well? 


That should be fine as maximal value of the PARAM is 1<32. And if I'm correct
HOST_WIDE_INT should have 64-bits?

Martin

> I suppose changing

> the compute/compare to uint64_t would fix the overflow but downstream

> the issue would re-appear?

> 

> Richard.

> 

>> Thanks,

>> Martin

>>

>> gcc/ChangeLog:

>>

>> 2019-05-15  Martin Liska  <mliska@suse.cz>

>>

>>         PR middle-end/90478

>>         * tree-switch-conversion.c (jump_table_cluster::can_be_handled):

>>         Check for overflow.

>>

>> gcc/testsuite/ChangeLog:

>>

>> 2019-05-15  Martin Liska  <mliska@suse.cz>

>>

>>         PR middle-end/90478

>>         * gcc.dg/tree-ssa/pr90478-2.c: New test.

>>         * gcc.dg/tree-ssa/pr90478.c: New test.

>> ---

>>  gcc/testsuite/gcc.dg/tree-ssa/pr90478-2.c | 17 +++++++++++++++++

>>  gcc/testsuite/gcc.dg/tree-ssa/pr90478.c   | 18 ++++++++++++++++++

>>  gcc/tree-switch-conversion.c              |  6 +++++-

>>  3 files changed, 40 insertions(+), 1 deletion(-)

>>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr90478-2.c

>>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr90478.c

>>

>>
Richard Biener May 15, 2019, 12:23 p.m. | #3
On Wed, May 15, 2019 at 1:30 PM Martin Liška <mliska@suse.cz> wrote:
>

> On 5/15/19 1:21 PM, Richard Biener wrote:

> > On Wed, May 15, 2019 at 12:21 PM Martin Liška <mliska@suse.cz> wrote:

> >>

> >> Hi.

> >>

> >> The patch is about handling of overflow in jump_table_cluster::can_be_handled.

> >> I calculate 100 * range and compare it to range. If it's smaller, then

> >> overflow happens.

> >>

> >> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

> >>

> >> Ready to be installed?

> >

> > Looks good, but can the RHS overflow as well?

>

> That should be fine as maximal value of the PARAM is 1<32. And if I'm correct

> HOST_WIDE_INT should have 64-bits?


Yes.

> Martin

>

> > I suppose changing

> > the compute/compare to uint64_t would fix the overflow but downstream

> > the issue would re-appear?

> >

> > Richard.

> >

> >> Thanks,

> >> Martin

> >>

> >> gcc/ChangeLog:

> >>

> >> 2019-05-15  Martin Liska  <mliska@suse.cz>

> >>

> >>         PR middle-end/90478

> >>         * tree-switch-conversion.c (jump_table_cluster::can_be_handled):

> >>         Check for overflow.

> >>

> >> gcc/testsuite/ChangeLog:

> >>

> >> 2019-05-15  Martin Liska  <mliska@suse.cz>

> >>

> >>         PR middle-end/90478

> >>         * gcc.dg/tree-ssa/pr90478-2.c: New test.

> >>         * gcc.dg/tree-ssa/pr90478.c: New test.

> >> ---

> >>  gcc/testsuite/gcc.dg/tree-ssa/pr90478-2.c | 17 +++++++++++++++++

> >>  gcc/testsuite/gcc.dg/tree-ssa/pr90478.c   | 18 ++++++++++++++++++

> >>  gcc/tree-switch-conversion.c              |  6 +++++-

> >>  3 files changed, 40 insertions(+), 1 deletion(-)

> >>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr90478-2.c

> >>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr90478.c

> >>

> >>

>

Patch

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr90478-2.c b/gcc/testsuite/gcc.dg/tree-ssa/pr90478-2.c
new file mode 100644
index 00000000000..f0fc103a888
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr90478-2.c
@@ -0,0 +1,17 @@ 
+/* { dg-do compile } */
+/* { dg-options "-Os --param jump-table-max-growth-ratio-for-size=2147483647" } */
+
+long
+foo (long x, long y)
+{
+  x = x & y;
+  switch (y)
+    {
+    case 63L: x >>= 0; break;
+    case 4032L: x >>= 6; break;
+    case 258048L: x >>= 12; break;
+    case 16515072L: x >>= 18; break;
+    default: __builtin_unreachable ();
+    }
+  return x;
+}
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr90478.c b/gcc/testsuite/gcc.dg/tree-ssa/pr90478.c
new file mode 100644
index 00000000000..f19808d2941
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr90478.c
@@ -0,0 +1,18 @@ 
+/* { dg-do compile } */
+
+typedef struct {
+  long a;
+} c;
+
+void e();
+
+void d() {
+  c *b;
+  switch (b->a)
+  case 8:
+  case 2:
+  case 2057594037927936:
+  case 0:
+  case 4611686018427387904:
+    e();
+}
diff --git a/gcc/tree-switch-conversion.c b/gcc/tree-switch-conversion.c
index bedeb2fd865..5f8ed46f496 100644
--- a/gcc/tree-switch-conversion.c
+++ b/gcc/tree-switch-conversion.c
@@ -1284,7 +1284,11 @@  jump_table_cluster::can_be_handled (const vec<cluster *> &clusters,
       comparison_count += sc->m_range_p ? 2 : 1;
     }
 
-  return 100 * range <= max_ratio * comparison_count;
+  unsigned HOST_WIDE_INT lhs = 100 * range;
+  if (lhs < range)
+    return false;
+
+  return lhs <= max_ratio * comparison_count;
 }
 
 /* Return true if cluster starting at START and ending at END (inclusive)