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

[Alex Bennée]

Ilya Leoshkevich <iii@linux.ibm.com> writes:

> 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.

That seems a simpler solution, lets do that.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro