Re: [PATCH v4 8/9] tests/functional: Adapt reverse_debugging to run w/o Avocado

[Thomas Huth <thuth@redhat.com>]

On 26/09/2025 07.15, Gustavo Romero wrote:
> This commit removes Avocado as a dependency for running the
> reverse_debugging test.
> 
> The main benefit, beyond eliminating an extra dependency, is that there
> is no longer any need to handle GDB packets manually. This removes the
> need for ad-hoc functions dealing with endianness and arch-specific
> register numbers, making the test easier to read. The timeout variable
> is also removed, since Meson now manages timeouts automatically.
> 
> reverse_debugging now uses the pygdbmi module to interact with GDB, if
> it's available in the test environment, otherwise the test is skipped.
> GDB is detect via the QEMU_TEST_GDB env. variable.
> 
> This commit also significantly improves the output for the test and
> now prints all the GDB commands used in sequence. It also adds
> some clarifications to existing comments, for example, clarifying that
> once the replay-break is reached, a SIGINT is captured in GDB.
> 
> reverse_debugging is kept "skipped" for aarch64, ppc64, and x86_64, so
> won't run unless QEMU_TEST_FLAKY_TESTS=1 is set in the test environment,
> before running 'make check-functional' or 'meson test [...]'.
> 
> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
> ---
...
> @@ -53,49 +55,11 @@ def run_vm(self, record, shift, args, replay_path, image_path, port):
>           vm.launch()
>           return vm
>   
> -    @staticmethod
> -    def get_reg_le(g, reg):
> -        res = g.cmd(b'p%x' % reg)
> -        num = 0
> -        for i in range(len(res))[-2::-2]:
> -            num = 0x100 * num + int(res[i:i + 2], 16)
> -        return num
> -
> -    @staticmethod
> -    def get_reg_be(g, reg):
> -        res = g.cmd(b'p%x' % reg)
> -        return int(res, 16)
> -
> -    def get_reg(self, g, reg):
> -        # value may be encoded in BE or LE order
> -        if self.endian_is_le:
> -            return self.get_reg_le(g, reg)
> -        else:
> -            return self.get_reg_be(g, reg)
> -
> -    def get_pc(self, g):
> -        return self.get_reg(g, self.REG_PC)
> -
> -    def check_pc(self, g, addr):
> -        pc = self.get_pc(g)
> -        if pc != addr:
> -            self.fail('Invalid PC (read %x instead of %x)' % (pc, addr))

I think it would make sense to keep wrapper functions get_pc() and 
check_pc(), since that functionality is still used in a bunch of places 
(e.g. "gdb.cli("print $pc").get_addr()" is used a couple of times).

> @@ -124,68 +88,99 @@ def reverse_debugging(self, shift=7, args=None):
>           with Ports() as ports:
>               port = ports.find_free_port()
>               vm = self.run_vm(False, shift, args, replay_path, image_path, port)
> -        logger.info('connecting to gdbstub')
> -        g = gdb.GDBRemote('127.0.0.1', port, False, False)
> -        g.connect()
> -        r = g.cmd(b'qSupported')
> -        if b'qXfer:features:read+' in r:
> -            g.cmd(b'qXfer:features:read:target.xml:0,ffb')
> -        if b'ReverseStep+' not in r:
> +
> +        try:
> +            logger.info('Connecting to gdbstub...')
> +            self.reverse_debugging_run(vm, port, last_icount)
> +            logger.info('Test passed.')
> +        except GdbTimeoutError:
> +            self.fail("Connection to gdbstub timeouted...")

I'm not a native English speaker, but I think "timeout" is not a verb. So 
maybe better: "Timeout while connecting to gdbstub" or something similar?

> +    @skipIfMissingEnv("QEMU_TEST_GDB")

Somehow this SKIP is always triggered on my system, even if I correctly 
pointed "configure" to the right GDB with the "--gdb" option ... not sure 
why this happens, but I'll try to find out...

  Thomas