On Fri Nov 17, 2023 at 3:39 AM AEST, Alex Bennée wrote:
> The reason the ppc64 and s390x test where failing was because gdb
> hides them although they are still accessible via regnum. We can
> re-arrange the test a little bit and include these two arches in our
> test.
>
> We still don't explicitly fail for registers that just disappear like
> in the ARM case:
>
> xml-tdesc has 228 registers
> remote-registers has 219 registers
> of which 0 are hidden
> {'name': 'CNTP_CVAL', 'regnum': 96} wasn't seen in remote-registers
> {'name': 'CNTV_CVAL', 'regnum': 101} wasn't seen in remote-registers
> {'name': 'PAR', 'regnum': 113} wasn't seen in remote-registers
> {'name': 'CPUACTLR', 'regnum': 114} wasn't seen in remote-registers
> {'name': 'CPUECTLR', 'regnum': 127} wasn't seen in remote-registers
> {'name': 'CPUMERRSR', 'regnum': 140} wasn't seen in remote-registers
> {'name': 'TTBR1', 'regnum': 148} wasn't seen in remote-registers
> {'name': 'L2MERRSR', 'regnum': 161} wasn't seen in remote-registers
> {'name': 'TTBR0', 'regnum': 168} wasn't seen in remote-registers
Nice! Thanks for getting ppc64 working, comment below.
>
> Signed-off-by: Alex Bennée <alex.ben...@linaro.org>
> Cc: Ilya Leoshkevich <i...@linux.ibm.com>
> Cc: qemu-s3...@nongnu.org
> Cc: Nicholas Piggin <npig...@gmail.com>
> Cc: Daniel Henrique Barboza <danielhb...@gmail.com>
> Cc: qemu-...@nongnu.org
> Cc: Luis Machado <luis.mach...@arm.com>
> ---
> tests/tcg/multiarch/gdbstub/registers.py | 80 ++++++++++++++++++------
> tests/tcg/ppc64/Makefile.target | 7 ---
> tests/tcg/s390x/Makefile.target | 4 --
> 3 files changed, 61 insertions(+), 30 deletions(-)
>
> diff --git a/tests/tcg/multiarch/gdbstub/registers.py
> b/tests/tcg/multiarch/gdbstub/registers.py
> index ff6076b09e..342d6fd78f 100644
> --- a/tests/tcg/multiarch/gdbstub/registers.py
> +++ b/tests/tcg/multiarch/gdbstub/registers.py
> @@ -44,7 +44,6 @@ def fetch_xml_regmap():
>
> total_regs = 0
> reg_map = {}
> - frame = gdb.selected_frame()
>
> tree = ET.fromstring(xml)
> for f in tree.findall("feature"):
> @@ -61,12 +60,8 @@ def fetch_xml_regmap():
> for r in regs:
> name = r.attrib["name"]
> regnum = int(r.attrib["regnum"])
> - try:
> - value = frame.read_register(name)
> - except ValueError:
> - report(False, f"failed to read reg: {name}")
>
> - entry = { "name": name, "initial": value, "regnum": regnum }
> + entry = { "name": name, "regnum": regnum }
>
> if name in reg_map:
> report(False, f"duplicate register {entry} vs
> {reg_map[name]}")
> @@ -80,6 +75,15 @@ def fetch_xml_regmap():
>
> return reg_map
>
> +def get_register_by_regnum(reg_map, regnum):
> + """
> + Helper to find a register from the map via its XML regnum
> + """
> + for regname, entry in reg_map.items():
> + if entry['regnum'] == regnum:
> + return entry
> + return None
> +
> def crosscheck_remote_xml(reg_map):
> """
> Cross-check the list of remote-registers with the XML info.
> @@ -90,6 +94,7 @@ def crosscheck_remote_xml(reg_map):
>
> total_regs = len(reg_map.keys())
> total_r_regs = 0
> + total_r_elided_regs = 0
>
> for r in r_regs:
> fields = r.split()
> @@ -100,6 +105,15 @@ def crosscheck_remote_xml(reg_map):
> r_name = fields[0]
> r_regnum = int(fields[6])
>
> + # Some registers are "hidden" so don't have a name
> + # although they still should have a register number
> + if r_name == "''":
> + total_r_elided_regs += 1
Should this also increment total_r_regs so that it doesn't trip the
warning?
Thanks,
Nick
> + x_reg = get_register_by_regnum(reg_map, r_regnum)
> + if x_reg is not None:
> + x_reg["hidden"] = True
> + continue
> +
> # check in the XML
> try:
> x_reg = reg_map[r_name]
> @@ -118,14 +132,40 @@ def crosscheck_remote_xml(reg_map):
> # registers on a 32 bit machine. Also print what is missing to
> # help with debug.
> if total_regs != total_r_regs:
> - print(f"xml-tdesc has ({total_regs}) registers")
> - print(f"remote-registers has ({total_r_regs}) registers")
> + print(f"xml-tdesc has {total_regs} registers")
> + print(f"remote-registers has {total_r_regs} registers")
> + print(f"of which {total_r_elided_regs} are hidden")
>
> for x_key in reg_map.keys():
> x_reg = reg_map[x_key]
> - if "seen" not in x_reg:
> + if "hidden" in x_reg:
> + print(f"{x_reg} elided by gdb")
> + elif "seen" not in x_reg:
> print(f"{x_reg} wasn't seen in remote-registers")
>
> +def initial_register_read(reg_map):
> + """
> + Do an initial read of all registers that we know gdb cares about
> + (so ignore the elided ones).
> + """
> + frame = gdb.selected_frame()
> +
> + for e in reg_map.values():
> + name = e["name"]
> + regnum = e["regnum"]
> +
> + try:
> + if "hidden" in e:
> + value = frame.read_register(regnum)
> + e["initial"] = value
> + elif "seen" in e:
> + value = frame.read_register(name)
> + e["initial"] = value
> +
> + except ValueError:
> + report(False, f"failed to read reg: {name}")
> +
> +
> def complete_and_diff(reg_map):
> """
> Let the program run to (almost) completion and then iterate
> @@ -144,18 +184,19 @@ def complete_and_diff(reg_map):
> changed = 0
>
> for e in reg_map.values():
> - name = e["name"]
> - old_val = e["initial"]
> + if "initial" in e and "hidden" not in e:
> + name = e["name"]
> + old_val = e["initial"]
>
> - try:
> - new_val = frame.read_register(name)
> - except:
> - report(False, f"failed to read {name} at end of run")
> - continue
> + try:
> + new_val = frame.read_register(name)
> + except ValueError:
> + 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
> + 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")
> @@ -168,6 +209,7 @@ def run_test():
>
> if reg_map is not None:
> crosscheck_remote_xml(reg_map)
> + initial_register_read(reg_map)
> complete_and_diff(reg_map)
>
>
> diff --git a/tests/tcg/ppc64/Makefile.target b/tests/tcg/ppc64/Makefile.target
> index 1d08076756..5721c159f2 100644
> --- a/tests/tcg/ppc64/Makefile.target
> +++ b/tests/tcg/ppc64/Makefile.target
> @@ -38,11 +38,4 @@ PPC64_TESTS += signal_save_restore_xer
> PPC64_TESTS += xxspltw
> PPC64_TESTS += test-aes
>
> -ifneq ($(GDB),)
> -# Skip for now until vsx registers sorted out
> -run-gdbstub-registers:
> - $(call skip-test, $<, "BROKEN reading VSX registers")
> -endif
> -
> -
> TESTS += $(PPC64_TESTS)
> diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
> index 46544fecd4..0e670f3f8b 100644
> --- a/tests/tcg/s390x/Makefile.target
> +++ b/tests/tcg/s390x/Makefile.target
> @@ -103,10 +103,6 @@ run-gdbstub-svc: hello-s390x-asm
> --bin $< --test $(S390X_SRC)/gdbstub/test-svc.py, \
> single-stepping svc)
>
> -# Skip for now until vx registers sorted out
> -run-gdbstub-registers:
> - $(call skip-test, $<, "BROKEN reading VX registers")
> -
> EXTRA_RUNS += run-gdbstub-signals-s390x run-gdbstub-svc
> endif
>
