diff mbox series

tests/tcg: Suppress compiler false-positive warning on sha1.c

Message ID 20250227141343.1675415-1-peter.maydell@linaro.org (mailing list archive)
State New, archived
Headers show
Series tests/tcg: Suppress compiler false-positive warning on sha1.c | expand

Commit Message

Peter Maydell Feb. 27, 2025, 2:13 p.m. UTC
GCC versions at least 12 through 15 incorrectly report a warning
about code in sha1.c:

tests/tcg/multiarch/sha1.c:161:13: warning: ‘SHA1Transform’ reading 64 bytes from a region of size 0 [-Wstringop-overread]
  161 |             SHA1Transform(context->state, &data[i]);
      |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This is a piece of stock library code for doing SHA1 which we've
simply copied, rather than writing ourselves. The bug has been
reported to upstream GCC (about a different library's use of this
code):
 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106709

For our test case, since this isn't our original code and there isn't
actually a bug in it, suppress the incorrect warning rather than
trying to modify the code to work around the compiler issue.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2328
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 tests/tcg/aarch64/Makefile.target   | 3 ++-
 tests/tcg/arm/Makefile.target       | 3 ++-
 tests/tcg/multiarch/Makefile.target | 8 ++++++++
 3 files changed, 12 insertions(+), 2 deletions(-)

Comments

Richard Henderson Feb. 27, 2025, 4:35 p.m. UTC | #1
On 2/27/25 06:13, Peter Maydell wrote:
> GCC versions at least 12 through 15 incorrectly report a warning
> about code in sha1.c:
> 
> tests/tcg/multiarch/sha1.c:161:13: warning: ‘SHA1Transform’ reading 64 bytes from a region of size 0 [-Wstringop-overread]
>    161 |             SHA1Transform(context->state, &data[i]);
>        |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> This is a piece of stock library code for doing SHA1 which we've
> simply copied, rather than writing ourselves. The bug has been
> reported to upstream GCC (about a different library's use of this
> code):
>   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106709
> 
> For our test case, since this isn't our original code and there isn't
> actually a bug in it, suppress the incorrect warning rather than
> trying to modify the code to work around the compiler issue.
> 
> Resolves:https://gitlab.com/qemu-project/qemu/-/issues/2328
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   tests/tcg/aarch64/Makefile.target   | 3 ++-
>   tests/tcg/arm/Makefile.target       | 3 ++-
>   tests/tcg/multiarch/Makefile.target | 8 ++++++++
>   3 files changed, 12 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~
Alex Bennée Feb. 27, 2025, 10:23 p.m. UTC | #2
Peter Maydell <peter.maydell@linaro.org> writes:

> GCC versions at least 12 through 15 incorrectly report a warning
> about code in sha1.c:
>
> tests/tcg/multiarch/sha1.c:161:13: warning: ‘SHA1Transform’ reading 64 bytes from a region of size 0 [-Wstringop-overread]
>   161 |             SHA1Transform(context->state, &data[i]);
>       |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> This is a piece of stock library code for doing SHA1 which we've
> simply copied, rather than writing ourselves. The bug has been
> reported to upstream GCC (about a different library's use of this
> code):
>  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106709
>
> For our test case, since this isn't our original code and there isn't
> actually a bug in it, suppress the incorrect warning rather than
> trying to modify the code to work around the compiler issue.

Queued to maintainer/for-10.0-softfreeze, thanks.
Alex Bennée March 4, 2025, 11:56 a.m. UTC | #3
Peter Maydell <peter.maydell@linaro.org> writes:

> GCC versions at least 12 through 15 incorrectly report a warning
> about code in sha1.c:
>
> tests/tcg/multiarch/sha1.c:161:13: warning: ‘SHA1Transform’ reading 64 bytes from a region of size 0 [-Wstringop-overread]
>   161 |             SHA1Transform(context->state, &data[i]);
>       |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> This is a piece of stock library code for doing SHA1 which we've
> simply copied, rather than writing ourselves. The bug has been
> reported to upstream GCC (about a different library's use of this
> code):
>  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106709
>
> For our test case, since this isn't our original code and there isn't
> actually a bug in it, suppress the incorrect warning rather than
> trying to modify the code to work around the compiler issue.
<snip>
> --- a/tests/tcg/multiarch/Makefile.target
> +++ b/tests/tcg/multiarch/Makefile.target
> @@ -45,6 +45,14 @@ vma-pthread: LDFLAGS+=-pthread
>  sigreturn-sigmask: CFLAGS+=-pthread
>  sigreturn-sigmask: LDFLAGS+=-pthread
>  
> +# GCC versions 12/13/14/15 at least incorrectly complain about
> +# "'SHA1Transform' reading 64 bytes from a region of size 0"; see the gcc bug
> +# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106709
> +# Since this is just a standard piece of library code we've borrowed for a
> +# TCG test case, suppress the warning rather than trying to modify the
> +# code to work around the compiler.
> +sha1: CFLAGS+=-Wno-stringop-overread
> +

Sadly this breaks the hexagon compiler:

  error: unknown warning option '-Wno-stringop-overread' [-Werror,-Wunknown-warning-option]
  Traceback (most recent call last):
    File "/home/alex/lsrc/qemu.git/tests/docker/docker.py", line 683, in <module>
      sys.exit(main())
               ^^^^^^
    File "/home/alex/lsrc/qemu.git/tests/docker/docker.py", line 679, in main
      return args.cmdobj.run(args, argv)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/home/alex/lsrc/qemu.git/tests/docker/docker.py", line 657, in run
      return Docker().run(cmd, False, quiet=args.quiet,
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/home/alex/lsrc/qemu.git/tests/docker/docker.py", line 370, in run
      ret = self._do_check(["run", "--rm", "--label",
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/home/alex/lsrc/qemu.git/tests/docker/docker.py", line 247, in _do_check
      return subprocess.check_call(self._command + cmd, **kwargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/usr/lib/python3.11/subprocess.py", line 413, in check_call
      raise CalledProcessError(retcode, cmd)
  subprocess.CalledProcessError: Command '['podman', 'run', '--rm', '--label', 'com.qemu.instance.uuid=5bbb7b6ed2ea4377b9b6d646859ec4ea', '--userns=keep-id', '-u', '1000', '-w', '/home/alex/lsrc/qemu.git/builds/all/tests/tcg/hexagon-linux-user', '-v', '/home/alex/lsrc/qemu.git/builds/all/tests/tcg/hexagon-linux-user:/home/alex/lsrc/qemu.git/builds/all/tests/tcg/hexagon-linux-user:rw', '-v', '/home/alex/lsrc/qemu.git:/home/alex/lsrc/qemu.git:ro,z', 'qemu/debian-hexagon-cross', 'hexagon-unknown-linux-musl-clang', '-Wno-incompatible-pointer-types', '-Wno-undefined-internal', '-fno-unroll-loops', '-fno-stack-protector', '-Wall', '-Werror', '-O0', '-g', '-fno-strict-aliasing', '-Wno-stringop-overread', '-mv73', '-O2', '-static', '/home/alex/lsrc/qemu.git/tests/tcg/multiarch/sha1.c', '-o', 'sha1', '-static']' returned non-zero exit status 1.
  filter=--filter=label=com.qemu.instance.uuid=5bbb7b6ed2ea4377b9b6d646859ec4ea
  make[1]: *** [Makefile:122: sha1] Error 1
  make: *** [/home/alex/lsrc/qemu.git/tests/Makefile.include:52: build-tcg-tests-hexagon-linux-user] Error 2

Is it that new an option?

>  # The vma-pthread seems very sensitive on gitlab and we currently
>  # don't know if its exposing a real bug or the test is flaky.
>  ifneq ($(GITLAB_CI),)
Peter Maydell March 4, 2025, 12:51 p.m. UTC | #4
On Tue, 4 Mar 2025 at 11:56, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Peter Maydell <peter.maydell@linaro.org> writes:
> > +# GCC versions 12/13/14/15 at least incorrectly complain about
> > +# "'SHA1Transform' reading 64 bytes from a region of size 0"; see the gcc bug
> > +# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106709
> > +# Since this is just a standard piece of library code we've borrowed for a
> > +# TCG test case, suppress the warning rather than trying to modify the
> > +# code to work around the compiler.
> > +sha1: CFLAGS+=-Wno-stringop-overread
> > +
>
> Sadly this breaks the hexagon compiler:
>
>   error: unknown warning option '-Wno-stringop-overread' [-Werror,-Wunknown-warning-option]
>   Traceback (most recent call last):
>     File "/home/alex/lsrc/qemu.git/tests/docker/docker.py", line 683, in <module>
>       sys.exit(main())
>                ^^^^^^
>     File "/home/alex/lsrc/qemu.git/tests/docker/docker.py", line 679, in main
>       return args.cmdobj.run(args, argv)
>              ^^^^^^^^^^^^^^^^^^^^^^^^^^^
>     File "/home/alex/lsrc/qemu.git/tests/docker/docker.py", line 657, in run
>       return Docker().run(cmd, False, quiet=args.quiet,
>              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>     File "/home/alex/lsrc/qemu.git/tests/docker/docker.py", line 370, in run
>       ret = self._do_check(["run", "--rm", "--label",
>             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>     File "/home/alex/lsrc/qemu.git/tests/docker/docker.py", line 247, in _do_check
>       return subprocess.check_call(self._command + cmd, **kwargs)
>              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>     File "/usr/lib/python3.11/subprocess.py", line 413, in check_call
>       raise CalledProcessError(retcode, cmd)
>   subprocess.CalledProcessError: Command '['podman', 'run', '--rm', '--label', 'com.qemu.instance.uuid=5bbb7b6ed2ea4377b9b6d646859ec4ea', '--userns=keep-id', '-u', '1000', '-w', '/home/alex/lsrc/qemu.git/builds/all/tests/tcg/hexagon-linux-user', '-v', '/home/alex/lsrc/qemu.git/builds/all/tests/tcg/hexagon-linux-user:/home/alex/lsrc/qemu.git/builds/all/tests/tcg/hexagon-linux-user:rw', '-v', '/home/alex/lsrc/qemu.git:/home/alex/lsrc/qemu.git:ro,z', 'qemu/debian-hexagon-cross', 'hexagon-unknown-linux-musl-clang', '-Wno-incompatible-pointer-types', '-Wno-undefined-internal', '-fno-unroll-loops', '-fno-stack-protector', '-Wall', '-Werror', '-O0', '-g', '-fno-strict-aliasing', '-Wno-stringop-overread', '-mv73', '-O2', '-static', '/home/alex/lsrc/qemu.git/tests/tcg/multiarch/sha1.c', '-o', 'sha1', '-static']' returned non-zero exit status 1.
>   filter=--filter=label=com.qemu.instance.uuid=5bbb7b6ed2ea4377b9b6d646859ec4ea
>   make[1]: *** [Makefile:122: sha1] Error 1
>   make: *** [/home/alex/lsrc/qemu.git/tests/Makefile.include:52: build-tcg-tests-hexagon-linux-user] Error 2
>
> Is it that new an option?

I think it's new-ish (gcc 11?). On the other hand
-Wno-unknown-warning-option is quite old, and would suppress
this error. If we do
 CFLAGS+=-Wno-unknown-warning-option -Wno-stringop-overread

does that work?

(Meson has cc.get_supported_arguments() that we can use to
filter out -Wfoo/-Wno-foo options that the compiler doesn't
support, but since this is built via a makefile rather than
by meson that's not conveniently accessible.)

-- PMM
Alex Bennée March 4, 2025, 1:44 p.m. UTC | #5
Peter Maydell <peter.maydell@linaro.org> writes:

> On Tue, 4 Mar 2025 at 11:56, Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>> Peter Maydell <peter.maydell@linaro.org> writes:
>> > +# GCC versions 12/13/14/15 at least incorrectly complain about
>> > +# "'SHA1Transform' reading 64 bytes from a region of size 0"; see the gcc bug
>> > +# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106709
>> > +# Since this is just a standard piece of library code we've borrowed for a
>> > +# TCG test case, suppress the warning rather than trying to modify the
>> > +# code to work around the compiler.
>> > +sha1: CFLAGS+=-Wno-stringop-overread
>> > +
>>
>> Sadly this breaks the hexagon compiler:
>>
>>   error: unknown warning option '-Wno-stringop-overread' [-Werror,-Wunknown-warning-option]
>>   Traceback (most recent call last):
>>     File "/home/alex/lsrc/qemu.git/tests/docker/docker.py", line 683, in <module>
>>       sys.exit(main())
>>                ^^^^^^
>>     File "/home/alex/lsrc/qemu.git/tests/docker/docker.py", line 679, in main
>>       return args.cmdobj.run(args, argv)
>>              ^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>     File "/home/alex/lsrc/qemu.git/tests/docker/docker.py", line 657, in run
>>       return Docker().run(cmd, False, quiet=args.quiet,
>>              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>     File "/home/alex/lsrc/qemu.git/tests/docker/docker.py", line 370, in run
>>       ret = self._do_check(["run", "--rm", "--label",
>>             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>     File "/home/alex/lsrc/qemu.git/tests/docker/docker.py", line 247, in _do_check
>>       return subprocess.check_call(self._command + cmd, **kwargs)
>>              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>     File "/usr/lib/python3.11/subprocess.py", line 413, in check_call
>>       raise CalledProcessError(retcode, cmd)
>>   subprocess.CalledProcessError: Command '['podman', 'run', '--rm', '--label', 'com.qemu.instance.uuid=5bbb7b6ed2ea4377b9b6d646859ec4ea', '--userns=keep-id', '-u', '1000', '-w', '/home/alex/lsrc/qemu.git/builds/all/tests/tcg/hexagon-linux-user', '-v', '/home/alex/lsrc/qemu.git/builds/all/tests/tcg/hexagon-linux-user:/home/alex/lsrc/qemu.git/builds/all/tests/tcg/hexagon-linux-user:rw', '-v', '/home/alex/lsrc/qemu.git:/home/alex/lsrc/qemu.git:ro,z', 'qemu/debian-hexagon-cross', 'hexagon-unknown-linux-musl-clang', '-Wno-incompatible-pointer-types', '-Wno-undefined-internal', '-fno-unroll-loops', '-fno-stack-protector', '-Wall', '-Werror', '-O0', '-g', '-fno-strict-aliasing', '-Wno-stringop-overread', '-mv73', '-O2', '-static', '/home/alex/lsrc/qemu.git/tests/tcg/multiarch/sha1.c', '-o', 'sha1', '-static']' returned non-zero exit status 1.
>>   filter=--filter=label=com.qemu.instance.uuid=5bbb7b6ed2ea4377b9b6d646859ec4ea
>>   make[1]: *** [Makefile:122: sha1] Error 1
>>   make: *** [/home/alex/lsrc/qemu.git/tests/Makefile.include:52: build-tcg-tests-hexagon-linux-user] Error 2
>>
>> Is it that new an option?
>
> I think it's new-ish (gcc 11?). On the other hand
> -Wno-unknown-warning-option is quite old, and would suppress
> this error. If we do
>  CFLAGS+=-Wno-unknown-warning-option -Wno-stringop-overread
>
> does that work?

Yes, I did:

modified   tests/tcg/hexagon/Makefile.target
@@ -18,7 +18,7 @@
 # Hexagon doesn't support gdb, so skip the EXTRA_RUNS
 EXTRA_RUNS =
 
-CFLAGS += -Wno-incompatible-pointer-types -Wno-undefined-internal
+CFLAGS += -Wno-incompatible-pointer-types -Wno-undefined-internal -Wno-unknown-warning-option
 CFLAGS += -fno-unroll-loops -fno-stack-protector


>
> (Meson has cc.get_supported_arguments() that we can use to
> filter out -Wfoo/-Wno-foo options that the compiler doesn't
> support, but since this is built via a makefile rather than
> by meson that's not conveniently accessible.)
>
> -- PMM
Peter Maydell March 4, 2025, 1:53 p.m. UTC | #6
On Tue, 4 Mar 2025 at 13:44, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Peter Maydell <peter.maydell@linaro.org> writes:
> > I think it's new-ish (gcc 11?). On the other hand
> > -Wno-unknown-warning-option is quite old, and would suppress
> > this error. If we do
> >  CFLAGS+=-Wno-unknown-warning-option -Wno-stringop-overread
> >
> > does that work?
>
> Yes, I did:
>
> modified   tests/tcg/hexagon/Makefile.target
> @@ -18,7 +18,7 @@
>  # Hexagon doesn't support gdb, so skip the EXTRA_RUNS
>  EXTRA_RUNS =
>
> -CFLAGS += -Wno-incompatible-pointer-types -Wno-undefined-internal
> +CFLAGS += -Wno-incompatible-pointer-types -Wno-undefined-internal -Wno-unknown-warning-option
>  CFLAGS += -fno-unroll-loops -fno-stack-protector

I think we should do this where we add -Wno-stringop-overread,
not just for the hexagon tests -- or are the tcg tests
guaranteed to be run with a fixed compiler from a container
regardless of the local dev environment?

-- PMm
Alex Bennée March 4, 2025, 2:29 p.m. UTC | #7
Peter Maydell <peter.maydell@linaro.org> writes:

> On Tue, 4 Mar 2025 at 13:44, Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>> Peter Maydell <peter.maydell@linaro.org> writes:
>> > I think it's new-ish (gcc 11?). On the other hand
>> > -Wno-unknown-warning-option is quite old, and would suppress
>> > this error. If we do
>> >  CFLAGS+=-Wno-unknown-warning-option -Wno-stringop-overread
>> >
>> > does that work?
>>
>> Yes, I did:
>>
>> modified   tests/tcg/hexagon/Makefile.target
>> @@ -18,7 +18,7 @@
>>  # Hexagon doesn't support gdb, so skip the EXTRA_RUNS
>>  EXTRA_RUNS =
>>
>> -CFLAGS += -Wno-incompatible-pointer-types -Wno-undefined-internal
>> +CFLAGS += -Wno-incompatible-pointer-types -Wno-undefined-internal -Wno-unknown-warning-option
>>  CFLAGS += -fno-unroll-loops -fno-stack-protector
>
> I think we should do this where we add -Wno-stringop-overread,
> not just for the hexagon tests -- or are the tcg tests
> guaranteed to be run with a fixed compiler from a container
> regardless of the local dev environment?

I can move it, but hexagon is unusual in being clang based. However the
oldest compilers we use are 10.2 in the qemu/debian-legacy-test-cross
container.


>
> -- PMm
Peter Maydell March 4, 2025, 2:36 p.m. UTC | #8
On Tue, 4 Mar 2025 at 14:29, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Peter Maydell <peter.maydell@linaro.org> writes:
>
> > On Tue, 4 Mar 2025 at 13:44, Alex Bennée <alex.bennee@linaro.org> wrote:
> >>
> >> Peter Maydell <peter.maydell@linaro.org> writes:
> >> > I think it's new-ish (gcc 11?). On the other hand
> >> > -Wno-unknown-warning-option is quite old, and would suppress
> >> > this error. If we do
> >> >  CFLAGS+=-Wno-unknown-warning-option -Wno-stringop-overread
> >> >
> >> > does that work?
> >>
> >> Yes, I did:
> >>
> >> modified   tests/tcg/hexagon/Makefile.target
> >> @@ -18,7 +18,7 @@
> >>  # Hexagon doesn't support gdb, so skip the EXTRA_RUNS
> >>  EXTRA_RUNS =
> >>
> >> -CFLAGS += -Wno-incompatible-pointer-types -Wno-undefined-internal
> >> +CFLAGS += -Wno-incompatible-pointer-types -Wno-undefined-internal -Wno-unknown-warning-option
> >>  CFLAGS += -fno-unroll-loops -fno-stack-protector
> >
> > I think we should do this where we add -Wno-stringop-overread,
> > not just for the hexagon tests -- or are the tcg tests
> > guaranteed to be run with a fixed compiler from a container
> > regardless of the local dev environment?
>
> I can move it, but hexagon is unusual in being clang based. However the
> oldest compilers we use are 10.2 in the qemu/debian-legacy-test-cross
> container.

My question was more "do we only ever build this test with
a fixed set of compilers that we control, or are we instead
maybe sometimes using the user's clang/cc/gcc" ?
If the latter, we definitely need to associate the
"don't warn about unknown warnings" with the place we are
adding the option that's not known across all our supported
compilers. If the former, then putting it in the hexagon
specific file seems OK I guess.

-- PMM
Alex Bennée March 4, 2025, 3:58 p.m. UTC | #9
Peter Maydell <peter.maydell@linaro.org> writes:

> On Tue, 4 Mar 2025 at 14:29, Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>> Peter Maydell <peter.maydell@linaro.org> writes:
>>
>> > On Tue, 4 Mar 2025 at 13:44, Alex Bennée <alex.bennee@linaro.org> wrote:
>> >>
>> >> Peter Maydell <peter.maydell@linaro.org> writes:
>> >> > I think it's new-ish (gcc 11?). On the other hand
>> >> > -Wno-unknown-warning-option is quite old, and would suppress
>> >> > this error. If we do
>> >> >  CFLAGS+=-Wno-unknown-warning-option -Wno-stringop-overread
>> >> >
>> >> > does that work?
>> >>
>> >> Yes, I did:
>> >>
>> >> modified   tests/tcg/hexagon/Makefile.target
>> >> @@ -18,7 +18,7 @@
>> >>  # Hexagon doesn't support gdb, so skip the EXTRA_RUNS
>> >>  EXTRA_RUNS =
>> >>
>> >> -CFLAGS += -Wno-incompatible-pointer-types -Wno-undefined-internal
>> >> +CFLAGS += -Wno-incompatible-pointer-types -Wno-undefined-internal -Wno-unknown-warning-option
>> >>  CFLAGS += -fno-unroll-loops -fno-stack-protector
>> >
>> > I think we should do this where we add -Wno-stringop-overread,
>> > not just for the hexagon tests -- or are the tcg tests
>> > guaranteed to be run with a fixed compiler from a container
>> > regardless of the local dev environment?
>>
>> I can move it, but hexagon is unusual in being clang based. However the
>> oldest compilers we use are 10.2 in the qemu/debian-legacy-test-cross
>> container.
>
> My question was more "do we only ever build this test with
> a fixed set of compilers that we control, or are we instead
> maybe sometimes using the user's clang/cc/gcc" ?

Its certainly possible - at least on a Debian system or with the
appropriate --cross-cc-$ARCH flags. I expect 99% of devs either have
Debian or Docker/Podman to fall back on.

> If the latter, we definitely need to associate the
> "don't warn about unknown warnings" with the place we are
> adding the option that's not known across all our supported
> compilers. If the former, then putting it in the hexagon
> specific file seems OK I guess.
>
> -- PMM
Brian Cain March 5, 2025, 2:56 p.m. UTC | #10
On 3/4/2025 6:51 AM, Peter Maydell wrote:
> On Tue, 4 Mar 2025 at 11:56, Alex Bennée <alex.bennee@linaro.org> wrote:
>> Peter Maydell <peter.maydell@linaro.org> writes:
>>> +# GCC versions 12/13/14/15 at least incorrectly complain about
>>> +# "'SHA1Transform' reading 64 bytes from a region of size 0"; see the gcc bug
>>> +# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106709
>>> +# Since this is just a standard piece of library code we've borrowed for a
>>> +# TCG test case, suppress the warning rather than trying to modify the
>>> +# code to work around the compiler.
>>> +sha1: CFLAGS+=-Wno-stringop-overread
>>> +
>> Sadly this breaks the hexagon compiler:
>>
>>    error: unknown warning option '-Wno-stringop-overread' [-Werror,-Wunknown-warning-option]
>>    Traceback (most recent call last):
>>      File "/home/alex/lsrc/qemu.git/tests/docker/docker.py", line 683, in <module>
>>        sys.exit(main())
>>                 ^^^^^^
>>      File "/home/alex/lsrc/qemu.git/tests/docker/docker.py", line 679, in main
>>        return args.cmdobj.run(args, argv)
>>               ^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>      File "/home/alex/lsrc/qemu.git/tests/docker/docker.py", line 657, in run
>>        return Docker().run(cmd, False, quiet=args.quiet,
>>               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>      File "/home/alex/lsrc/qemu.git/tests/docker/docker.py", line 370, in run
>>        ret = self._do_check(["run", "--rm", "--label",
>>              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>      File "/home/alex/lsrc/qemu.git/tests/docker/docker.py", line 247, in _do_check
>>        return subprocess.check_call(self._command + cmd, **kwargs)
>>               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>      File "/usr/lib/python3.11/subprocess.py", line 413, in check_call
>>        raise CalledProcessError(retcode, cmd)
>>    subprocess.CalledProcessError: Command '['podman', 'run', '--rm', '--label', 'com.qemu.instance.uuid=5bbb7b6ed2ea4377b9b6d646859ec4ea', '--userns=keep-id', '-u', '1000', '-w', '/home/alex/lsrc/qemu.git/builds/all/tests/tcg/hexagon-linux-user', '-v', '/home/alex/lsrc/qemu.git/builds/all/tests/tcg/hexagon-linux-user:/home/alex/lsrc/qemu.git/builds/all/tests/tcg/hexagon-linux-user:rw', '-v', '/home/alex/lsrc/qemu.git:/home/alex/lsrc/qemu.git:ro,z', 'qemu/debian-hexagon-cross', 'hexagon-unknown-linux-musl-clang', '-Wno-incompatible-pointer-types', '-Wno-undefined-internal', '-fno-unroll-loops', '-fno-stack-protector', '-Wall', '-Werror', '-O0', '-g', '-fno-strict-aliasing', '-Wno-stringop-overread', '-mv73', '-O2', '-static', '/home/alex/lsrc/qemu.git/tests/tcg/multiarch/sha1.c', '-o', 'sha1', '-static']' returned non-zero exit status 1.
>>    filter=--filter=label=com.qemu.instance.uuid=5bbb7b6ed2ea4377b9b6d646859ec4ea
>>    make[1]: *** [Makefile:122: sha1] Error 1
>>    make: *** [/home/alex/lsrc/qemu.git/tests/Makefile.include:52: build-tcg-tests-hexagon-linux-user] Error 2
>>
>> Is it that new an option?
> I think it's new-ish (gcc 11?). On the other hand


I was going to volunteer to update the toolchain used for hexagon to 
address this.  But unfortunately this warning is still not supported in 
clang 21.

https://github.com/llvm/llvm-project/issues/72455 sounds like it's not 
very likely to arrive any time soon.


So "-Wno-unknown-warning-option" does indeed seem like a good workaround.


> -Wno-unknown-warning-option is quite old, and would suppress
> this error. If we do
>   CFLAGS+=-Wno-unknown-warning-option -Wno-stringop-overread
>
> does that work?
>
> (Meson has cc.get_supported_arguments() that we can use to
> filter out -Wfoo/-Wno-foo options that the compiler doesn't
> support, but since this is built via a makefile rather than
> by meson that's not conveniently accessible.)
>
> -- PMM
Peter Maydell March 5, 2025, 4:52 p.m. UTC | #11
On Wed, 5 Mar 2025 at 14:56, Brian Cain <brian.cain@oss.qualcomm.com> wrote:
> I was going to volunteer to update the toolchain used for hexagon to
> address this.  But unfortunately this warning is still not supported in
> clang 21.
>
> https://github.com/llvm/llvm-project/issues/72455 sounds like it's not
> very likely to arrive any time soon.
>
>
> So "-Wno-unknown-warning-option" does indeed seem like a good workaround.

I think the main problem with clang is that it doesn't default to
"don't complain about unknown warning options", whereas gcc does:

e104462:noble:qemu$ gcc -Wno-bang -o /tmp/hello /tmp/hello.c
e104462:noble:qemu$ clang -Wno-bang -o /tmp/hello /tmp/hello.c
warning: unknown warning option '-Wno-bang' [-Wunknown-warning-option]
1 warning generated.

-- PMM
Alex Bennée March 5, 2025, 4:56 p.m. UTC | #12
Brian Cain <brian.cain@oss.qualcomm.com> writes:

> On 3/4/2025 6:51 AM, Peter Maydell wrote:
>> On Tue, 4 Mar 2025 at 11:56, Alex Bennée <alex.bennee@linaro.org> wrote:
>>> Peter Maydell <peter.maydell@linaro.org> writes:
>>>> +# GCC versions 12/13/14/15 at least incorrectly complain about
>>>> +# "'SHA1Transform' reading 64 bytes from a region of size 0"; see the gcc bug
>>>> +# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106709
>>>> +# Since this is just a standard piece of library code we've borrowed for a
>>>> +# TCG test case, suppress the warning rather than trying to modify the
>>>> +# code to work around the compiler.
>>>> +sha1: CFLAGS+=-Wno-stringop-overread
>>>> +
>>> Sadly this breaks the hexagon compiler:
>>>
>>>    error: unknown warning option '-Wno-stringop-overread' [-Werror,-Wunknown-warning-option]
>>>    Traceback (most recent call last):
>>>      File "/home/alex/lsrc/qemu.git/tests/docker/docker.py", line 683, in <module>
>>>        sys.exit(main())
>>>                 ^^^^^^
>>>      File "/home/alex/lsrc/qemu.git/tests/docker/docker.py", line 679, in main
>>>        return args.cmdobj.run(args, argv)
>>>               ^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>      File "/home/alex/lsrc/qemu.git/tests/docker/docker.py", line 657, in run
>>>        return Docker().run(cmd, False, quiet=args.quiet,
>>>               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>      File "/home/alex/lsrc/qemu.git/tests/docker/docker.py", line 370, in run
>>>        ret = self._do_check(["run", "--rm", "--label",
>>>              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>      File "/home/alex/lsrc/qemu.git/tests/docker/docker.py", line 247, in _do_check
>>>        return subprocess.check_call(self._command + cmd, **kwargs)
>>>               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>      File "/usr/lib/python3.11/subprocess.py", line 413, in check_call
>>>        raise CalledProcessError(retcode, cmd)
>>>    subprocess.CalledProcessError: Command '['podman', 'run', '--rm', '--label', 'com.qemu.instance.uuid=5bbb7b6ed2ea4377b9b6d646859ec4ea', '--userns=keep-id', '-u', '1000', '-w', '/home/alex/lsrc/qemu.git/builds/all/tests/tcg/hexagon-linux-user', '-v', '/home/alex/lsrc/qemu.git/builds/all/tests/tcg/hexagon-linux-user:/home/alex/lsrc/qemu.git/builds/all/tests/tcg/hexagon-linux-user:rw', '-v', '/home/alex/lsrc/qemu.git:/home/alex/lsrc/qemu.git:ro,z', 'qemu/debian-hexagon-cross', 'hexagon-unknown-linux-musl-clang', '-Wno-incompatible-pointer-types', '-Wno-undefined-internal', '-fno-unroll-loops', '-fno-stack-protector', '-Wall', '-Werror', '-O0', '-g', '-fno-strict-aliasing', '-Wno-stringop-overread', '-mv73', '-O2', '-static', '/home/alex/lsrc/qemu.git/tests/tcg/multiarch/sha1.c', '-o', 'sha1', '-static']' returned non-zero exit status 1.
>>>    filter=--filter=label=com.qemu.instance.uuid=5bbb7b6ed2ea4377b9b6d646859ec4ea
>>>    make[1]: *** [Makefile:122: sha1] Error 1
>>>    make: *** [/home/alex/lsrc/qemu.git/tests/Makefile.include:52: build-tcg-tests-hexagon-linux-user] Error 2
>>>
>>> Is it that new an option?
>> I think it's new-ish (gcc 11?). On the other hand
>
>
> I was going to volunteer to update the toolchain used for hexagon to
> address this.  But unfortunately this warning is still not supported
> in clang 21.
>
> https://github.com/llvm/llvm-project/issues/72455 sounds like it's not
> very likely to arrive any time soon.
>
>
> So "-Wno-unknown-warning-option" does indeed seem like a good workaround.
>
>
>> -Wno-unknown-warning-option is quite old, and would suppress
>> this error. If we do
>>   CFLAGS+=-Wno-unknown-warning-option -Wno-stringop-overread

Workaround is fine for the time being then.
diff mbox series

Patch

diff --git a/tests/tcg/aarch64/Makefile.target b/tests/tcg/aarch64/Makefile.target
index 9efe2f81adf..16ddcf4f883 100644
--- a/tests/tcg/aarch64/Makefile.target
+++ b/tests/tcg/aarch64/Makefile.target
@@ -83,7 +83,8 @@  test-aes: CFLAGS += -O -march=armv8-a+aes
 test-aes: test-aes-main.c.inc
 
 # Vector SHA1
-sha1-vector: CFLAGS=-O3
+# Work around compiler false-positive warning, as we do for the 'sha1' test
+sha1-vector: CFLAGS=-O3 -Wno-stringop-overread
 sha1-vector: sha1.c
 	$(CC) $(CFLAGS) $(EXTRA_CFLAGS) $< -o $@ $(LDFLAGS)
 run-sha1-vector: sha1-vector run-sha1
diff --git a/tests/tcg/arm/Makefile.target b/tests/tcg/arm/Makefile.target
index 99a953b6671..6189d7a0e24 100644
--- a/tests/tcg/arm/Makefile.target
+++ b/tests/tcg/arm/Makefile.target
@@ -61,7 +61,8 @@  endif
 ARM_TESTS += commpage
 
 # Vector SHA1
-sha1-vector: CFLAGS=-O3
+# Work around compiler false-positive warning, as we do for the 'sha1' test
+sha1-vector: CFLAGS=-O3 -Wno-stringop-overread
 sha1-vector: sha1.c
 	$(CC) $(CFLAGS) $(EXTRA_CFLAGS) $< -o $@ $(LDFLAGS)
 run-sha1-vector: sha1-vector run-sha1
diff --git a/tests/tcg/multiarch/Makefile.target b/tests/tcg/multiarch/Makefile.target
index 688a6be203c..c769a7d69d9 100644
--- a/tests/tcg/multiarch/Makefile.target
+++ b/tests/tcg/multiarch/Makefile.target
@@ -45,6 +45,14 @@  vma-pthread: LDFLAGS+=-pthread
 sigreturn-sigmask: CFLAGS+=-pthread
 sigreturn-sigmask: LDFLAGS+=-pthread
 
+# GCC versions 12/13/14/15 at least incorrectly complain about
+# "'SHA1Transform' reading 64 bytes from a region of size 0"; see the gcc bug
+# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106709
+# Since this is just a standard piece of library code we've borrowed for a
+# TCG test case, suppress the warning rather than trying to modify the
+# code to work around the compiler.
+sha1: CFLAGS+=-Wno-stringop-overread
+
 # The vma-pthread seems very sensitive on gitlab and we currently
 # don't know if its exposing a real bug or the test is flaky.
 ifneq ($(GITLAB_CI),)