Re: [PATCH v3 8/8] tests/tcg: Add a test for info proc mappings

[Ilya Leoshkevich]

On Wed, 2023-06-21 at 11:21 +0100, Alex Bennée wrote:
> 
> Ilya Leoshkevich <iii@linux.ibm.com> writes:
> 
> > Add a small test to prevent regressions.
> > Since there are issues with how GDB interprets QEMU's target.xml,
> > enable the test only on aarch64 and s390x for now.
> > 
> > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > ---
> >  tests/tcg/aarch64/Makefile.target             |  3 +-
> >  tests/tcg/multiarch/Makefile.target           |  7 +++
> >  .../multiarch/gdbstub/test-proc-mappings.py   | 55
> > +++++++++++++++++++
> >  tests/tcg/s390x/Makefile.target               |  2 +-
> >  4 files changed, 65 insertions(+), 2 deletions(-)
> >  create mode 100644 tests/tcg/multiarch/gdbstub/test-proc-
> > mappings.py
> > 
> > diff --git a/tests/tcg/aarch64/Makefile.target
> > b/tests/tcg/aarch64/Makefile.target
> > index 03157954871..38402b0ba1f 100644
> > --- a/tests/tcg/aarch64/Makefile.target
> > +++ b/tests/tcg/aarch64/Makefile.target
> > @@ -97,7 +97,8 @@ run-gdbstub-sve-ioctls: sve-ioctls
> >                 --bin $< --test $(AARCH64_SRC)/gdbstub/test-sve-
> > ioctl.py, \
> >         basic gdbstub SVE ZLEN support)
> >  
> > -EXTRA_RUNS += run-gdbstub-sysregs run-gdbstub-sve-ioctls
> > +EXTRA_RUNS += run-gdbstub-sysregs run-gdbstub-sve-ioctls \
> > +              run-gdbstub-proc-mappings
> >  endif
> >  endif
> >  
> > diff --git a/tests/tcg/multiarch/Makefile.target
> > b/tests/tcg/multiarch/Makefile.target
> > index 373db696481..cbc0b75787a 100644
> > --- a/tests/tcg/multiarch/Makefile.target
> > +++ b/tests/tcg/multiarch/Makefile.target
> > @@ -81,6 +81,13 @@ run-gdbstub-qxfer-auxv-read: sha1
> >                 --bin $< --test $(MULTIARCH_SRC)/gdbstub/test-
> > qxfer-auxv-read.py, \
> >         basic gdbstub qXfer:auxv:read support)
> >  
> > +run-gdbstub-proc-mappings: sha1
> > +       $(call run-test, $@, $(GDB_SCRIPT) \
> > +               --gdb $(HAVE_GDB_BIN) \
> > +               --qemu $(QEMU) --qargs "$(QEMU_OPTS)" \
> > +               --bin $< --test $(MULTIARCH_SRC)/gdbstub/test-proc-
> > mappings.py, \
> > +       proc mappings support)
> > +
> 
> I wondered if it makes more sense to keep the extra test
> configuration
> logic in multiarch:
> 
>   run-gdbstub-proc-mappings: sha1
>           $(call run-test, $@, $(GDB_SCRIPT) \
>                   --gdb $(HAVE_GDB_BIN) \
>                   --qemu $(QEMU) --qargs "$(QEMU_OPTS)" \
>                   --bin $< --test $(MULTIARCH_SRC)/gdbstub/test-proc-
> mappings.py, \
>           proc mappings support)
> 
>   # only enable for s390x and aarch64 for now
>   ifneq (,$(findstring aarch64,$(TARGET_NAME)))
>   EXTRA_RUNS += run-gdbstub-proc-mappings
>   else ifneq (,$(findstring s390x,$(TARGET_NAME)))
>   EXTRA_RUNS += run-gdbstub-proc-mappings
>   endif
> 
> but it still ends up pretty ugly. Is the gdb handling fixed for other
> arches in other versions. Maybe we could probe gdb for support and
> wrap
> the whole stanza in something like:
> 
>   ifeq ($(HOST_GDB_SUPPORTS_PROC_MAPPING),y)
>   ...
>   endif
> 
> ?

I think I better add the check to the test itself, because otherwise we
have to probe GDB against QEMU binary we just built, which sounds
unnecessarily complicated.

The error message on all arches without this series is:

    warning: unable to open /proc file '/proc/1/maps'

The error message on x86_64 (expected) with this series is:

   Not supported on this target.

So we can simply exit(0) from the test if we see this.