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 <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?
No need to respin the patch, I'll fix it up and send a v2 pull request. Thomas
