[RFC] tests/tcg: add an explicit gdbstub register tester

We already do a couple of "info registers" for specific tests but this
is a more comprehensive multiarch test. It also has some output
helpful for debugging the gdbstub by showing which XML features are
advertised and what the underlying register numbers are.

My initial motivation was to see if there are any duplicate register
names exposed via the gdbstub while I was reviewing the proposed
register interface for TCG plugins.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Cc: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 tests/tcg/multiarch/Makefile.target           |  10 +-
 tests/tcg/multiarch/gdbstub/registers.py      | 173 ++++++++++++++++++
 .../multiarch/system/Makefile.softmmu-target  |  14 +-
 3 files changed, 195 insertions(+), 2 deletions(-)
 create mode 100644 tests/tcg/multiarch/gdbstub/registers.py

On 2023/10/13 2:04, Alex Bennée wrote:
> We already do a couple of "info registers" for specific tests but this
> is a more comprehensive multiarch test. It also has some output
> helpful for debugging the gdbstub by showing which XML features are
> advertised and what the underlying register numbers are.
> 
> My initial motivation was to see if there are any duplicate register
> names exposed via the gdbstub while I was reviewing the proposed
> register interface for TCG plugins.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Akihiko Odaki <akihiko.odaki@daynix.com>

I have some comments for some implementation details, but the intention 
sounds good to me.

> ---
>   tests/tcg/multiarch/Makefile.target           |  10 +-
>   tests/tcg/multiarch/gdbstub/registers.py      | 173 ++++++++++++++++++
>   .../multiarch/system/Makefile.softmmu-target  |  14 +-
>   3 files changed, 195 insertions(+), 2 deletions(-)
>   create mode 100644 tests/tcg/multiarch/gdbstub/registers.py
> 
> diff --git a/tests/tcg/multiarch/Makefile.target b/tests/tcg/multiarch/Makefile.target
> index 43bddeaf21..d5e05ac343 100644
> --- a/tests/tcg/multiarch/Makefile.target
> +++ b/tests/tcg/multiarch/Makefile.target
> @@ -95,6 +95,13 @@ run-gdbstub-thread-breakpoint: testthread
>   		--bin $< --test $(MULTIARCH_SRC)/gdbstub/test-thread-breakpoint.py, \
>   	hitting a breakpoint on non-main thread)
>   
> +run-gdbstub-registers: sha512
> +	$(call run-test, $@, $(GDB_SCRIPT) \
> +		--gdb $(HAVE_GDB_BIN) \
> +		--qemu $(QEMU) --qargs "$(QEMU_OPTS)" \
> +		--bin $< --test $(MULTIARCH_SRC)/gdbstub/registers.py, \
> +	checking register enumeration)
> +
>   else
>   run-gdbstub-%:
>   	$(call skip-test, "gdbstub test $*", "no guest arch support")
> @@ -104,7 +111,8 @@ run-gdbstub-%:
>   	$(call skip-test, "gdbstub test $*", "need working gdb")
>   endif
>   EXTRA_RUNS += run-gdbstub-sha1 run-gdbstub-qxfer-auxv-read \
> -	      run-gdbstub-proc-mappings run-gdbstub-thread-breakpoint
> +	      run-gdbstub-proc-mappings run-gdbstub-thread-breakpoint \
> +	      run-gdbstub-registers
>   
>   # ARM Compatible Semi Hosting Tests
>   #
> diff --git a/tests/tcg/multiarch/gdbstub/registers.py b/tests/tcg/multiarch/gdbstub/registers.py
> new file mode 100644
> index 0000000000..7f331082cb
> --- /dev/null
> +++ b/tests/tcg/multiarch/gdbstub/registers.py
> @@ -0,0 +1,173 @@
> +from __future__ import print_function

I think this and other lines are just copied from the other tests, but I 
leave some comments anyway:
This line is no longer necessary since Python 2 is already dead.

> +#
> +# Exercise the register functionality by exhaustively iterating
> +# through all supported registers on the system.
> +#
> +# This is launched via tests/guest-debug/run-test.py but you can also
> +# call it directly if using it for debugging/introspection:
> +#
> +#
> +#

Let's have SPDX-License-Identifier and remove supurious lines that only 
have #.

> +
> +import gdb
> +import sys
> +import xml.etree.ElementTree as ET
> +
> +initial_vlen = 0
> +failcount = 0
> +
> +def report(cond, msg):
> +    "Report success/fail of test."
> +    if cond:
> +        print("PASS: %s" % (msg))
> +    else:
> +        print("FAIL: %s" % (msg))
> +        global failcount
> +        failcount += 1

What about using Python's unittest?

> +
> +
> +def fetch_xml_regmap():
> +    """
> +    Iterate through the XML descriptions and validate.
> +
> +    We check for any duplicate registers and report them. Return a
> +    reg_map hash containing the names, regnums and initial values of
> +    all registers.
> +    """
> +
> +    total_regs = 0
> +    reg_map = {}
> +    frame = gdb.selected_frame()
> +
> +    # First check the XML descriptions we have sent
> +    xml = gdb.execute("maint print xml-tdesc", False, True)
> +    tree = ET.fromstring(xml)
> +    for f in tree.findall("feature"):
> +        name = f.attrib["name"]
> +        regs = f.findall("reg")
> +
> +        total = len(regs)
> +        total_regs += total
> +        base = int(regs[0].attrib["regnum"])
> +        top = int(regs[-1].attrib["regnum"])
> +
> +        print(f"feature: {name} has {total} registers from {base} to {top}")
> +
> +        for r in regs:
> +            name = r.attrib["name"]
> +            value = frame.read_register(name).__str__()
> +            regnum = int(r.attrib["regnum"])
> +            entry = { "name": name,
> +                      "initial": value,
> +                      "regnum": regnum }
> +            try:
> +                reg_map[name] = entry
> +            except KeyError:
> +                report(False, f"duplicate register {r} vs {reg_map[name]}")

I don't think python raises KeyError for duplicate key.

> +
> +    # Validate we match
> +    report(total_regs == len(reg_map.keys()),
> +           f"counted all {total_regs} registers in XML")
> +
> +    return reg_map
> +
> +def crosscheck_remote_xml(reg_map):
> +    """
> +    Cross-check the list of remote-registers with the XML info.
> +    """
> +
> +    remote = gdb.execute("maint print remote-registers", False, True)
> +    r_regs = remote.split("\n")
> +
> +    total_regs = len(reg_map.keys())
> +    total_r_regs = 0
> +
> +    for r in r_regs:
> +        fields = r.split()
> +        # Some of the registers reported here are "pseudo" registers that
> +        # gdb invents based on actual registers so we need to filter them
> +        # out.
> +        if len(fields) == 8:
> +            r_name = fields[0]
> +            r_regnum = int(fields[1])
> +
> +            # check in the XML
> +            try:
> +                x_reg = reg_map[r_name]
> +            except KeyError:
> +                report(False, "{r_name} not in XML description")
> +                continue
> +
> +            x_regnum = x_reg["regnum"]
> +            if r_regnum != x_regnum:
> +                report(False, f"{r_name} {r_regnum} == {x_regnum} (xml)")
> +            else:
> +                total_r_regs += 1
> +
> +    report(total_regs == total_r_regs, f"xml-tdesc and remote-registers agree")
> +
> +def complete_and_diff(reg_map):
> +    """
> +    Let the program run to (almost) completion and then iterate
> +    through all the registers we know about and report which ones have
> +    changed.
> +    """
> +    # Let the program get to the end and we can check what changed
> +    gdb.Breakpoint("_exit")
> +    gdb.execute("continue")
> +
> +    frame = gdb.selected_frame()
> +    changed = 0
> +
> +    for e in reg_map.values():
> +        name = e["name"]
> +        old_val = e["initial"]
> +
> +        try:
> +            new_val = frame.read_register(name).__str__()
> +        except:
> +            report(False, f"failed to read {name} at end of run")
> +            continue
> +
> +        if new_val != old_val:
> +            print(f"{name} changes from {old_val} to {new_val}")
> +            changed += 1
> +
> +    # as long as something changed we can be confident its working
> +    report(changed > 0, f"{changed} registers were changed")
> +
> +
> +def run_test():
> +    "Run through the tests"
> +
> +    reg_map = fetch_xml_regmap()
> +
> +    crosscheck_remote_xml(reg_map)
> +
> +    complete_and_diff(reg_map)
> +
> +
> +#
> +# This runs as the script it sourced (via -x, via run-test.py)
> +#
> +try:
> +    inferior = gdb.selected_inferior()
> +    arch = inferior.architecture()
> +    print("ATTACHED: %s" % arch.name())
> +except (gdb.error, AttributeError):
> +    print("SKIPPING (not connected)", file=sys.stderr)
> +    exit(0)
> +
> +if gdb.parse_and_eval('$pc') == 0:
> +    print("SKIP: PC not set")
> +    exit(0)
> +
> +try:
> +    run_test()
> +except (gdb.error):
> +    print ("GDB Exception: %s" % (sys.exc_info()[0]))
> +    failcount += 1
> +    pass
> +
> +print("All tests complete: %d failures" % failcount)
> +exit(failcount)
> diff --git a/tests/tcg/multiarch/system/Makefile.softmmu-target b/tests/tcg/multiarch/system/Makefile.softmmu-target
> index 90810a32b2..dc97d71e42 100644
> --- a/tests/tcg/multiarch/system/Makefile.softmmu-target
> +++ b/tests/tcg/multiarch/system/Makefile.softmmu-target
> @@ -27,6 +27,17 @@ run-gdbstub-memory: memory
>   		"-monitor none -display none -chardev file$(COMMA)path=$<.out$(COMMA)id=output $(QEMU_OPTS)" \
>   		--bin $< --test $(MULTIARCH_SRC)/gdbstub/memory.py, \
>   	softmmu gdbstub support)
> +
> +run-gdbstub-registers: memory
> +	$(call run-test, $@, $(GDB_SCRIPT) \
> +		--gdb $(HAVE_GDB_BIN) \
> +		--qemu $(QEMU) \
> +		--output $<.registers.gdb.out \
> +		--qargs \
> +		"-monitor none -display none -chardev file$(COMMA)path=$<.out$(COMMA)id=output $(QEMU_OPTS)" \
> +		--bin $< --test $(MULTIARCH_SRC)/gdbstub/registers.py, \
> +	softmmu gdbstub support)
> +
>   run-gdbstub-interrupt: interrupt
>   	$(call run-test, $@, $(GDB_SCRIPT) \
>   		--gdb $(HAVE_GDB_BIN) \
> @@ -58,4 +69,5 @@ run-gdbstub-%:
>   	$(call skip-test, "gdbstub test $*", "need working gdb")
>   endif
>   
> -MULTIARCH_RUNS += run-gdbstub-memory run-gdbstub-interrupt run-gdbstub-untimely-packet
> +MULTIARCH_RUNS += run-gdbstub-memory run-gdbstub-registers \
> +		  run-gdbstub-interrupt run-gdbstub-untimely-packet