* [PATCH] Fix UBSAN errors in dse.c (PR rtl-optimization/82044).
@ 2017-09-20 7:51 Martin Liška
2017-09-20 8:15 ` Jakub Jelinek
0 siblings, 1 reply; 14+ messages in thread
From: Martin Liška @ 2017-09-20 7:51 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 546 bytes --]
Hello.
Following patch handles UBSAN (overflow) in dce.c.
Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
Ready to be installed?
Martin
gcc/ChangeLog:
2017-09-11 Martin Liska <mliska@suse.cz>
PR rtl-optimization/82044
PR tree-optimization/82042
* dse.c (set_usage_bits): Check properly for a big offset
value.
(record_store): Do not overflow and set maximum value.
(check_mem_read_rtx): Bail out for a big offset.
---
gcc/dse.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
[-- Attachment #2: 0001-Fix-UBSAN-errors-in-dse.c-PR-rtl-optimization-82044.patch --]
[-- Type: text/x-patch, Size: 1263 bytes --]
diff --git a/gcc/dse.c b/gcc/dse.c
index cff3ac47356..d519ac70ed5 100644
--- a/gcc/dse.c
+++ b/gcc/dse.c
@@ -929,7 +929,9 @@ set_usage_bits (group_info *group, HOST_WIDE_INT offset, HOST_WIDE_INT width,
{
HOST_WIDE_INT i;
bool expr_escapes = can_escape (expr);
- if (offset > -MAX_OFFSET && offset + width < MAX_OFFSET)
+ if (offset > -MAX_OFFSET
+ && offset < MAX_OFFSET
+ && offset + width < MAX_OFFSET)
for (i=offset; i<offset+width; i++)
{
bitmap store1;
@@ -1536,7 +1538,11 @@ record_store (rtx body, bb_info_t bb_info)
}
store_info->group_id = group_id;
store_info->begin = offset;
- store_info->end = offset + width;
+ if (offset > HOST_WIDE_INT_MAX - width)
+ store_info->end = HOST_WIDE_INT_MAX;
+ else
+ store_info->end = offset + width;
+
store_info->is_set = GET_CODE (body) == SET;
store_info->rhs = rhs;
store_info->const_rhs = const_rhs;
@@ -1976,6 +1982,14 @@ check_mem_read_rtx (rtx *loc, bb_info_t bb_info)
return;
}
+ if (offset > MAX_OFFSET)
+ {
+ if (dump_file && (dump_flags & TDF_DETAILS))
+ fprintf (dump_file, " reaches MAX_OFFSET.\n");
+ add_wild_read (bb_info);
+ return;
+ }
+
if (GET_MODE (mem) == BLKmode)
width = -1;
else
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix UBSAN errors in dse.c (PR rtl-optimization/82044).
2017-09-20 7:51 [PATCH] Fix UBSAN errors in dse.c (PR rtl-optimization/82044) Martin Liška
@ 2017-09-20 8:15 ` Jakub Jelinek
2017-10-19 11:58 ` Martin Liška
0 siblings, 1 reply; 14+ messages in thread
From: Jakub Jelinek @ 2017-09-20 8:15 UTC (permalink / raw)
To: Martin Liška; +Cc: gcc-patches
On Wed, Sep 20, 2017 at 09:50:32AM +0200, Martin Liška wrote:
> Hello.
>
> Following patch handles UBSAN (overflow) in dce.c.
dse.c ;)
> --- a/gcc/dse.c
> +++ b/gcc/dse.c
> @@ -929,7 +929,9 @@ set_usage_bits (group_info *group, HOST_WIDE_INT offset, HOST_WIDE_INT width,
> {
> HOST_WIDE_INT i;
> bool expr_escapes = can_escape (expr);
> - if (offset > -MAX_OFFSET && offset + width < MAX_OFFSET)
> + if (offset > -MAX_OFFSET
> + && offset < MAX_OFFSET
> + && offset + width < MAX_OFFSET)
This can still overflow if width is close to HOST_WIDE_INT_MAX.
Anyway, I don't remember this code too much, but wonder if either offset or
width or their sum is outside of the -MAX_OFFSET, MAX_OFFSET range if we
still don't want to record usage bits at least in the intersection of
-MAX_OFFSET, MAX_OFFSET and offset, offset + width (the latter performed
with infinite precision; though, if record_store is changed as suggested
below, offset + width shouldn't overflow).
> for (i=offset; i<offset+width; i++)
> {
> bitmap store1;
> @@ -1536,7 +1538,11 @@ record_store (rtx body, bb_info_t bb_info)
> }
> store_info->group_id = group_id;
> store_info->begin = offset;
> - store_info->end = offset + width;
> + if (offset > HOST_WIDE_INT_MAX - width)
> + store_info->end = HOST_WIDE_INT_MAX;
> + else
> + store_info->end = offset + width;
If offset + width overflows, I think we risk wrong-code by doing this, plus
there are 3 other offset + width computations earlier in record_store
before we reach this. I think instead we should treat such cases as wild
stores early, i.e.:
if (!canon_address (mem, &group_id, &offset, &base))
{
clear_rhs_from_active_local_stores ();
return 0;
}
if (GET_MODE (mem) == BLKmode)
width = MEM_SIZE (mem);
else
width = GET_MODE_SIZE (GET_MODE (mem));
+ if (offset > HOST_WIDE_INT_MAX - width)
+ {
+ clear_rhs_from_active_local_stores ();
+ return 0;
+ }
or so.
> +
> store_info->is_set = GET_CODE (body) == SET;
> store_info->rhs = rhs;
> store_info->const_rhs = const_rhs;
> @@ -1976,6 +1982,14 @@ check_mem_read_rtx (rtx *loc, bb_info_t bb_info)
> return;
> }
>
> + if (offset > MAX_OFFSET)
> + {
> + if (dump_file && (dump_flags & TDF_DETAILS))
> + fprintf (dump_file, " reaches MAX_OFFSET.\n");
> + add_wild_read (bb_info);
> + return;
> + }
> +
Is offset > MAX_OFFSET really problematic (and not just the width != -1 &&
offset + width overflowing case)?
> if (GET_MODE (mem) == BLKmode)
> width = -1;
> else
>
Jakub
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix UBSAN errors in dse.c (PR rtl-optimization/82044).
2017-09-20 8:15 ` Jakub Jelinek
@ 2017-10-19 11:58 ` Martin Liška
2017-11-02 13:15 ` Martin Liška
0 siblings, 1 reply; 14+ messages in thread
From: Martin Liška @ 2017-10-19 11:58 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On 09/20/2017 10:15 AM, Jakub Jelinek wrote:
> On Wed, Sep 20, 2017 at 09:50:32AM +0200, Martin Liška wrote:
>> Hello.
>>
>> Following patch handles UBSAN (overflow) in dce.c.
>
> dse.c ;)
>
>> --- a/gcc/dse.c
>> +++ b/gcc/dse.c
>> @@ -929,7 +929,9 @@ set_usage_bits (group_info *group, HOST_WIDE_INT offset, HOST_WIDE_INT width,
>> {
>> HOST_WIDE_INT i;
>> bool expr_escapes = can_escape (expr);
>> - if (offset > -MAX_OFFSET && offset + width < MAX_OFFSET)
>> + if (offset > -MAX_OFFSET
>> + && offset < MAX_OFFSET
>> + && offset + width < MAX_OFFSET)
>
> This can still overflow if width is close to HOST_WIDE_INT_MAX.
> Anyway, I don't remember this code too much, but wonder if either offset or
> width or their sum is outside of the -MAX_OFFSET, MAX_OFFSET range if we
> still don't want to record usage bits at least in the intersection of
> -MAX_OFFSET, MAX_OFFSET and offset, offset + width (the latter performed
> with infinite precision; though, if record_store is changed as suggested
> below, offset + width shouldn't overflow).
>
>> for (i=offset; i<offset+width; i++)
>> {
>> bitmap store1;
>> @@ -1536,7 +1538,11 @@ record_store (rtx body, bb_info_t bb_info)
>> }
>> store_info->group_id = group_id;
>> store_info->begin = offset;
>> - store_info->end = offset + width;
>> + if (offset > HOST_WIDE_INT_MAX - width)
>> + store_info->end = HOST_WIDE_INT_MAX;
>> + else
>> + store_info->end = offset + width;
>
> If offset + width overflows, I think we risk wrong-code by doing this, plus
> there are 3 other offset + width computations earlier in record_store
> before we reach this. I think instead we should treat such cases as wild
> stores early, i.e.:
> if (!canon_address (mem, &group_id, &offset, &base))
> {
> clear_rhs_from_active_local_stores ();
> return 0;
> }
>
> if (GET_MODE (mem) == BLKmode)
> width = MEM_SIZE (mem);
> else
> width = GET_MODE_SIZE (GET_MODE (mem));
>
> + if (offset > HOST_WIDE_INT_MAX - width)
> + {
> + clear_rhs_from_active_local_stores ();
> + return 0;
> + }
>
> or so.
>
>> +
>> store_info->is_set = GET_CODE (body) == SET;
>> store_info->rhs = rhs;
>> store_info->const_rhs = const_rhs;
>> @@ -1976,6 +1982,14 @@ check_mem_read_rtx (rtx *loc, bb_info_t bb_info)
>> return;
>> }
>>
>> + if (offset > MAX_OFFSET)
>> + {
>> + if (dump_file && (dump_flags & TDF_DETAILS))
>> + fprintf (dump_file, " reaches MAX_OFFSET.\n");
>> + add_wild_read (bb_info);
>> + return;
>> + }
>> +
Hi.
The later one works for me. I'm going to regtest that.
Ready after it survives regression tests?
Thanks,
Martin
>
> Is offset > MAX_OFFSET really problematic (and not just the width != -1 &&
> offset + width overflowing case)?
>
>> if (GET_MODE (mem) == BLKmode)
>> width = -1;
>> else
>>
>
>
> Jakub
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix UBSAN errors in dse.c (PR rtl-optimization/82044).
2017-10-19 11:58 ` Martin Liška
@ 2017-11-02 13:15 ` Martin Liška
2017-11-08 16:42 ` Jeff Law
0 siblings, 1 reply; 14+ messages in thread
From: Martin Liška @ 2017-11-02 13:15 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
PING^1
On 10/19/2017 01:36 PM, Martin Liška wrote:
> On 09/20/2017 10:15 AM, Jakub Jelinek wrote:
>> On Wed, Sep 20, 2017 at 09:50:32AM +0200, Martin Liška wrote:
>>> Hello.
>>>
>>> Following patch handles UBSAN (overflow) in dce.c.
>>
>> dse.c ;)
>>
>>> --- a/gcc/dse.c
>>> +++ b/gcc/dse.c
>>> @@ -929,7 +929,9 @@ set_usage_bits (group_info *group, HOST_WIDE_INT offset, HOST_WIDE_INT width,
>>> {
>>> HOST_WIDE_INT i;
>>> bool expr_escapes = can_escape (expr);
>>> - if (offset > -MAX_OFFSET && offset + width < MAX_OFFSET)
>>> + if (offset > -MAX_OFFSET
>>> + && offset < MAX_OFFSET
>>> + && offset + width < MAX_OFFSET)
>>
>> This can still overflow if width is close to HOST_WIDE_INT_MAX.
>> Anyway, I don't remember this code too much, but wonder if either offset or
>> width or their sum is outside of the -MAX_OFFSET, MAX_OFFSET range if we
>> still don't want to record usage bits at least in the intersection of
>> -MAX_OFFSET, MAX_OFFSET and offset, offset + width (the latter performed
>> with infinite precision; though, if record_store is changed as suggested
>> below, offset + width shouldn't overflow).
>>
>>> for (i=offset; i<offset+width; i++)
>>> {
>>> bitmap store1;
>>> @@ -1536,7 +1538,11 @@ record_store (rtx body, bb_info_t bb_info)
>>> }
>>> store_info->group_id = group_id;
>>> store_info->begin = offset;
>>> - store_info->end = offset + width;
>>> + if (offset > HOST_WIDE_INT_MAX - width)
>>> + store_info->end = HOST_WIDE_INT_MAX;
>>> + else
>>> + store_info->end = offset + width;
>>
>> If offset + width overflows, I think we risk wrong-code by doing this, plus
>> there are 3 other offset + width computations earlier in record_store
>> before we reach this. I think instead we should treat such cases as wild
>> stores early, i.e.:
>> if (!canon_address (mem, &group_id, &offset, &base))
>> {
>> clear_rhs_from_active_local_stores ();
>> return 0;
>> }
>>
>> if (GET_MODE (mem) == BLKmode)
>> width = MEM_SIZE (mem);
>> else
>> width = GET_MODE_SIZE (GET_MODE (mem));
>>
>> + if (offset > HOST_WIDE_INT_MAX - width)
>> + {
>> + clear_rhs_from_active_local_stores ();
>> + return 0;
>> + }
>>
>> or so.
>>
>>> +
>>> store_info->is_set = GET_CODE (body) == SET;
>>> store_info->rhs = rhs;
>>> store_info->const_rhs = const_rhs;
>>> @@ -1976,6 +1982,14 @@ check_mem_read_rtx (rtx *loc, bb_info_t bb_info)
>>> return;
>>> }
>>>
>>> + if (offset > MAX_OFFSET)
>>> + {
>>> + if (dump_file && (dump_flags & TDF_DETAILS))
>>> + fprintf (dump_file, " reaches MAX_OFFSET.\n");
>>> + add_wild_read (bb_info);
>>> + return;
>>> + }
>>> +
>
> Hi.
>
> The later one works for me. I'm going to regtest that.
>
> Ready after it survives regression tests?
>
> Thanks,
> Martin
>
>>
>> Is offset > MAX_OFFSET really problematic (and not just the width != -1 &&
>> offset + width overflowing case)?
>>
>>> if (GET_MODE (mem) == BLKmode)
>>> width = -1;
>>> else
>>>
>>
>>
>> Jakub
>>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix UBSAN errors in dse.c (PR rtl-optimization/82044).
2017-11-02 13:15 ` Martin Liška
@ 2017-11-08 16:42 ` Jeff Law
2017-11-15 7:34 ` Martin Liška
0 siblings, 1 reply; 14+ messages in thread
From: Jeff Law @ 2017-11-08 16:42 UTC (permalink / raw)
To: Martin Liška, Jakub Jelinek; +Cc: gcc-patches
On 11/02/2017 07:15 AM, Martin Liška wrote:
> PING^1
I don't see an updated patch in this thread? THe last message I see is
this one where you indicate you're going to tweak the patch and re-test.
Jeff
>
> On 10/19/2017 01:36 PM, Martin Liška wrote:
>> On 09/20/2017 10:15 AM, Jakub Jelinek wrote:
>>> On Wed, Sep 20, 2017 at 09:50:32AM +0200, Martin Liška wrote:
>>>> Hello.
>>>>
>>>> Following patch handles UBSAN (overflow) in dce.c.
>>>
>>> dse.c ;)
>>>
>>>> --- a/gcc/dse.c
>>>> +++ b/gcc/dse.c
>>>> @@ -929,7 +929,9 @@ set_usage_bits (group_info *group, HOST_WIDE_INT offset, HOST_WIDE_INT width,
>>>> {
>>>> HOST_WIDE_INT i;
>>>> bool expr_escapes = can_escape (expr);
>>>> - if (offset > -MAX_OFFSET && offset + width < MAX_OFFSET)
>>>> + if (offset > -MAX_OFFSET
>>>> + && offset < MAX_OFFSET
>>>> + && offset + width < MAX_OFFSET)
>>>
>>> This can still overflow if width is close to HOST_WIDE_INT_MAX.
>>> Anyway, I don't remember this code too much, but wonder if either offset or
>>> width or their sum is outside of the -MAX_OFFSET, MAX_OFFSET range if we
>>> still don't want to record usage bits at least in the intersection of
>>> -MAX_OFFSET, MAX_OFFSET and offset, offset + width (the latter performed
>>> with infinite precision; though, if record_store is changed as suggested
>>> below, offset + width shouldn't overflow).
>>>
>>>> for (i=offset; i<offset+width; i++)
>>>> {
>>>> bitmap store1;
>>>> @@ -1536,7 +1538,11 @@ record_store (rtx body, bb_info_t bb_info)
>>>> }
>>>> store_info->group_id = group_id;
>>>> store_info->begin = offset;
>>>> - store_info->end = offset + width;
>>>> + if (offset > HOST_WIDE_INT_MAX - width)
>>>> + store_info->end = HOST_WIDE_INT_MAX;
>>>> + else
>>>> + store_info->end = offset + width;
>>>
>>> If offset + width overflows, I think we risk wrong-code by doing this, plus
>>> there are 3 other offset + width computations earlier in record_store
>>> before we reach this. I think instead we should treat such cases as wild
>>> stores early, i.e.:
>>> if (!canon_address (mem, &group_id, &offset, &base))
>>> {
>>> clear_rhs_from_active_local_stores ();
>>> return 0;
>>> }
>>>
>>> if (GET_MODE (mem) == BLKmode)
>>> width = MEM_SIZE (mem);
>>> else
>>> width = GET_MODE_SIZE (GET_MODE (mem));
>>>
>>> + if (offset > HOST_WIDE_INT_MAX - width)
>>> + {
>>> + clear_rhs_from_active_local_stores ();
>>> + return 0;
>>> + }
>>>
>>> or so.
>>>
>>>> +
>>>> store_info->is_set = GET_CODE (body) == SET;
>>>> store_info->rhs = rhs;
>>>> store_info->const_rhs = const_rhs;
>>>> @@ -1976,6 +1982,14 @@ check_mem_read_rtx (rtx *loc, bb_info_t bb_info)
>>>> return;
>>>> }
>>>>
>>>> + if (offset > MAX_OFFSET)
>>>> + {
>>>> + if (dump_file && (dump_flags & TDF_DETAILS))
>>>> + fprintf (dump_file, " reaches MAX_OFFSET.\n");
>>>> + add_wild_read (bb_info);
>>>> + return;
>>>> + }
>>>> +
>>
>> Hi.
>>
>> The later one works for me. I'm going to regtest that.
>>
>> Ready after it survives regression tests?
>>
>> Thanks,
>> Martin
>>
>>>
>>> Is offset > MAX_OFFSET really problematic (and not just the width != -1 &&
>>> offset + width overflowing case)?
>>>
>>>> if (GET_MODE (mem) == BLKmode)
>>>> width = -1;
>>>> else
>>>>
>>>
>>>
>>> Jakub
>>>
>>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix UBSAN errors in dse.c (PR rtl-optimization/82044).
2017-11-08 16:42 ` Jeff Law
@ 2017-11-15 7:34 ` Martin Liška
2017-11-17 0:57 ` Jeff Law
2017-11-22 0:27 ` [PATCH] Fix i?86 bootstrap " Jakub Jelinek
0 siblings, 2 replies; 14+ messages in thread
From: Martin Liška @ 2017-11-15 7:34 UTC (permalink / raw)
To: Jeff Law, Jakub Jelinek; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 251 bytes --]
On 11/08/2017 05:31 PM, Jeff Law wrote:
> I don't see an updated patch in this thread? THe last message I see is
> this one where you indicate you're going to tweak the patch and re-test.
>
> Jeff
Yes, I tweaked and tested following patch.
Martin
[-- Attachment #2: 0001-Fix-UBSAN-errors-in-dse.c-PR-rtl-optimization-82044.patch --]
[-- Type: text/x-patch, Size: 910 bytes --]
From a369ac78b887e219a375e17d6817c1f744e71779 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Thu, 19 Oct 2017 13:38:01 +0200
Subject: [PATCH] Fix UBSAN errors in dse.c (PR rtl-optimization/82044).
gcc/ChangeLog:
2017-10-19 Martin Liska <mliska@suse.cz>
PR rtl-optimization/82044
PR tree-optimization/82042
* dse.c (check_mem_read_rtx): Check for overflow.
---
gcc/dse.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/gcc/dse.c b/gcc/dse.c
index 563ca9f56f3..f6d5e6e6fe2 100644
--- a/gcc/dse.c
+++ b/gcc/dse.c
@@ -1981,6 +1981,12 @@ check_mem_read_rtx (rtx *loc, bb_info_t bb_info)
else
width = GET_MODE_SIZE (GET_MODE (mem));
+ if (offset > HOST_WIDE_INT_MAX - width)
+ {
+ clear_rhs_from_active_local_stores ();
+ return;
+ }
+
read_info = read_info_type_pool.allocate ();
read_info->group_id = group_id;
read_info->mem = mem;
--
2.14.3
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix UBSAN errors in dse.c (PR rtl-optimization/82044).
2017-11-15 7:34 ` Martin Liška
@ 2017-11-17 0:57 ` Jeff Law
2017-11-22 0:27 ` [PATCH] Fix i?86 bootstrap " Jakub Jelinek
1 sibling, 0 replies; 14+ messages in thread
From: Jeff Law @ 2017-11-17 0:57 UTC (permalink / raw)
To: Martin Liška, Jakub Jelinek; +Cc: gcc-patches
On 11/15/2017 12:31 AM, Martin Liška wrote:
> On 11/08/2017 05:31 PM, Jeff Law wrote:
>> I don't see an updated patch in this thread? THe last message I see is
>> this one where you indicate you're going to tweak the patch and re-test.
>>
>> Jeff
> Yes, I tweaked and tested following patch.
>
> Martin
>
>
> 0001-Fix-UBSAN-errors-in-dse.c-PR-rtl-optimization-82044.patch
>
>
> From a369ac78b887e219a375e17d6817c1f744e71779 Mon Sep 17 00:00:00 2001
> From: marxin <mliska@suse.cz>
> Date: Thu, 19 Oct 2017 13:38:01 +0200
> Subject: [PATCH] Fix UBSAN errors in dse.c (PR rtl-optimization/82044).
>
> gcc/ChangeLog:
>
> 2017-10-19 Martin Liska <mliska@suse.cz>
>
> PR rtl-optimization/82044
> PR tree-optimization/82042
> * dse.c (check_mem_read_rtx): Check for overflow.
OK.
jeff
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] Fix i?86 bootstrap (PR rtl-optimization/82044)
2017-11-15 7:34 ` Martin Liška
2017-11-17 0:57 ` Jeff Law
@ 2017-11-22 0:27 ` Jakub Jelinek
2017-11-22 8:01 ` Jakub Jelinek
2017-11-22 9:45 ` Eric Botcazou
1 sibling, 2 replies; 14+ messages in thread
From: Jakub Jelinek @ 2017-11-22 0:27 UTC (permalink / raw)
To: Martin Liška; +Cc: Jeff Law, gcc-patches
Hi!
On Wed, Nov 15, 2017 at 08:31:00AM +0100, Martin Liška wrote:
> On 11/08/2017 05:31 PM, Jeff Law wrote:
> > I don't see an updated patch in this thread? THe last message I see is
> > this one where you indicate you're going to tweak the patch and re-test.
> >
> > Jeff
>
> Yes, I tweaked and tested following patch.
>
> Martin
> >From a369ac78b887e219a375e17d6817c1f744e71779 Mon Sep 17 00:00:00 2001
> From: marxin <mliska@suse.cz>
> Date: Thu, 19 Oct 2017 13:38:01 +0200
> Subject: [PATCH] Fix UBSAN errors in dse.c (PR rtl-optimization/82044).
>
> gcc/ChangeLog:
>
> 2017-10-19 Martin Liska <mliska@suse.cz>
>
> PR rtl-optimization/82044
> PR tree-optimization/82042
> * dse.c (check_mem_read_rtx): Check for overflow.
Unfortunately this patch broke i686-linux bootstrap, during stage2
libgcc configure fails due to numerous ICEs.
There are 2 problems with the patch:
1) if the mode of the read is BLKmode, then width is set to -1,
so offset > HOST_WIDE_INT_MAX - width invokes UB at compile time
and is true for any offset > HOST_WIDE_INT_MIN if the compiler wraps
the result around.
2) clear_rhs_from_active_local_stores () is the punt action in record_store,
but not in check_mem_read_rtx, where e.g. a few lines above it if
canon_address fails it does add_wild_read instead.
The following patch fixes those two issues and adds similar overflow
check to record_store too (in that spot width is always non-negative, so
we don't need a special width == -1 handling).
Bootstrapped successfully on i686-linux, ok for trunk if it passes regtest
there (and pending x86_64-linux bootstrap + regtest)?
2017-11-21 Jakub Jelinek <jakub@redhat.com>
PR rtl-optimization/82044
PR tree-optimization/82042
* dse.c (record_store): Check for overflow.
(check_mem_read_rtx): Properly check for overflow if width == -1, call
add_wild_read instead of clear_rhs_from_active_local_stores on
overflow and log it into dump_file.
--- gcc/dse.c.jj 2017-11-21 23:18:18.000000000 +0100
+++ gcc/dse.c 2017-11-21 23:28:08.952439915 +0100
@@ -1342,6 +1342,12 @@ record_store (rtx body, bb_info_t bb_inf
else
width = GET_MODE_SIZE (GET_MODE (mem));
+ if (offset > HOST_WIDE_INT_MAX - width)
+ {
+ clear_rhs_from_active_local_stores ();
+ return 0;
+ }
+
if (group_id >= 0)
{
/* In the restrictive case where the base is a constant or the
@@ -1981,9 +1987,13 @@ check_mem_read_rtx (rtx *loc, bb_info_t
else
width = GET_MODE_SIZE (GET_MODE (mem));
- if (offset > HOST_WIDE_INT_MAX - width)
+ if (width == -1
+ ? offset == HOST_WIDE_INT_MIN
+ : offset > HOST_WIDE_INT_MAX - width)
{
- clear_rhs_from_active_local_stores ();
+ if (dump_file && (dump_flags & TDF_DETAILS))
+ fprintf (dump_file, " adding wild read, due to overflow.\n");
+ add_wild_read (bb_info);
return;
}
Jakub
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix i?86 bootstrap (PR rtl-optimization/82044)
2017-11-22 0:27 ` [PATCH] Fix i?86 bootstrap " Jakub Jelinek
@ 2017-11-22 8:01 ` Jakub Jelinek
2017-11-22 9:00 ` Richard Biener
2017-11-22 9:45 ` Eric Botcazou
1 sibling, 1 reply; 14+ messages in thread
From: Jakub Jelinek @ 2017-11-22 8:01 UTC (permalink / raw)
To: Jeff Law, Richard Biener, Martin Liška; +Cc: gcc-patches
On Wed, Nov 22, 2017 at 12:27:20AM +0100, Jakub Jelinek wrote:
> The following patch fixes those two issues and adds similar overflow
> check to record_store too (in that spot width is always non-negative, so
> we don't need a special width == -1 handling).
>
> Bootstrapped successfully on i686-linux, ok for trunk if it passes regtest
> there (and pending x86_64-linux bootstrap + regtest)?
Now successfully bootstrapped/regtested on both.
Jakub
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix i?86 bootstrap (PR rtl-optimization/82044)
2017-11-22 8:01 ` Jakub Jelinek
@ 2017-11-22 9:00 ` Richard Biener
2017-11-22 9:11 ` Richard Biener
0 siblings, 1 reply; 14+ messages in thread
From: Richard Biener @ 2017-11-22 9:00 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Jeff Law, Martin Liška, gcc-patches
On Wed, 22 Nov 2017, Jakub Jelinek wrote:
> On Wed, Nov 22, 2017 at 12:27:20AM +0100, Jakub Jelinek wrote:
> > The following patch fixes those two issues and adds similar overflow
> > check to record_store too (in that spot width is always non-negative, so
> > we don't need a special width == -1 handling).
> >
> > Bootstrapped successfully on i686-linux, ok for trunk if it passes regtest
> > there (and pending x86_64-linux bootstrap + regtest)?
>
> Now successfully bootstrapped/regtested on both.
Ok.
Richard.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix i?86 bootstrap (PR rtl-optimization/82044)
2017-11-22 9:00 ` Richard Biener
@ 2017-11-22 9:11 ` Richard Biener
2017-12-19 11:26 ` Martin Liška
0 siblings, 1 reply; 14+ messages in thread
From: Richard Biener @ 2017-11-22 9:11 UTC (permalink / raw)
To: Richard Biener; +Cc: Jakub Jelinek, Jeff Law, Martin Liška, GCC Patches
On Wed, Nov 22, 2017 at 9:54 AM, Richard Biener <rguenther@suse.de> wrote:
> On Wed, 22 Nov 2017, Jakub Jelinek wrote:
>
>> On Wed, Nov 22, 2017 at 12:27:20AM +0100, Jakub Jelinek wrote:
>> > The following patch fixes those two issues and adds similar overflow
>> > check to record_store too (in that spot width is always non-negative, so
>> > we don't need a special width == -1 handling).
>> >
>> > Bootstrapped successfully on i686-linux, ok for trunk if it passes regtest
>> > there (and pending x86_64-linux bootstrap + regtest)?
>>
>> Now successfully bootstrapped/regtested on both.
>
> Ok.
I've reverted the patch on the branch. This isn't stuff we should
backport IMHO.
Richard.
> Richard.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix i?86 bootstrap (PR rtl-optimization/82044)
2017-11-22 0:27 ` [PATCH] Fix i?86 bootstrap " Jakub Jelinek
2017-11-22 8:01 ` Jakub Jelinek
@ 2017-11-22 9:45 ` Eric Botcazou
2017-11-22 9:52 ` Jakub Jelinek
1 sibling, 1 reply; 14+ messages in thread
From: Eric Botcazou @ 2017-11-22 9:45 UTC (permalink / raw)
To: gcc-patches, Jakub Jelinek; +Cc: Martin Liška, Jeff Law
> Unfortunately this patch broke i686-linux bootstrap, during stage2
> libgcc configure fails due to numerous ICEs.
Note that the patch is on the 7 branch too.
--
Eric Botcazou
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix i?86 bootstrap (PR rtl-optimization/82044)
2017-11-22 9:45 ` Eric Botcazou
@ 2017-11-22 9:52 ` Jakub Jelinek
0 siblings, 0 replies; 14+ messages in thread
From: Jakub Jelinek @ 2017-11-22 9:52 UTC (permalink / raw)
To: Eric Botcazou; +Cc: gcc-patches, Martin Liška, Jeff Law
On Wed, Nov 22, 2017 at 10:43:47AM +0100, Eric Botcazou wrote:
> > Unfortunately this patch broke i686-linux bootstrap, during stage2
> > libgcc configure fails due to numerous ICEs.
>
> Note that the patch is on the 7 branch too.
Richard has reverted it there.
Jakub
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix i?86 bootstrap (PR rtl-optimization/82044)
2017-11-22 9:11 ` Richard Biener
@ 2017-12-19 11:26 ` Martin Liška
0 siblings, 0 replies; 14+ messages in thread
From: Martin Liška @ 2017-12-19 11:26 UTC (permalink / raw)
To: Richard Biener, Richard Biener; +Cc: Jakub Jelinek, Jeff Law, GCC Patches
On 11/22/2017 10:05 AM, Richard Biener wrote:
> On Wed, Nov 22, 2017 at 9:54 AM, Richard Biener <rguenther@suse.de> wrote:
>> On Wed, 22 Nov 2017, Jakub Jelinek wrote:
>>
>>> On Wed, Nov 22, 2017 at 12:27:20AM +0100, Jakub Jelinek wrote:
>>>> The following patch fixes those two issues and adds similar overflow
>>>> check to record_store too (in that spot width is always non-negative, so
>>>> we don't need a special width == -1 handling).
>>>>
>>>> Bootstrapped successfully on i686-linux, ok for trunk if it passes regtest
>>>> there (and pending x86_64-linux bootstrap + regtest)?
>>>
>>> Now successfully bootstrapped/regtested on both.
>>
>> Ok.
>
> I've reverted the patch on the branch. This isn't stuff we should
> backport IMHO.
>
> Richard.
>
>> Richard.
Thank you all for fixing that. I'm sorry for that.
Martin
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2017-12-19 11:26 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-20 7:51 [PATCH] Fix UBSAN errors in dse.c (PR rtl-optimization/82044) Martin Liška
2017-09-20 8:15 ` Jakub Jelinek
2017-10-19 11:58 ` Martin Liška
2017-11-02 13:15 ` Martin Liška
2017-11-08 16:42 ` Jeff Law
2017-11-15 7:34 ` Martin Liška
2017-11-17 0:57 ` Jeff Law
2017-11-22 0:27 ` [PATCH] Fix i?86 bootstrap " Jakub Jelinek
2017-11-22 8:01 ` Jakub Jelinek
2017-11-22 9:00 ` Richard Biener
2017-11-22 9:11 ` Richard Biener
2017-12-19 11:26 ` Martin Liška
2017-11-22 9:45 ` Eric Botcazou
2017-11-22 9:52 ` Jakub Jelinek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).