Re: [PULL 13/21] tests/tcg/s390x: Test MVCRL with a large value in R0

[Richard Henderson <richard.henderson@linaro.org>]

On 7/10/23 13:15, Thomas Huth wrote:
> From: Ilya Leoshkevich <iii@linux.ibm.com>
> 
> Add a small test to prevent regressions.
> 
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> Reviewed-by: David Hildenbrand <david@redhat.com>
> Message-Id: <20230704081506.276055-13-iii@linux.ibm.com>
> Signed-off-by: Thomas Huth <thuth@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~