Re: [RFC PATCH 2/2] tests/functional: Adapt reverse_debugging to run w/o Avocado

[Gustavo Romero <gustavo.romero@linaro.org>]

Hi Thomas,

On 9/15/25 09:42, Thomas Huth wrote:
> From: Gustavo Romero <gustavo.romero@linaro.org>
> 
> 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.
> 
> The reverse_debugging test is now executed through running GDB via a
> python script. The test itself is only responsible for invoking
> GDB with the appropriate arguments and for passing the test script to
> 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>
> [thuth: Rework the test to run without tests/guest-debug/run-test.py]
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   .../functional/aarch64/test_reverse_debug.py  |  16 +-
>   tests/functional/ppc64/test_reverse_debug.py  |  18 +-
>   tests/functional/reverse_debugging.py         | 235 +++++++++++-------
>   tests/functional/x86_64/test_reverse_debug.py |  20 +-
>   4 files changed, 171 insertions(+), 118 deletions(-)
> 
> diff --git a/tests/functional/aarch64/test_reverse_debug.py b/tests/functional/aarch64/test_reverse_debug.py
> index 85e35645db0..a84ddd07acb 100755
> --- a/tests/functional/aarch64/test_reverse_debug.py
> +++ b/tests/functional/aarch64/test_reverse_debug.py
> @@ -2,7 +2,7 @@
>   #
>   # SPDX-License-Identifier: GPL-2.0-or-later
>   #
> -# Reverse debugging test
> +# Reverse debugging test for aarch64
>   #
>   # Copyright (c) 2020 ISP RAS
>   #
> @@ -12,14 +12,13 @@
>   # This work is licensed under the terms of the GNU GPL, version 2 or
>   # later.  See the COPYING file in the top-level directory.
>   
> -from qemu_test import Asset, skipIfMissingImports, skipFlakyTest
> -from reverse_debugging import ReverseDebugging
> +from qemu_test import QemuSystemTest
> +from qemu_test import Asset, skipFlakyTest
>   
> +from reverse_debugging import reverse_debug
>   
> -@skipIfMissingImports('avocado.utils')
> -class ReverseDebugging_AArch64(ReverseDebugging):
>   
> -    REG_PC = 32
> +class ReverseDebugging_AArch64(QemuSystemTest):
>   
>       ASSET_KERNEL = Asset(
>           ('https://archives.fedoraproject.org/pub/archive/fedora/linux/'
> @@ -29,9 +28,8 @@ class ReverseDebugging_AArch64(ReverseDebugging):
>       def test_aarch64_virt(self):
>           self.set_machine('virt')
>           self.cpu = 'cortex-a53'
> -        kernel_path = self.ASSET_KERNEL.fetch()
> -        self.reverse_debugging(args=('-kernel', kernel_path))
> +        reverse_debug(self, self.ASSET_KERNEL)
>   
>   
>   if __name__ == '__main__':
> -    ReverseDebugging.main()
> +    QemuSystemTest.main()
> diff --git a/tests/functional/ppc64/test_reverse_debug.py b/tests/functional/ppc64/test_reverse_debug.py
> index 5931adef5a9..7da5ede06c8 100755
> --- a/tests/functional/ppc64/test_reverse_debug.py
> +++ b/tests/functional/ppc64/test_reverse_debug.py
> @@ -2,7 +2,7 @@
>   #
>   # SPDX-License-Identifier: GPL-2.0-or-later
>   #
> -# Reverse debugging test
> +# Reverse debugging test for ppc64
>   #
>   # Copyright (c) 2020 ISP RAS
>   #
> @@ -12,14 +12,12 @@
>   # This work is licensed under the terms of the GNU GPL, version 2 or
>   # later.  See the COPYING file in the top-level directory.
>   
> -from qemu_test import skipIfMissingImports, skipFlakyTest
> -from reverse_debugging import ReverseDebugging
> +from qemu_test import QemuSystemTest, skipFlakyTest
>   
> +from reverse_debugging import reverse_debug
>   
> -@skipIfMissingImports('avocado.utils')
> -class ReverseDebugging_ppc64(ReverseDebugging):
>   
> -    REG_PC = 0x40
> +class ReverseDebugging_ppc64(QemuSystemTest):
>   
>       @skipFlakyTest("https://gitlab.com/qemu-project/qemu/-/issues/1992")
>       def test_ppc64_pseries(self):
> @@ -27,15 +25,13 @@ def test_ppc64_pseries(self):
>           # SLOF branches back to its entry point, which causes this test
>           # to take the 'hit a breakpoint again' path. That's not a problem,
>           # just slightly different than the other machines.
> -        self.endian_is_le = False
> -        self.reverse_debugging()
> +        reverse_debug(self)
>   
>       @skipFlakyTest("https://gitlab.com/qemu-project/qemu/-/issues/1992")
>       def test_ppc64_powernv(self):
>           self.set_machine('powernv')
> -        self.endian_is_le = False
> -        self.reverse_debugging()
> +        reverse_debug(self)
>   
>   
>   if __name__ == '__main__':
> -    ReverseDebugging.main()
> +    QemuSystemTest.main()
> diff --git a/tests/functional/reverse_debugging.py b/tests/functional/reverse_debugging.py
> index f9a1d395f1d..c889247defa 100644
> --- a/tests/functional/reverse_debugging.py
> +++ b/tests/functional/reverse_debugging.py
> @@ -1,21 +1,72 @@
> -# Reverse debugging test
> -#
>   # SPDX-License-Identifier: GPL-2.0-or-later
>   #
> +# Reverse debugging test
> +#
>   # Copyright (c) 2020 ISP RAS
> +# Copyright (c) 2025 Linaro Limited
>   #
>   # Author:
>   #  Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
> +#  Gustavo Romero <gustavo.romero@linaro.org> (Run without Avocado)
>   #
>   # This work is licensed under the terms of the GNU GPL, version 2 or
>   # later.  See the COPYING file in the top-level directory.
> -import os
> +
>   import logging
> +import os
> +import subprocess
> +import sys
> +
> +try:
> +    import gdb
> +    _has_gdb = True
> +except ModuleNotFoundError:
> +    _has_gdb = False

nit: maybe instead of '_has_gdb' name it '_in_gdb' (for running inside gdb),
since if we have or not GDB in the system is part of our GDB detection in
configure, QEMU_TEST_GDB, etc?



>   from qemu_test import LinuxKernelTest, get_qemu_img
>   from qemu_test.ports import Ports
>   
>   
> +def reverse_debug(test, asset_kernel=None, shift=7, args=None):
> +
> +    # Now launch gdb with our test and collect the result.
> +    gdb_cmd = os.getenv('QEMU_TEST_GDB')
> +    assert(gdb_cmd)

I think we should skip the test if no GDB is available instead of failing it?

So how about something like:

@@ -31,7 +31,8 @@ def reverse_debug(test, asset_kernel=None, shift=7, args=None):
  
      # Now launch gdb with our test and collect the result.
      gdb_cmd = os.getenv('QEMU_TEST_GDB')
-    assert(gdb_cmd)
+    if not gdb_cmd:
+        test.skipTest("Test skipped because there is no GDB available!")


> +    # Run quietly and ignore .gdbinit.
> +    gdb_cmd += " -q -n -batch"
> +    # Disable pagination.
> +    gdb_cmd += " -ex 'set pagination off'"
> +    # Disable prompts in case of crash.
> +    gdb_cmd += " -ex 'set confirm off'"
> +    # Finally the test script itself.
> +    argv = [__file__]
> +    gdb_cmd += f" -ex \"py sys.argv={argv}\""
> +    gdb_cmd += " -x %s" % __file__
> +
> +    test.log.info("GDB CMD: %s" % gdb_cmd)
> +
> +    gdb_env = dict(os.environ)
> +    gdb_pythonpath = gdb_env.get("PYTHONPATH", "").split(os.pathsep)
> +    gdb_pythonpath.append(os.path.dirname(os.path.realpath(__file__)))
> +    gdb_env["PYTHONPATH"] = os.pathsep.join(gdb_pythonpath)
> +    gdb_env["QEMU_TEST_MACHINE"] = test.machine
> +    if test.cpu:
> +        gdb_env["QEMU_TEST_CPU"] = test.cpu
> +    if asset_kernel:
> +        gdb_env["QEMU_TEST_KERNEL"] = asset_kernel.fetch()
> +    result = subprocess.run(gdb_cmd, shell=True, check=False,
> +                            stdout=subprocess.PIPE,
> +                            stderr=subprocess.STDOUT,
> +                            encoding='utf8',
> +                            env=gdb_env)
> +    test.log.info("gdb output:\n %s" % result.stdout)
> +    if result.returncode != 0:
> +        test.fail(f"gdb failed with return code {result.returncode}")
> +    else:
> +        test.log.info("gdb run succeeded!")
> +
> +
>   class ReverseDebugging(LinuxKernelTest):
>       """
>       Test GDB reverse debugging commands: reverse step and reverse continue.
> @@ -28,13 +79,9 @@ class ReverseDebugging(LinuxKernelTest):
>       that the execution is stopped at the last of them.
>       """
>   
> -    timeout = 10
>       STEPS = 10
> -    endian_is_le = True
>   
>       def run_vm(self, record, shift, args, replay_path, image_path, port):
> -        from avocado.utils import datadrainer
> -
>           logger = logging.getLogger('replay')
>           vm = self.get_vm(name='record' if record else 'replay')
>           vm.set_console()
> @@ -52,59 +99,58 @@ def run_vm(self, record, shift, args, replay_path, image_path, port):
>           if args:
>               vm.add_args(*args)
>           vm.launch()
> -        console_drainer = datadrainer.LineLogger(vm.console_socket.fileno(),
> -                                    logger=self.log.getChild('console'),
> -                                    stop_check=(lambda : not vm.is_running()))
> -        console_drainer.start()
> +
>           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
> +    def gdb_connect(host, port):
> +        # Set debug on connection to get the qSupport string.
> +        gdb.execute("set debug remote 1")
> +        r = gdb.execute(f"target remote {host}:{port}", False, True)
> +        gdb.execute("set debug remote 0")
> +
> +        return r
>   
>       @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():
> +        val = gdb.parse_and_eval("$pc")
> +        pc = int(val)
>   
> -    def get_pc(self, g):
> -        return self.get_reg(g, self.REG_PC)
> +        return pc
>   
> -    def check_pc(self, g, addr):
> -        pc = self.get_pc(g)
> +    def check_pc(self, addr):
> +        logger = logging.getLogger('reply')
> +        pc = self.get_pc()
>           if pc != addr:
> -            self.fail('Invalid PC (read %x instead of %x)' % (pc, addr))
> +            logger.info('Invalid PC (read %x instead of %x)' % (pc, addr))
> +            gdb.execute("exit 1")
>   
>       @staticmethod
> -    def gdb_step(g):
> -        g.cmd(b's', b'T05thread:01;')
> +    def gdb_step():
> +        gdb.execute("stepi")
>   
>       @staticmethod
> -    def gdb_bstep(g):
> -        g.cmd(b'bs', b'T05thread:01;')
> +    def gdb_bstep():
> +        gdb.execute("reverse-stepi")
>   
>       @staticmethod
>       def vm_get_icount(vm):
>           return vm.qmp('query-replay')['return']['icount']
>   
> -    def reverse_debugging(self, shift=7, args=None):
> -        from avocado.utils import gdb
> -        from avocado.utils import process
> +    def test_reverse_debugging(self):
> +
> +        shift = 7
> +
> +        self.set_machine(os.getenv('QEMU_TEST_MACHINE'))
> +        self.cpu = os.getenv('QEMU_TEST_CPU')
> +        kernel_path = os.getenv('QEMU_TEST_KERNEL')
> +        args = None
> +        if kernel_path:
> +            args = ['-kernel', kernel_path]
>   
>           logger = logging.getLogger('replay')
>   
> -        # create qcow2 for snapshots
> +        # Create qcow2 for snapshots
>           logger.info('creating qcow2 image for VM snapshots')
>           image_path = os.path.join(self.workdir, 'disk.qcow2')
>           qemu_img = get_qemu_img(self)
> @@ -112,11 +158,11 @@ def reverse_debugging(self, shift=7, args=None):
>               self.skipTest('Could not find "qemu-img", which is required to '
>                             'create the temporary qcow2 image')
>           cmd = '%s create -f qcow2 %s 128M' % (qemu_img, image_path)
> -        process.run(cmd)
> +        subprocess.run(cmd, shell=True)
>   
>           replay_path = os.path.join(self.workdir, 'replay.bin')
>   
> -        # record the log
> +        # Record the log.
>           vm = self.run_vm(True, shift, args, replay_path, image_path, -1)
>           while self.vm_get_icount(vm) <= self.STEPS:
>               pass
> @@ -125,72 +171,91 @@ def reverse_debugging(self, shift=7, args=None):
>   
>           logger.info("recorded log with %s+ steps" % last_icount)
>   
> -        # replay and run debug commands
> +        # Replay and run debug commands.
>           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:
> +        logger.info('Connecting to gdbstub')
> +        r = self.gdb_connect('127.0.0.1', port)
> +        if 'ReverseStep+' not in r:
>               self.fail('Reverse step is not supported by QEMU')
> -        if b'ReverseContinue+' not in r:
> +        if 'ReverseContinue+' not in r:
>               self.fail('Reverse continue is not supported by QEMU')
>   
> -        logger.info('stepping forward')
> +        logger.info('Stepping forward')
>           steps = []
> -        # record first instruction addresses
> +        # Record first instruction addresses.
>           for _ in range(self.STEPS):
> -            pc = self.get_pc(g)
> -            logger.info('saving position %x' % pc)
> +            pc = self.get_pc()
> +            logger.info('Saving position %x' % pc)
>               steps.append(pc)
> -            self.gdb_step(g)
> +            self.gdb_step()
>   
> -        # visit the recorded instruction in reverse order
> -        logger.info('stepping backward')
> +        # Visit the recorded instruction in reverse order.
> +        logger.info('Stepping backward')
>           for addr in steps[::-1]:
> -            self.gdb_bstep(g)
> -            self.check_pc(g, addr)
> -            logger.info('found position %x' % addr)
> +            self.gdb_bstep()
> +            self.check_pc(addr)
> +            logger.info('Found position %x' % addr)
>   
> -        # visit the recorded instruction in forward order
> -        logger.info('stepping forward')
> +        # Visit the recorded instruction in forward order.
> +        logger.info('Stepping forward')
>           for addr in steps:
> -            self.check_pc(g, addr)
> -            self.gdb_step(g)
> -            logger.info('found position %x' % addr)
> +            self.check_pc(addr)
> +            self.gdb_step()
> +            logger.info('Found position %x' % addr)
>   
> -        # set breakpoints for the instructions just stepped over
> -        logger.info('setting breakpoints')
> +        # Set breakpoints for the instructions just stepped over.
> +        logger.info('Setting breakpoints')
>           for addr in steps:
>               # hardware breakpoint at addr with len=1
> -            g.cmd(b'Z1,%x,1' % addr, b'OK')
> +            gdb.execute(f"break *{hex(addr)}")
>   
> -        # this may hit a breakpoint if first instructions are executed
> -        # again
> -        logger.info('continuing execution')
> +        # This may hit a breakpoint if first instructions are executed again.
> +        logger.info('Continuing execution')
>           vm.qmp('replay-break', icount=last_icount - 1)
> -        # continue - will return after pausing
> +        # continue - will return after pausing.
>           # This could stop at the end and get a T02 return, or by
>           # re-executing one of the breakpoints and get a T05 return.
> -        g.cmd(b'c')
> +        gdb.execute("continue")
>           if self.vm_get_icount(vm) == last_icount - 1:
> -            logger.info('reached the end (icount %s)' % (last_icount - 1))
> +            logger.info('Reached the end (icount %s)' % (last_icount - 1))
>           else:
> -            logger.info('hit a breakpoint again at %x (icount %s)' %
> -                        (self.get_pc(g), self.vm_get_icount(vm)))
> +            logger.info('Hit a breakpoint again at %x (icount %s)' %
> +                        (self.get_pc(), self.vm_get_icount(vm)))
>   
> -        logger.info('running reverse continue to reach %x' % steps[-1])
> -        # reverse continue - will return after stopping at the breakpoint
> -        g.cmd(b'bc', b'T05thread:01;')
> +        logger.info('Running reverse continue to reach %x' % steps[-1])
> +        # reverse continue - will return after stopping at the breakpoint.
> +        gdb.execute("reverse-continue")
>   
> -        # assume that none of the first instructions is executed again
> -        # breaking the order of the breakpoints
> -        self.check_pc(g, steps[-1])
> -        logger.info('successfully reached %x' % steps[-1])
> +        # Assume that none of the first instructions is executed again
> +        # breaking the order of the breakpoints.
> +        # steps[-1] is the first saved $pc in reverse order.
> +        self.check_pc(steps[-1])
> +        logger.info('Successfully reached %x' % steps[-1])
>   
> -        logger.info('exiting gdb and qemu')
> +        logger.info('Exiting GDB and QEMU...')
> +        # Disconnect from the VM.
> +        gdb.execute("disconnect")
> +        # Guarantee VM is shutdown.
>           vm.shutdown()
> +        # Gently exit from GDB.
> +        gdb.execute('print "test succeeded"')
> +        gdb.execute("exit 0")
> +
> +    @staticmethod
> +    def main():
> +        try:
> +            LinuxKernelTest.main()
> +        except SystemExit:
> +            # If the test is marked with @skipFlakyTest, then it will be exited
> +            # via sys.exit() before we have the chance to exit from GDB gently.
> +            # Because recent versions of GDB will return a failure value if this
> +            # happens, we catch the SystemExit and exit from GDB gently with 77,
> +            # which meson interprets correctly as a skipped test.
> +            gdb.execute("exit 77")

Now that we're not passing the exitcode directly to Meson we can remove this overriden
main() method completely.



> +if __name__ == '__main__':
> +    if not _has_gdb:
> +        sys.exit("This script must be launched via tests/guest-debug/run-test.py!")

Since we don't rely on any run-test.py code anymore, how about:

  if __name__ == '__main__':
      if not _has_gdb:
-        sys.exit("This script must be launched via tests/guest-debug/run-test.py!")
+        sys.exit("This script must be run with GDB using the -x option.")


Cheers,
Gustavo

> +    ReverseDebugging.main()
> diff --git a/tests/functional/x86_64/test_reverse_debug.py b/tests/functional/x86_64/test_reverse_debug.py
> index d713e91e144..e823f0d4953 100755
> --- a/tests/functional/x86_64/test_reverse_debug.py
> +++ b/tests/functional/x86_64/test_reverse_debug.py
> @@ -2,7 +2,7 @@
>   #
>   # SPDX-License-Identifier: GPL-2.0-or-later
>   #
> -# Reverse debugging test
> +# Reverse debugging test for x86_64
>   #
>   # Copyright (c) 2020 ISP RAS
>   #
> @@ -12,25 +12,19 @@
>   # This work is licensed under the terms of the GNU GPL, version 2 or
>   # later.  See the COPYING file in the top-level directory.
>   
> -from qemu_test import skipIfMissingImports, skipFlakyTest
> -from reverse_debugging import ReverseDebugging
> +from qemu_test import QemuSystemTest, skipFlakyTest
>   
> +from reverse_debugging import reverse_debug
>   
> -@skipIfMissingImports('avocado.utils')
> -class ReverseDebugging_X86_64(ReverseDebugging):
>   
> -    REG_PC = 0x10
> -    REG_CS = 0x12
> -    def get_pc(self, g):
> -        return self.get_reg_le(g, self.REG_PC) \
> -            + self.get_reg_le(g, self.REG_CS) * 0x10
> +class ReverseDebugging_X86_64(QemuSystemTest):
>   
>       @skipFlakyTest("https://gitlab.com/qemu-project/qemu/-/issues/2922")
>       def test_x86_64_pc(self):
>           self.set_machine('pc')
> -        # start with BIOS only
> -        self.reverse_debugging()
> +        # Start with BIOS only
> +        reverse_debug(self)
>   
>   
>   if __name__ == '__main__':
> -    ReverseDebugging.main()
> +    QemuSystemTest.main()