On Mon, 2023-07-10 at 14:09 +0100, Richard Henderson wrote:
> On 7/10/23 13:15, Thomas Huth wrote:
> > From: Ilya Leoshkevich <i...@linux.ibm.com>
> >
> > Add a small test to prevent regressions.
> >
> > Signed-off-by: Ilya Leoshkevich <i...@linux.ibm.com>
> > Reviewed-by: David Hildenbrand <da...@redhat.com>
> > Message-Id: <20230704081506.276055-13-...@linux.ibm.com>
> > Signed-off-by: Thomas Huth <th...@redhat.com>
> > ---
> > tests/tcg/s390x/mie3-mvcrl.c | 46 ++++++++++++++++++++++++++++---
> > -----
> > 1 file changed, 36 insertions(+), 10 deletions(-)
> >
> > diff --git a/tests/tcg/s390x/mie3-mvcrl.c b/tests/tcg/s390x/mie3-
> > mvcrl.c
> > index 93c7b0a290..ec78dd1d49 100644
> > --- a/tests/tcg/s390x/mie3-mvcrl.c
> > +++ b/tests/tcg/s390x/mie3-mvcrl.c
> > @@ -1,29 +1,55 @@
> > +#include <stdbool.h>
> > #include <stdint.h>
> > +#include <stdlib.h>
> > #include <string.h>
> >
> > -
> > -static inline void mvcrl_8(const char *dst, const char *src)
> > +static void mvcrl(const char *dst, const char *src, size_t len)
> > {
> > + register long r0 asm("r0") = len;
> > +
> > asm volatile (
> > - "llill %%r0, 8\n"
> > ".insn sse, 0xE50A00000000, 0(%[dst]), 0(%[src])"
> > - : : [dst] "d" (dst), [src] "d" (src)
> > - : "r0", "memory");
> > + : : [dst] "d" (dst), [src] "d" (src), "r" (r0)
> > + : "memory");
> > }
> >
> > -
> > -int main(int argc, char *argv[])
> > +static bool test(void)
> > {
> > const char *alpha = "abcdefghijklmnop";
> >
> > /* array missing 'i' */
> > - char tstr[17] = "abcdefghjklmnop\0" ;
> > + char tstr[17] = "abcdefghjklmnop\0";
> >
> > /* mvcrl reference use: 'open a hole in an array' */
> > - mvcrl_8(tstr + 9, tstr + 8);
> > + mvcrl(tstr + 9, tstr + 8, 8);
> >
> > /* place missing 'i' */
> > tstr[8] = 'i';
> >
> > - return strncmp(alpha, tstr, 16ul);
> > + return strncmp(alpha, tstr, 16ul) == 0;
> > +}
> > +
> > +static bool test_bad_r0(void)
> > +{
> > + char src[256];
> > +
> > + /*
> > + * PoP says: Bits 32-55 of general register 0 should contain
> > zeros;
> > + * otherwise, the program may not operate compatibly in the
> > future.
> > + *
> > + * Try it anyway in order to check whether this would crash
> > QEMU itself.
> > + */
> > + mvcrl(src, src, (size_t)-1);
> > +
> > + return true;
> > +}
>
> gcc 11 doesn't like this:
>
> https://gitlab.com/qemu-project/qemu/-/jobs/4623964826#L3921
>
> /home/gitlab-runner/builds/E8PpwMky/0/qemu-
> project/qemu/tests/tcg/s390x/mie3-mvcrl.c: In
> function ‘test_bad_r0’:
> /home/gitlab-runner/builds/E8PpwMky/0/qemu-
> project/qemu/tests/tcg/s390x/mie3-mvcrl.c:42:5:
> error: ‘src’ may be used uninitialized [-Werror=maybe-uninitialized]
> 42 | mvcrl(src, src, (size_t)-1);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> /home/gitlab-runner/builds/E8PpwMky/0/qemu-
> project/qemu/tests/tcg/s390x/mie3-mvcrl.c:6:13:
> note: by argument 1 of type ‘const char *’ to ‘mvcrl’ declared here
> 6 | static void mvcrl(const char *dst, const char *src, size_t
> len)
> | ^~~~~
> /home/gitlab-runner/builds/E8PpwMky/0/qemu-
> project/qemu/tests/tcg/s390x/mie3-mvcrl.c:34:10:
> note: ‘src’ declared here
> 34 | char src[256];
> | ^~~
> /home/gitlab-runner/builds/E8PpwMky/0/qemu-
> project/qemu/tests/tcg/s390x/mie3-mvcrl.c:42:5:
> error: ‘src’ may be used uninitialized [-Werror=maybe-uninitialized]
> 42 | mvcrl(src, src, (size_t)-1);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> /home/gitlab-runner/builds/E8PpwMky/0/qemu-
> project/qemu/tests/tcg/s390x/mie3-mvcrl.c:6:13:
> note: by argument 2 of type ‘const char *’ to ‘mvcrl’ declared here
> 6 | static void mvcrl(const char *dst, const char *src, size_t
> len)
> | ^~~~~
> /home/gitlab-runner/builds/E8PpwMky/0/qemu-
> project/qemu/tests/tcg/s390x/mie3-mvcrl.c:34:10:
> note: ‘src’ declared here
> 34 | char src[256];
> | ^~~
> cc1: all warnings being treated as errors
>
> How it sees any use of the structure, initialized or otherwise, I
> don't know -- it's all
> hidden within the asm. However, src[256] = { } is enough to silence
> the error.
>
>
> r~
Thanks for having a look at this. I assume you applied this fixup, and
I don't need to send a follow-up patch?
