Re: [PATCH 1/6] sim: sh: rework register layout with anonymous unions & structs

[Mike Frysinger via Gdb-patches <gdb-patches@sourceware.org>]

[-- Attachment #1: Type: text/plain, Size: 2780 bytes --]

On 11 Nov 2021 19:32, Luis Machado via Gdb-patches wrote:
> On 11/11/21 7:25 PM, Mike Frysinger wrote:
> > On 11 Nov 2021 09:41, Luis Machado wrote:
> >> I can't pinpoint the exact SH patch, but builds are broken for
> >> --enable-targets=all in Ubuntu 18.04 with GCC 7.5:
> > 
> > pretty sure the warnings have always been there for you, we just turned
> > -Werror in more cases recently
> 
> That's fine and I'm all for more -Werror. But builds should be able to 
> complete regardless, right?

they do build for me in a huge variety of configurations.  but i don't test
every random distro version that people might have.  plus for release, we
turn off automatic -Werror, so the issue you highlight only affects the
live development tree.  it's not going to break releases that distros use.

> >> binutils-gdb/sim/sh/interp.c: In function ‘ppi_insn’:
> >> ./ppi.c:875:21: error: assuming signed overflow does not occur when
> >> assuming that (X + c) < X is always false [-Werror=strict-overflow]
> >>            carry = res < Sy;
> >>                    ~~~~^~~~
> >> ./ppi.c:849:21: error: assuming signed overflow does not occur when
> >> assuming that (X - c) > X is always false [-Werror=strict-overflow]
> >>            carry = res > Sy;
> >>                    ~~~~^~~~
> >> ./ppi.c:823:21: error: assuming signed overflow does not occur when
> >> assuming that (X + c) < X is always false [-Werror=strict-overflow]
> >>            carry = res < Sx;
> >>                    ~~~~^~~~
> >> ./ppi.c:797:21: error: assuming signed overflow does not occur when
> >> assuming that (X - c) > X is always false [-Werror=strict-overflow]
> >>            carry = res > Sx;
> >>                    ~~~~^~~~
> >> binutils-gdb-arm64-bionic/sim/../../../repos/binutils-gdb/sim/sh/interp.c:
> >> In function ‘sim_resume’:
> >> ./ppi.c:1178:28: warning: ‘res’ may be used uninitialized in this
> >> function [-Wmaybe-uninitialized]
> >>              MACL = DSP_R (z) = res;
> >> ./ppi.c:44:7: note: ‘res’ was declared here
> >>      int res, res_grd;
> >>          ^~~
> > 
> > how exactly are you building things ?  we don't enable -Wstrict-overflow.
> 
> No special way. I just ./configure with --enable-targets=all and let it 
> run. I can bisect it or gather a bit more information if you think it 
> would be useful.

bisect would't really help.  as i said, the warnings have probably long been
there.  what changed is using -Werror in more places, and that commit is easy
to pick out.

> Worth mentioning that it doesn't run into such warnings/failures on 
> Ubuntu 20.04 with gcc 9.3.

it looks like gcc changed behavior starting in gcc-8.  in older versions,
-fstrict-overflow was enabled at -O2.
-mike

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]