[PATCH] tests/tcg: Stop using exit() in the gdbstub testcases

[PATCH] tests/tcg: Stop using exit() in the gdbstub testcases

[Ilya Leoshkevich]

GDB 15 does not like exit() anymore:

    (gdb) python exit(0)
    Python Exception <class 'SystemExit'>: 0
    Error occurred in Python: 0

Use the GDB's own exit command, like it's already done in a couple
places, everywhere.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 tests/guest-debug/test_gdbstub.py                 | 14 ++++++++++----
 tests/tcg/multiarch/gdbstub/interrupt.py          |  4 ++--
 tests/tcg/multiarch/gdbstub/prot-none.py          |  4 ++--
 tests/tcg/multiarch/gdbstub/test-proc-mappings.py |  4 ++--
 4 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/tests/guest-debug/test_gdbstub.py b/tests/guest-debug/test_gdbstub.py
index a715c0e3f5e..4f08089e6a9 100644
--- a/tests/guest-debug/test_gdbstub.py
+++ b/tests/guest-debug/test_gdbstub.py
@@ -10,10 +10,16 @@
 
 fail_count = 0
 
+
+def gdb_exit(status):
+    gdb.execute(f"exit {status}")
+
+
 class arg_parser(argparse.ArgumentParser):
     def exit(self, status=None, message=""):
         print("Wrong GDB script test argument! " + message)
-        gdb.execute("exit 1")
+        gdb_exit(1)
+
 
 def report(cond, msg):
     """Report success/fail of a test"""
@@ -38,11 +44,11 @@ def main(test, expected_arch=None):
                    "connected to {}".format(expected_arch))
     except (gdb.error, AttributeError):
         print("SKIP: not connected")
-        exit(0)
+        gdb_exit(0)
 
     if gdb.parse_and_eval("$pc") == 0:
         print("SKIP: PC not set")
-        exit(0)
+        gdb_exit(0)
 
     try:
         test()
@@ -62,4 +68,4 @@ def main(test, expected_arch=None):
         pass
 
     print("All tests complete: {} failures".format(fail_count))
-    gdb.execute(f"exit {fail_count}")
+    gdb_exit(fail_count)
diff --git a/tests/tcg/multiarch/gdbstub/interrupt.py b/tests/tcg/multiarch/gdbstub/interrupt.py
index 90a45b5140a..2d5654d1540 100644
--- a/tests/tcg/multiarch/gdbstub/interrupt.py
+++ b/tests/tcg/multiarch/gdbstub/interrupt.py
@@ -8,7 +8,7 @@
 #
 
 import gdb
-from test_gdbstub import main, report
+from test_gdbstub import gdb_exit, main, report
 
 
 def check_interrupt(thread):
@@ -49,7 +49,7 @@ def run_test():
     """
     if len(gdb.selected_inferior().threads()) == 1:
         print("SKIP: set to run on a single thread")
-        exit(0)
+        gdb_exit(0)
 
     gdb.execute("set scheduler-locking on")
     for thread in gdb.selected_inferior().threads():
diff --git a/tests/tcg/multiarch/gdbstub/prot-none.py b/tests/tcg/multiarch/gdbstub/prot-none.py
index 7e264589cb8..51082a30e40 100644
--- a/tests/tcg/multiarch/gdbstub/prot-none.py
+++ b/tests/tcg/multiarch/gdbstub/prot-none.py
@@ -5,7 +5,7 @@
 SPDX-License-Identifier: GPL-2.0-or-later
 """
 import ctypes
-from test_gdbstub import main, report
+from test_gdbstub import gdb_exit, main, report
 
 
 def probe_proc_self_mem():
@@ -22,7 +22,7 @@ def run_test():
     """Run through the tests one by one"""
     if not probe_proc_self_mem():
         print("SKIP: /proc/self/mem is not usable")
-        exit(0)
+        gdb_exit(0)
     gdb.Breakpoint("break_here")
     gdb.execute("continue")
     val = gdb.parse_and_eval("*(char[2] *)q").string()
diff --git a/tests/tcg/multiarch/gdbstub/test-proc-mappings.py b/tests/tcg/multiarch/gdbstub/test-proc-mappings.py
index 0f687f3284a..6eb6ebf7b17 100644
--- a/tests/tcg/multiarch/gdbstub/test-proc-mappings.py
+++ b/tests/tcg/multiarch/gdbstub/test-proc-mappings.py
@@ -3,7 +3,7 @@
 This runs as a sourced script (via -x, via run-test.py)."""
 from __future__ import print_function
 import gdb
-from test_gdbstub import main, report
+from test_gdbstub import gdb_exit, main, report
 
 
 def run_test():
@@ -12,7 +12,7 @@ def run_test():
         # m68k GDB supports only GDB_OSABI_SVR4, but GDB_OSABI_LINUX is
         # required for the info proc support (see set_gdbarch_info_proc()).
         print("SKIP: m68k GDB does not support GDB_OSABI_LINUX")
-        exit(0)
+        gdb_exit(0)
     mappings = gdb.execute("info proc mappings", False, True)
     report(isinstance(mappings, str), "Fetched the mappings from the inferior")
     # Broken with host page size > guest page size
-- 
2.47.0

Re: [PATCH] tests/tcg: Stop using exit() in the gdbstub testcases

[Peter Maydell]

On Mon, 21 Oct 2024 at 16:02, Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>
> GDB 15 does not like exit() anymore:
>
>     (gdb) python exit(0)
>     Python Exception <class 'SystemExit'>: 0
>     Error occurred in Python: 0
>
> Use the GDB's own exit command, like it's already done in a couple
> places, everywhere.

This is the same bug that commit 93a3048dcf4565 is
fixing, but it looks like we didn't catch everywhere.

thanks
-- PMM

Re: [PATCH] tests/tcg: Stop using exit() in the gdbstub testcases

[Gustavo Romero]

Hi,

On 10/21/24 12:08, Peter Maydell wrote:
> On Mon, 21 Oct 2024 at 16:02, Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>>
>> GDB 15 does not like exit() anymore:
>>
>>      (gdb) python exit(0)
>>      Python Exception <class 'SystemExit'>: 0
>>      Error occurred in Python: 0
>>
>> Use the GDB's own exit command, like it's already done in a couple
>> places, everywhere.
> 
> This is the same bug that commit 93a3048dcf4565 is
> fixing, but it looks like we didn't catch everywhere.

Yep.

So maybe now change in test_gdbstub.py this line:

     gdb.execute(f"exit {fail_count}")

to use the new gdb_exit()?


BTW, last news from this issue is that it seems to be fixed
in GDB 15.2 [0] (I haven't tried it). However, the fix using
gdb.exit(n) is still correct.

Acked-by: Gustavo Romero <gustavo.romero@linaro.org>


Cheers,
Gustavo

[0] https://sourceware.org/bugzilla/show_bug.cgi?id=31946

Re: [PATCH] tests/tcg: Stop using exit() in the gdbstub testcases

[Ilya Leoshkevich]

On Mon, 2024-10-21 at 16:08 -0300, Gustavo Romero wrote:
> Hi,
> 
> On 10/21/24 12:08, Peter Maydell wrote:
> > On Mon, 21 Oct 2024 at 16:02, Ilya Leoshkevich <iii@linux.ibm.com>
> > wrote:
> > > 
> > > GDB 15 does not like exit() anymore:
> > > 
> > >      (gdb) python exit(0)
> > >      Python Exception <class 'SystemExit'>: 0
> > >      Error occurred in Python: 0
> > > 
> > > Use the GDB's own exit command, like it's already done in a
> > > couple
> > > places, everywhere.
> > 
> > This is the same bug that commit 93a3048dcf4565 is
> > fixing, but it looks like we didn't catch everywhere.
> 
> Yep.
> 
> So maybe now change in test_gdbstub.py this line:
> 
>      gdb.execute(f"exit {fail_count}")
> 
> to use the new gdb_exit()?

Thanks for taking a look! I think I'm already doing this here?

@@ -62,4 +68,4 @@ def main(test, expected_arch=None):
         pass
 
     print("All tests complete: {} failures".format(fail_count))
-    gdb.execute(f"exit {fail_count}")
+    gdb_exit(fail_count)

> BTW, last news from this issue is that it seems to be fixed
> in GDB 15.2 [0] (I haven't tried it). However, the fix using
> gdb.exit(n) is still correct.

Interesting, I didn't realize this was a bug and not a design
change. Still, given that the buggy GDBs are out there, I'd prefer to
have this change.

> Acked-by: Gustavo Romero <gustavo.romero@linaro.org>
> 
> 
> Cheers,
> Gustavo
> 
> [0] https://sourceware.org/bugzilla/show_bug.cgi?id=31946

Re: [PATCH] tests/tcg: Stop using exit() in the gdbstub testcases

[Philippe Mathieu-Daudé]

On 21/10/24 16:31, Ilya Leoshkevich wrote:
> On Mon, 2024-10-21 at 16:08 -0300, Gustavo Romero wrote:
>> Hi,
>>
>> On 10/21/24 12:08, Peter Maydell wrote:
>>> On Mon, 21 Oct 2024 at 16:02, Ilya Leoshkevich <iii@linux.ibm.com>
>>> wrote:
>>>>
>>>> GDB 15 does not like exit() anymore:
>>>>
>>>>       (gdb) python exit(0)
>>>>       Python Exception <class 'SystemExit'>: 0
>>>>       Error occurred in Python: 0
>>>>
>>>> Use the GDB's own exit command, like it's already done in a
>>>> couple
>>>> places, everywhere.
>>>
>>> This is the same bug that commit 93a3048dcf4565 is
>>> fixing, but it looks like we didn't catch everywhere.
>>
>> Yep.
>>
>> So maybe now change in test_gdbstub.py this line:
>>
>>       gdb.execute(f"exit {fail_count}")
>>
>> to use the new gdb_exit()?
> 
> Thanks for taking a look! I think I'm already doing this here?
> 
> @@ -62,4 +68,4 @@ def main(test, expected_arch=None):
>           pass
>   
>       print("All tests complete: {} failures".format(fail_count))
> -    gdb.execute(f"exit {fail_count}")
> +    gdb_exit(fail_count)
> 
>> BTW, last news from this issue is that it seems to be fixed
>> in GDB 15.2 [0] (I haven't tried it). However, the fix using
>> gdb.exit(n) is still correct.
> 
> Interesting, I didn't realize this was a bug and not a design
> change. Still, given that the buggy GDBs are out there, I'd prefer to
> have this change.

Preferably mentioning commit 93a3048dcf4565,

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> 
>> Acked-by: Gustavo Romero <gustavo.romero@linaro.org>
>>
>>
>> Cheers,
>> Gustavo
>>
>> [0] https://sourceware.org/bugzilla/show_bug.cgi?id=31946
> 

Re: [PATCH] tests/tcg: Stop using exit() in the gdbstub testcases

[Gustavo Romero]

Hi Ilya,

On 10/21/24 16:31, Ilya Leoshkevich wrote:
> On Mon, 2024-10-21 at 16:08 -0300, Gustavo Romero wrote:
>> Hi,
>>
>> On 10/21/24 12:08, Peter Maydell wrote:
>>> On Mon, 21 Oct 2024 at 16:02, Ilya Leoshkevich <iii@linux.ibm.com>
>>> wrote:
>>>>
>>>> GDB 15 does not like exit() anymore:
>>>>
>>>>       (gdb) python exit(0)
>>>>       Python Exception <class 'SystemExit'>: 0
>>>>       Error occurred in Python: 0
>>>>
>>>> Use the GDB's own exit command, like it's already done in a
>>>> couple
>>>> places, everywhere.
>>>
>>> This is the same bug that commit 93a3048dcf4565 is
>>> fixing, but it looks like we didn't catch everywhere.
>>
>> Yep.
>>
>> So maybe now change in test_gdbstub.py this line:
>>
>>       gdb.execute(f"exit {fail_count}")
>>
>> to use the new gdb_exit()?
> 
> Thanks for taking a look! I think I'm already doing this here?
> 
> @@ -62,4 +68,4 @@ def main(test, expected_arch=None):
>           pass
>   
>       print("All tests complete: {} failures".format(fail_count))
> -    gdb.execute(f"exit {fail_count}")
> +    gdb_exit(fail_count)
> 
>> BTW, last news from this issue is that it seems to be fixed
>> in GDB 15.2 [0] (I haven't tried it). However, the fix using
>> gdb.exit(n) is still correct.
> 
> Interesting, I didn't realize this was a bug and not a design
> change. Still, given that the buggy GDBs are out there, I'd prefer to
> have this change.

Yeah, exactly, for my fix I understood it was a design change at that
time too, then there is this fix on GDB 15.2 :) Right, I also prefer
having these fixes in place for QEMU. Thanks.


Cheers,
Gustavo