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

[Thomas Huth <thuth@redhat.com>]

On 10/07/2023 15.13, Ilya Leoshkevich wrote:
> On Mon, 2023-07-10 at 14:09 +0100, Richard Henderson wrote:
>> 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~
> 
> Thanks for having a look at this. I assume you applied this fixup, and
> I don't need to send a follow-up patch?

No need to respin the patch, I'll fix it up and send a v2 pull request.

  Thomas