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

[Luis Machado via Gdb-patches <gdb-patches@sourceware.org>]

On 11/11/21 7:45 PM, Mike Frysinger wrote:
> 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.
> 

None of us test every possible combination. It is not easy to have high 
coverage without some sort of CI in place.

>>>> 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.

Since I have it handy here, let me go through it and try to come up with 
a fix for the ones I'm seeing.