8.1-rc0 testfloat fails to compile

8.1-rc0 testfloat fails to compile

[Olaf Hering]

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

This is going on since a few weeks. I guess there is no check in CI to see if qemu.git#master compiles in Tumbleweed.

Since the switch to meson submodules, berkeley-testfloat-3 became mandatory. I think in the past I was able to ignore this submodule and not export it, so the following error did not show up:

[  141s] ../subprojects/berkeley-testfloat-3/source/genCases_f64.c: In function 'f64Random':
[  141s] ../subprojects/berkeley-testfloat-3/source/genCases_f64.c:559:1: error: control reaches end of non-void function [-Werror=return-type]
[  141s]   559 | }
[  141s]       | ^
[  141s] cc1: some warnings being treated as errors

Apparently this is a known issue, 3ac1f81329f attempted to ignore such errors.
Do I need to tweak the global, system-provided CFLAGS myself, or can the source be fixed to address this? Disabling this error globally will hide errors elsewhere.

Maybe there is a way to append something to tests/fp/meson.build:libtestfloat.c_args? Right now it is apparently set to tfcflags+fpcflags+CFLAGS.


Olaf

[-- Attachment #2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: 8.1-rc0 testfloat fails to compile

[Thomas Huth]

On 20/07/2023 22.47, Olaf Hering wrote:
> This is going on since a few weeks. I guess there is no check in CI to see if qemu.git#master compiles in Tumbleweed.

We only have a check for openSUSE leap ...
Which compiler version is causing trouble for you?

> Since the switch to meson submodules, berkeley-testfloat-3 became mandatory. I think in the past I was able to ignore this submodule and not export it, so the following error did not show up:
> 
> [  141s] ../subprojects/berkeley-testfloat-3/source/genCases_f64.c: In function 'f64Random':
> [  141s] ../subprojects/berkeley-testfloat-3/source/genCases_f64.c:559:1: error: control reaches end of non-void function [-Werror=return-type]
> [  141s]   559 | }
> [  141s]       | ^
> [  141s] cc1: some warnings being treated as errors
> 
> Apparently this is a known issue, 3ac1f81329f attempted to ignore such errors.

Seems like the flag got lost in commit d2dfe0b506e47e14 ... Paolo, any ideas?

> Do I need to tweak the global, system-provided CFLAGS myself, or can the source be fixed to address this? Disabling this error globally will hide errors elsewhere.

We are using a forked version of the berkeley-testfloat repository, and it's 
possible to add patches there:

  https://gitlab.com/qemu-project/berkeley-testfloat-3

The f64Random function that you mentioned above could easily be fixed by 
adding a "default:" case to the switch statement, I think. Are there any 
other additional warnings/errors after fixing this?

Feel free to send a patch and CC: the people from 
https://gitlab.com/qemu-project/berkeley-testfloat-3/-/project_members

  Thanks,
   Thomas


> Maybe there is a way to append something to tests/fp/meson.build:libtestfloat.c_args? Right now it is apparently set to tfcflags+fpcflags+CFLAGS.
> 
> 
> Olaf

Re: 8.1-rc0 testfloat fails to compile

[Olaf Hering]

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

Fri, 21 Jul 2023 08:54:21 +0200 Thomas Huth <thuth@redhat.com>:

> Which compiler version is causing trouble for you?

Right now it is gcc 13, hopefully every compiler will error out with -Werror=return-type.

I think it makes sense to replace Leap with Tumbleweed. We already know it compiles fine with Leap, because the submission compiled fine on the submitters laptop. Where is the repository that needs to be adjusted to make this replacement?


Olaf

[-- Attachment #2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: 8.1-rc0 testfloat fails to compile

[Thomas Huth]

On 21/07/2023 09.03, Olaf Hering wrote:
> Fri, 21 Jul 2023 08:54:21 +0200 Thomas Huth <thuth@redhat.com>:
> 
>> Which compiler version is causing trouble for you?
> 
> Right now it is gcc 13, hopefully every compiler will error out with -Werror=return-type.

I don't think this will happen - otherwise we would have seen this already. 
It rather looks like your compiler version is missing something here - have 
a look at the affected function:

static float64_t f64Random( void )
{

     switch ( random_ui8() & 7 ) {
      case 0:
      case 1:
      case 2:
         return f64RandomQOutP3();
      case 3:
         return f64RandomQOutPInf();
      case 4:
      case 5:
      case 6:
         return f64RandomQInfP3();
      case 7:
         return f64RandomQInfPInf();
     }

}

The argument in the switch statement is limited with "& 7" to the range of 0 
... 7 , so there is no way the control flow can really reach the bottom of 
the function. I wonder why your GCC 13 gets that wrong, all other versions 
seem to be fine.
Or are you compiling with -O0 or something similar?

> I think it makes sense to replace Leap with Tumbleweed. 

IIRC we wanted to avoid rolling releases in the CI since this would rather 
force us to deal with distro bugs on a regular basis instead of focusing on 
bugs in QEMU.

  Thomas

Re: 8.1-rc0 testfloat fails to compile

[Olaf Hering]

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

Fri, 21 Jul 2023 09:18:08 +0200 Thomas Huth <thuth@redhat.com>:

> Or are you compiling with -O0 or something similar?

Does the master branch compile for you with 'export CFLAGS="-O2 -Wall -Werror=return-type"'?

I prepared a patch to deal with it, and indeed it looks like the compiler might be able to spot the fact that only values between 0 and 7 can ever appear there. Apparently it fails to recognize that.


Olaf

[-- Attachment #2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: 8.1-rc0 testfloat fails to compile

[Thomas Huth]

On 21/07/2023 09.35, Olaf Hering wrote:
> Fri, 21 Jul 2023 09:18:08 +0200 Thomas Huth <thuth@redhat.com>:
> 
>> Or are you compiling with -O0 or something similar?
> 
> Does the master branch compile for you with 'export CFLAGS="-O2 -Wall -Werror=return-type"'?

Oh, this fails for me, too! I thought that -Wreturn-type would be part of 
-Wall, but apparently it is only enabled by default for C++ in my version of 
GCC :-(

  Thomas

Re: 8.1-rc0 testfloat fails to compile

[Olaf Hering]

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

Fri, 21 Jul 2023 09:53:51 +0200 Thomas Huth <thuth@redhat.com>:

>  I thought that -Wreturn-type would be part of 
> -Wall, but apparently it is only enabled by default for C++ in my version of 
> GCC :-(

Hopefully gcc14 will fix that, and others. Only two decades late ...


Olaf

[-- Attachment #2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: 8.1-rc0 testfloat fails to compile

[Richard Henderson]

On 7/21/23 07:54, Thomas Huth wrote:
> On 20/07/2023 22.47, Olaf Hering wrote:
>> This is going on since a few weeks. I guess there is no check in CI to see if 
>> qemu.git#master compiles in Tumbleweed.
> 
> We only have a check for openSUSE leap ...
> Which compiler version is causing trouble for you?
> 
>> Since the switch to meson submodules, berkeley-testfloat-3 became mandatory. I think in 
>> the past I was able to ignore this submodule and not export it, so the following error 
>> did not show up:
>>
>> [  141s] ../subprojects/berkeley-testfloat-3/source/genCases_f64.c: In function 
>> 'f64Random':
>> [  141s] ../subprojects/berkeley-testfloat-3/source/genCases_f64.c:559:1: error: control 
>> reaches end of non-void function [-Werror=return-type]
>> [  141s]   559 | }
>> [  141s]       | ^
>> [  141s] cc1: some warnings being treated as errors
>>
>> Apparently this is a known issue, 3ac1f81329f attempted to ignore such errors.
> 
> Seems like the flag got lost in commit d2dfe0b506e47e14 ... Paolo, any ideas?
> 
>> Do I need to tweak the global, system-provided CFLAGS myself, or can the source be fixed 
>> to address this? Disabling this error globally will hide errors elsewhere.
> 
> We are using a forked version of the berkeley-testfloat repository, and it's possible to 
> add patches there:
> 
>   https://gitlab.com/qemu-project/berkeley-testfloat-3
> 
> The f64Random function that you mentioned above could easily be fixed by adding a 
> "default:" case to the switch statement, I think. Are there any other additional 
> warnings/errors after fixing this?
> 
> Feel free to send a patch and CC: the people from 
> https://gitlab.com/qemu-project/berkeley-testfloat-3/-/project_members

If this is with optimization enabled, the bug should be reported to gcc bugzilla.
The compiler should easily prove the default case is unreachable.


r~

Re: 8.1-rc0 testfloat fails to compile

[Olaf Hering]

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

Sat, 22 Jul 2023 13:49:40 +0100 Richard Henderson <richard.henderson@linaro.org>:

> If this is with optimization enabled, the bug should be reported to gcc bugzilla.
> The compiler should easily prove the default case is unreachable.

This happens also with -O0 or without any -On, or without -Wall.

https://bugzilla.suse.com/show_bug.cgi?id=1213600

https://gitlab.com/qemu-project/berkeley-testfloat-3/-/merge_requests/2


Olaf

[-- Attachment #2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]