This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Add offset_int to guard HOST_WIDE_INT overflow (PR tree-optimization/82042).


On Thu, Oct 19, 2017 at 1:21 PM, Martin Liška <mliska@suse.cz> wrote:
> On 09/20/2017 05:47 PM, Richard Biener wrote:
>> On Wed, Sep 20, 2017 at 9:50 AM, Martin Liška <mliska@suse.cz> wrote:
>>> Hi.
>>>
>>> This is partial fix for the PR, where I calculate 2 offsets in offset_int type.
>>>
>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>>>
>>> Ready to be installed?
>>
>> That seems to poorly handle the cases in offset_overlap_p.  What kind of
>> overflow do we see here?
>
> We hit:
>
>         (const:DI (plus:DI (symbol_ref:DI ("a") [flags 0x40] <var_decl 0x7ffff7fe6ea0 a>)
>                 (const_int -9223372036854775805 [0x8000000000000003])))) "/home/marxin/Programming/testcases/pr82042.c":8 81 {*movdi_internal}
>
> and:
>
>         (const:DI (plus:DI (symbol_ref:DI ("a") [flags 0x40] <var_decl 0x7ffff7fe6ea0 a>)
>                 (const_int 9223372036854775806 [0x7ffffffffffffffe])))) 81 {*movdi_internal}
>
> The UBSAN error is:
> ../../gcc/tree-ssa-alias.c:704:30: runtime error: signed integer overflow: 9223372036854775806 * 8 cannot be represented in type 'long int'

Huh, but that's a different error?  You are patching alias.c ...

I guess semantically we need to make 'c' sth that is big enough to handle
address differences in bytes (offset_int is for bits) and with
arbitrary positive/negative
value.  offset_int would work of course.   The testcase for the MEMs above is
artificial I guess (those are big offsets to a symbol...).

There are many more places we process offsets that way, for example
in nonoverlapping_memrefs_p (and I bet tree-ssa-alias.c as well).

In the end I don't think we want to slow down code just for the sake of
UBSAN.  IMHO for code invoking undefined behavior (object too large)
it's reasonable for the compiler to invoke undefined behavior ;)

Richard.

> Martin
>
>>
>> Richard.
>>
>>> Martin
>>>
>>> gcc/ChangeLog:
>>>
>>> 2017-09-20  Martin Liska  <mliska@suse.cz>
>>>
>>>         PR tree-optimization/82042
>>>         * alias.c (memrefs_conflict_p): Calculate offset in offset_int
>>>         type.
>>>         * cse.c (use_related_value): Likewise.
>>> ---
>>>  gcc/alias.c | 10 ++++++++--
>>>  gcc/cse.c   | 14 ++++++++++----
>>>  2 files changed, 18 insertions(+), 6 deletions(-)
>>>
>>>
>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]