Re: [PATCH] Fix UBSAN errors in dse.c (PR rtl-optimization/82044).

2017-11-16 Thread Jeff Law 
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 
> 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  
> 
>   PR rtl-optimization/82044
>   PR tree-optimization/82042
>   * dse.c (check_mem_read_rtx): Check for overflow.
OK.

jeff

Re: [PATCH] Fix UBSAN errors in dse.c (PR rtl-optimization/82044).

2017-11-14 Thread Martin Liška 
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 
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  

	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

Re: [PATCH] Fix UBSAN errors in dse.c (PR rtl-optimization/82044).

2017-11-08 Thread Jeff Law 
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>>>{
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
>>>
>>
>

Re: [PATCH] Fix UBSAN errors in dse.c (PR rtl-optimization/82044).

2017-11-02 Thread Martin Liška 
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>>{
>>> 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
>>
>

Re: [PATCH] Fix UBSAN errors in dse.c (PR rtl-optimization/82044).

2017-10-19 Thread Martin Liška 
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>{
>>  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
>

Re: [PATCH] Fix UBSAN errors in dse.c (PR rtl-optimization/82044).

2017-09-20 Thread Jakub Jelinek 
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{
>   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

[PATCH] Fix UBSAN errors in dse.c (PR rtl-optimization/82044).

2017-09-20 Thread Martin Liška 
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  

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


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; igroup_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