This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Add offset_int to guard HOST_WIDE_INT overflow (PR tree-optimization/82042).
- From: Martin Sebor <msebor at gmail dot com>
- To: Richard Biener <richard dot guenther at gmail dot com>, Martin Liška <mliska at suse dot cz>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Fri, 10 Nov 2017 09:06:29 -0700
- Subject: Re: [PATCH] Add offset_int to guard HOST_WIDE_INT overflow (PR tree-optimization/82042).
- Authentication-results: sourceware.org; auth=none
- References: <b8723c3d-561e-6681-9090-256d79b27914@suse.cz> <CAFiYyc2ugjwXaEum1--5upyV3Qa=8v-55p_V-V8wj+1b7ez1Zg@mail.gmail.com> <04946325-2fa3-2324-cd49-31b7d033152e@suse.cz> <CAFiYyc1LTZA5VS7OfWWyW_4nFFRp7eixpzD_dDw_uubEJ+hHEA@mail.gmail.com>
On 10/19/2017 05:58 AM, Richard Biener wrote:
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 ;)
Martin, thanks for trying to fix the bug. FWIW, from the winky
I suspect Richard's last comment was more tongue-in-cheek than
serious. I'm sure we don't want well-formed programs to trigger
undefined behavior in GCC, even if their own behavior isn't well
defined at runtime.
FWIW, I raised the bug report while testing a fix for a similar
bug Richard had noted in a review of one of my own patches that
wasn't properly handling unsigned wrapping (see below). I was
trying to verify that my final patch was free of the problem and
came across a couple of others elsewhere.
Martin
https://gcc.gnu.org/ml/gcc-patches/2017-08/msg01441.html