[PATCH 0/2] target/i386: Make sure that vsyscall's tb->size != 0

[PATCH 0/2] target/i386: Make sure that vsyscall's tb->size != 0

[Ilya Leoshkevich]

This series for the s390-next tree fixes [1]. Patch 1 is the fix, patch
2 is the new test to avoid similar regressions in the future.

[1] https://lists.nongnu.org/archive/html/qemu-devel/2021-05/msg02962.html

Ilya Leoshkevich (2):
  target/i386: Make sure that vsyscall's tb->size != 0
  tests/tcg/x86_64: add vsyscall smoke test

 target/i386/tcg/translate.c      |  1 +
 tests/tcg/x86_64/Makefile.target |  6 +++++-
 tests/tcg/x86_64/vsyscall.c      | 11 +++++++++++
 3 files changed, 17 insertions(+), 1 deletion(-)
 create mode 100644 tests/tcg/x86_64/vsyscall.c

-- 
2.29.2

[PATCH 1/2] target/i386: Make sure that vsyscall's tb->size != 0

[Ilya Leoshkevich]

tb_gen_code() assumes that tb->size must never be zero, otherwise it
may produce spurious exceptions. For x86_64 this may happen when
creating a translation block for the vsyscall page.

Fix by pretending that vsyscall translation blocks have at least one
instruction.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 target/i386/tcg/translate.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 880bc45561..b5a2b981e2 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -8572,6 +8572,7 @@ static void i386_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
      */
     if ((dc->base.pc_next & TARGET_PAGE_MASK) == TARGET_VSYSCALL_PAGE) {
         gen_exception(dc, EXCP_VSYSCALL, dc->base.pc_next);
+        dc->base.pc_next = dc->pc + 1;
         return;
     }
 #endif
-- 
2.29.2

Re: [PATCH 1/2] target/i386: Make sure that vsyscall's tb->size != 0

[Richard Henderson]

On 5/11/21 11:02 PM, Ilya Leoshkevich wrote:
> tb_gen_code() assumes that tb->size must never be zero, otherwise it
> may produce spurious exceptions. For x86_64 this may happen when
> creating a translation block for the vsyscall page.
> 
> Fix by pretending that vsyscall translation blocks have at least one
> instruction.
> 
> Signed-off-by: Ilya Leoshkevich<iii@linux.ibm.com>
> ---
>   target/i386/tcg/translate.c | 1 +
>   1 file changed, 1 insertion(+)

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

r~

[PATCH 2/2] tests/tcg/x86_64: add vsyscall smoke test

[Ilya Leoshkevich]

Having a small test will prevent trivial regressions in the future.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 tests/tcg/x86_64/Makefile.target |  6 +++++-
 tests/tcg/x86_64/vsyscall.c      | 11 +++++++++++
 2 files changed, 16 insertions(+), 1 deletion(-)
 create mode 100644 tests/tcg/x86_64/vsyscall.c

diff --git a/tests/tcg/x86_64/Makefile.target b/tests/tcg/x86_64/Makefile.target
index 20bf96202a..2151ea6302 100644
--- a/tests/tcg/x86_64/Makefile.target
+++ b/tests/tcg/x86_64/Makefile.target
@@ -3,14 +3,18 @@
 # x86_64 tests - included from tests/tcg/Makefile.target
 #
 # Currently we only build test-x86_64 and test-i386-ssse3 from
-# $(SRC)/tests/tcg/i386/
+# $(SRC_PATH)/tests/tcg/i386/
 #
 
 include $(SRC_PATH)/tests/tcg/i386/Makefile.target
 
+X86_64_TESTS += vsyscall
 TESTS=$(MULTIARCH_TESTS) $(X86_64_TESTS) test-x86_64
 QEMU_OPTS += -cpu max
 
 test-x86_64: LDFLAGS+=-lm -lc
 test-x86_64: test-i386.c test-i386.h test-i386-shift.h test-i386-muldiv.h
 	$(CC) $(CFLAGS) $< -o $@ $(LDFLAGS)
+
+vsyscall: $(SRC_PATH)/tests/tcg/x86_64/vsyscall.c
+	$(CC) $(CFLAGS) $< -o $@ $(LDFLAGS)
diff --git a/tests/tcg/x86_64/vsyscall.c b/tests/tcg/x86_64/vsyscall.c
new file mode 100644
index 0000000000..b9b7f87459
--- /dev/null
+++ b/tests/tcg/x86_64/vsyscall.c
@@ -0,0 +1,11 @@
+#include <stdio.h>
+#include <time.h>
+
+#define VSYSCALL_PAGE 0xffffffffff600000
+#define TIME_OFFSET 0x400
+typedef time_t (*time_func)(time_t *);
+
+int main(void)
+{
+    printf("%ld\n", ((time_func)(VSYSCALL_PAGE + TIME_OFFSET))(NULL));
+}
-- 
2.29.2

Re: [PATCH 2/2] tests/tcg/x86_64: add vsyscall smoke test

[Richard Henderson]

On 5/11/21 11:02 PM, Ilya Leoshkevich wrote:
> Having a small test will prevent trivial regressions in the future.
> 
> Signed-off-by: Ilya Leoshkevich<iii@linux.ibm.com>
> ---
>   tests/tcg/x86_64/Makefile.target |  6 +++++-
>   tests/tcg/x86_64/vsyscall.c      | 11 +++++++++++
>   2 files changed, 16 insertions(+), 1 deletion(-)
>   create mode 100644 tests/tcg/x86_64/vsyscall.c

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


r~

Re: [PATCH 2/2] tests/tcg/x86_64: add vsyscall smoke test

[Cornelia Huck]

On Wed, 12 May 2021 06:02:50 +0200
Ilya Leoshkevich <iii@linux.ibm.com> wrote:

> Having a small test will prevent trivial regressions in the future.
> 
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>  tests/tcg/x86_64/Makefile.target |  6 +++++-
>  tests/tcg/x86_64/vsyscall.c      | 11 +++++++++++
>  2 files changed, 16 insertions(+), 1 deletion(-)
>  create mode 100644 tests/tcg/x86_64/vsyscall.c

> diff --git a/tests/tcg/x86_64/vsyscall.c b/tests/tcg/x86_64/vsyscall.c
> new file mode 100644
> index 0000000000..b9b7f87459
> --- /dev/null
> +++ b/tests/tcg/x86_64/vsyscall.c
> @@ -0,0 +1,11 @@
> +#include <stdio.h>
> +#include <time.h>
> +
> +#define VSYSCALL_PAGE 0xffffffffff600000
> +#define TIME_OFFSET 0x400
> +typedef time_t (*time_func)(time_t *);
> +
> +int main(void)
> +{
> +    printf("%ld\n", ((time_func)(VSYSCALL_PAGE + TIME_OFFSET))(NULL));
> +}

Fails in https://gitlab.com/cohuck/qemu/-/jobs/1267727559 ("error:
control reaches end of non-void function").

Re: [PATCH 2/2] tests/tcg/x86_64: add vsyscall smoke test

[Ilya Leoshkevich]

On Mon, 2021-05-17 at 14:52 +0200, Cornelia Huck wrote:
> On Wed, 12 May 2021 06:02:50 +0200
> Ilya Leoshkevich <iii@linux.ibm.com> wrote:
> 
> > Having a small test will prevent trivial regressions in the future.
> > 
> > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > ---
> >  tests/tcg/x86_64/Makefile.target |  6 +++++-
> >  tests/tcg/x86_64/vsyscall.c      | 11 +++++++++++
> >  2 files changed, 16 insertions(+), 1 deletion(-)
> >  create mode 100644 tests/tcg/x86_64/vsyscall.c
> 
> > diff --git a/tests/tcg/x86_64/vsyscall.c
> > b/tests/tcg/x86_64/vsyscall.c
> > new file mode 100644
> > index 0000000000..b9b7f87459
> > --- /dev/null
> > +++ b/tests/tcg/x86_64/vsyscall.c
> > @@ -0,0 +1,11 @@
> > +#include <stdio.h>
> > +#include <time.h>
> > +
> > +#define VSYSCALL_PAGE 0xffffffffff600000
> > +#define TIME_OFFSET 0x400
> > +typedef time_t (*time_func)(time_t *);
> > +
> > +int main(void)
> > +{
> > +    printf("%ld\n", ((time_func)(VSYSCALL_PAGE +
> > TIME_OFFSET))(NULL));
> > +}
> 
> Fails in https://gitlab.com/cohuck/qemu/-/jobs/1267727559 ("error:
> control reaches end of non-void function").

I always thought that omitting return in main() is allowed, but
turns out this is a C99-only thing. I should have probably used `make
docker-all-tests` instead of just `make check` to catch this.

I will send a v2.