qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/3] linux-user: Allow gdbstub to ignore page protection
@ 2024-01-16  0:31 Ilya Leoshkevich
  2024-01-16  0:31 ` [PATCH v3 1/3] " Ilya Leoshkevich
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Ilya Leoshkevich @ 2024-01-16  0:31 UTC (permalink / raw)
  To: Richard Henderson, Peter Maydell, Alex Bennée,
	David Hildenbrand
  Cc: Paolo Bonzini, Philippe Mathieu-Daudé, qemu-devel, qemu-arm,
	qemu-s390x, Ilya Leoshkevich

v2: https://lists.gnu.org/archive/html/qemu-devel/2024-01/msg01592.html
v2 -> v3: Add Richard's R-b on [1/3].
          Fix printing the architecture name and the number of failures
          in test_gdbstub.py.
          Patches that need review: [2/3] and [3/3].

v1: https://lists.gnu.org/archive/html/qemu-devel/2024-01/msg01314.html
v1 -> v2: Use /proc/self/mem as a fallback. Handle TB invalidation
          (Richard).
          Test cross-page accesses.

RFC: https://lists.gnu.org/archive/html/qemu-devel/2023-12/msg02044.html
RFC -> v1: Use /proc/self/mem and accept that this will not work
           without /proc.
           Factor out a couple functions for gdbstub testing.
           Add a test.

Hi,

I've noticed that gdbstub behaves differently from gdbserver in that it
doesn't allow reading non-readable pages. This series improves the
situation by using the same mechanism as gdbserver: /proc/self/mem.

Best regards,
Ilya

Ilya Leoshkevich (3):
  linux-user: Allow gdbstub to ignore page protection
  tests/tcg: Factor out gdbstub test functions
  tests/tcg: Add the PROT_NONE gdbstub test

 cpu-target.c                                  | 76 +++++++++++++++----
 tests/guest-debug/run-test.py                 |  7 +-
 tests/guest-debug/test_gdbstub.py             | 58 ++++++++++++++
 tests/tcg/aarch64/gdbstub/test-sve-ioctl.py   | 34 +--------
 tests/tcg/aarch64/gdbstub/test-sve.py         | 33 +-------
 tests/tcg/multiarch/Makefile.target           |  9 ++-
 tests/tcg/multiarch/gdbstub/interrupt.py      | 47 ++----------
 tests/tcg/multiarch/gdbstub/memory.py         | 41 +---------
 tests/tcg/multiarch/gdbstub/prot-none.py      | 22 ++++++
 tests/tcg/multiarch/gdbstub/registers.py      | 41 ++--------
 tests/tcg/multiarch/gdbstub/sha1.py           | 40 ++--------
 .../multiarch/gdbstub/test-proc-mappings.py   | 39 +---------
 .../multiarch/gdbstub/test-qxfer-auxv-read.py | 37 +--------
 .../gdbstub/test-thread-breakpoint.py         | 37 +--------
 tests/tcg/multiarch/prot-none.c               | 40 ++++++++++
 tests/tcg/s390x/gdbstub/test-signals-s390x.py | 42 +---------
 tests/tcg/s390x/gdbstub/test-svc.py           | 39 +---------
 17 files changed, 229 insertions(+), 413 deletions(-)
 create mode 100644 tests/guest-debug/test_gdbstub.py
 create mode 100644 tests/tcg/multiarch/gdbstub/prot-none.py
 create mode 100644 tests/tcg/multiarch/prot-none.c

-- 
2.43.0



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v3 1/3] linux-user: Allow gdbstub to ignore page protection
  2024-01-16  0:31 [PATCH v3 0/3] linux-user: Allow gdbstub to ignore page protection Ilya Leoshkevich
@ 2024-01-16  0:31 ` Ilya Leoshkevich
  2024-01-16  0:31 ` [PATCH v3 2/3] tests/tcg: Factor out gdbstub test functions Ilya Leoshkevich
  2024-01-16  0:31 ` [PATCH v3 3/3] tests/tcg: Add the PROT_NONE gdbstub test Ilya Leoshkevich
  2 siblings, 0 replies; 11+ messages in thread
From: Ilya Leoshkevich @ 2024-01-16  0:31 UTC (permalink / raw)
  To: Richard Henderson, Peter Maydell, Alex Bennée,
	David Hildenbrand
  Cc: Paolo Bonzini, Philippe Mathieu-Daudé, qemu-devel, qemu-arm,
	qemu-s390x, Ilya Leoshkevich

gdbserver ignores page protection by virtue of using /proc/$pid/mem.
Teach qemu gdbstub to do this too. This will not work if /proc is not
mounted; accept this limitation.

One alternative is to temporarily grant the missing PROT_* bit, but
this is inherently racy. Another alternative is self-debugging with
ptrace(POKE), which will break if QEMU itself is being debugged - a
much more severe limitation.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 cpu-target.c | 76 +++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 61 insertions(+), 15 deletions(-)

diff --git a/cpu-target.c b/cpu-target.c
index 5eecd7ea2d7..723f6af5fba 100644
--- a/cpu-target.c
+++ b/cpu-target.c
@@ -406,6 +406,9 @@ int cpu_memory_rw_debug(CPUState *cpu, vaddr addr,
     vaddr l, page;
     void * p;
     uint8_t *buf = ptr;
+    ssize_t written;
+    int ret = -1;
+    int fd = -1;
 
     while (len > 0) {
         page = addr & TARGET_PAGE_MASK;
@@ -413,30 +416,73 @@ int cpu_memory_rw_debug(CPUState *cpu, vaddr addr,
         if (l > len)
             l = len;
         flags = page_get_flags(page);
-        if (!(flags & PAGE_VALID))
-            return -1;
+        if (!(flags & PAGE_VALID)) {
+            goto out_close;
+        }
         if (is_write) {
-            if (!(flags & PAGE_WRITE))
-                return -1;
-            /* XXX: this code should not depend on lock_user */
-            if (!(p = lock_user(VERIFY_WRITE, addr, l, 0)))
-                return -1;
-            memcpy(p, buf, l);
-            unlock_user(p, addr, l);
-        } else {
-            if (!(flags & PAGE_READ))
-                return -1;
+            if (flags & PAGE_WRITE) {
+                /* XXX: this code should not depend on lock_user */
+                p = lock_user(VERIFY_WRITE, addr, l, 0);
+                if (!p) {
+                    goto out_close;
+                }
+                memcpy(p, buf, l);
+                unlock_user(p, addr, l);
+            } else {
+                /* Bypass the host page protection using ptrace. */
+                if (fd == -1) {
+                    fd = open("/proc/self/mem", O_WRONLY);
+                    if (fd == -1) {
+                        goto out;
+                    }
+                }
+                /*
+                 * If there is a TranslationBlock and we weren't bypassing the
+                 * host page protection, the memcpy() above would SEGV,
+                 * ultimately leading to page_unprotect(). So invalidate the
+                 * translations manually. Both invalidation and pwrite() must
+                 * be under mmap_lock() in order to prevent the creation of
+                 * another TranslationBlock in between.
+                 */
+                mmap_lock();
+                tb_invalidate_phys_range(addr, addr + l - 1);
+                written = pwrite(fd, buf, l, (off_t)g2h_untagged(addr));
+                mmap_unlock();
+                if (written != l) {
+                    goto out_close;
+                }
+            }
+        } else if (flags & PAGE_READ) {
             /* XXX: this code should not depend on lock_user */
-            if (!(p = lock_user(VERIFY_READ, addr, l, 1)))
-                return -1;
+            p = lock_user(VERIFY_READ, addr, l, 1);
+            if (!p) {
+                goto out_close;
+            }
             memcpy(buf, p, l);
             unlock_user(p, addr, 0);
+        } else {
+            /* Bypass the host page protection using ptrace. */
+            if (fd == -1) {
+                fd = open("/proc/self/mem", O_RDONLY);
+                if (fd == -1) {
+                    goto out;
+                }
+            }
+            if (pread(fd, buf, l, (off_t)g2h_untagged(addr)) != l) {
+                goto out_close;
+            }
         }
         len -= l;
         buf += l;
         addr += l;
     }
-    return 0;
+    ret = 0;
+out_close:
+    if (fd != -1) {
+        close(fd);
+    }
+out:
+    return ret;
 }
 #endif
 
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 2/3] tests/tcg: Factor out gdbstub test functions
  2024-01-16  0:31 [PATCH v3 0/3] linux-user: Allow gdbstub to ignore page protection Ilya Leoshkevich
  2024-01-16  0:31 ` [PATCH v3 1/3] " Ilya Leoshkevich
@ 2024-01-16  0:31 ` Ilya Leoshkevich
  2024-01-22 16:00   ` Alex Bennée
  2024-01-16  0:31 ` [PATCH v3 3/3] tests/tcg: Add the PROT_NONE gdbstub test Ilya Leoshkevich
  2 siblings, 1 reply; 11+ messages in thread
From: Ilya Leoshkevich @ 2024-01-16  0:31 UTC (permalink / raw)
  To: Richard Henderson, Peter Maydell, Alex Bennée,
	David Hildenbrand
  Cc: Paolo Bonzini, Philippe Mathieu-Daudé, qemu-devel, qemu-arm,
	qemu-s390x, Ilya Leoshkevich

Both the report() function as well as the initial gdbstub test sequence
are copy-pasted into ~10 files with slight modifications. This
indicates that they are indeed generic, so factor them out. While
at it, add a few newlines to make the formatting closer to PEP-8.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 tests/guest-debug/run-test.py                 |  7 ++-
 tests/guest-debug/test_gdbstub.py             | 58 +++++++++++++++++++
 tests/tcg/aarch64/gdbstub/test-sve-ioctl.py   | 34 +----------
 tests/tcg/aarch64/gdbstub/test-sve.py         | 33 +----------
 tests/tcg/multiarch/gdbstub/interrupt.py      | 47 ++-------------
 tests/tcg/multiarch/gdbstub/memory.py         | 41 +------------
 tests/tcg/multiarch/gdbstub/registers.py      | 41 ++-----------
 tests/tcg/multiarch/gdbstub/sha1.py           | 40 ++-----------
 .../multiarch/gdbstub/test-proc-mappings.py   | 39 +------------
 .../multiarch/gdbstub/test-qxfer-auxv-read.py | 37 +-----------
 .../gdbstub/test-thread-breakpoint.py         | 37 +-----------
 tests/tcg/s390x/gdbstub/test-signals-s390x.py | 42 +-------------
 tests/tcg/s390x/gdbstub/test-svc.py           | 39 +------------
 13 files changed, 98 insertions(+), 397 deletions(-)
 create mode 100644 tests/guest-debug/test_gdbstub.py

diff --git a/tests/guest-debug/run-test.py b/tests/guest-debug/run-test.py
index b13b27d4b19..368ff8a8903 100755
--- a/tests/guest-debug/run-test.py
+++ b/tests/guest-debug/run-test.py
@@ -97,7 +97,12 @@ def log(output, msg):
     sleep(1)
     log(output, "GDB CMD: %s" % (gdb_cmd))
 
-    result = subprocess.call(gdb_cmd, shell=True, stdout=output, stderr=stderr)
+    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)
+    result = subprocess.call(gdb_cmd, shell=True, stdout=output, stderr=stderr,
+                             env=gdb_env)
 
     # A result of greater than 128 indicates a fatal signal (likely a
     # crash due to gdb internal failure). That's a problem for GDB and
diff --git a/tests/guest-debug/test_gdbstub.py b/tests/guest-debug/test_gdbstub.py
new file mode 100644
index 00000000000..a71cdaa915a
--- /dev/null
+++ b/tests/guest-debug/test_gdbstub.py
@@ -0,0 +1,58 @@
+"""Helper functions for gdbstub testing
+
+"""
+from __future__ import print_function
+import gdb
+import sys
+import traceback
+
+fail_count = 0
+
+
+def report(cond, msg):
+    """Report success/fail of a test"""
+    if cond:
+        print("PASS: {}".format(msg))
+    else:
+        print("FAIL: {}".format(msg))
+        global fail_count
+        fail_count += 1
+
+
+def main(test, expected_arch=None):
+    """Run a test function
+
+    This runs as the script it sourced (via -x, via run-test.py)."""
+    try:
+        inferior = gdb.selected_inferior()
+        arch = inferior.architecture()
+        print("ATTACHED: {}".format(arch.name()))
+        if expected_arch is not None:
+            report(arch.name() == expected_arch,
+                   "connected to {}".format(expected_arch))
+    except (gdb.error, AttributeError):
+        print("SKIP: not connected")
+        exit(0)
+
+    if gdb.parse_and_eval("$pc") == 0:
+        print("SKIP: PC not set")
+        exit(0)
+
+    try:
+        test()
+    except:
+        print("GDB Exception:")
+        traceback.print_exc(file=sys.stdout)
+        global fail_count
+        fail_count += 1
+        import code
+        code.InteractiveConsole(locals=globals()).interact()
+        raise
+
+    try:
+        gdb.execute("kill")
+    except gdb.error:
+        pass
+
+    print("All tests complete: {} failures".format(fail_count))
+    exit(fail_count)
diff --git a/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py b/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
index ee8d467e59d..a78a3a2514d 100644
--- a/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
+++ b/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
@@ -8,19 +8,10 @@
 #
 
 import gdb
-import sys
+from test_gdbstub import main, report
 
 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
 
 class TestBreakpoint(gdb.Breakpoint):
     def __init__(self, sym_name="__sve_ld_done"):
@@ -64,26 +55,5 @@ def run_test():
 
     gdb.execute("c")
 
-#
-# This runs as the script it sourced (via -x, via run-test.py)
-#
-try:
-    inferior = gdb.selected_inferior()
-    arch = inferior.architecture()
-    report(arch.name() == "aarch64", "connected to aarch64")
-except (gdb.error, AttributeError):
-    print("SKIPPING (not connected)", file=sys.stderr)
-    exit(0)
-
-try:
-    # Run the actual tests
-    run_test()
-except:
-    print ("GDB Exception: %s" % (sys.exc_info()[0]))
-    failcount += 1
-    import code
-    code.InteractiveConsole(locals=globals()).interact()
-    raise
 
-print("All tests complete: %d failures" % failcount)
-exit(failcount)
+main(run_test, expected_arch="aarch64")
diff --git a/tests/tcg/aarch64/gdbstub/test-sve.py b/tests/tcg/aarch64/gdbstub/test-sve.py
index afd8ece98dd..84cdcd4a32e 100644
--- a/tests/tcg/aarch64/gdbstub/test-sve.py
+++ b/tests/tcg/aarch64/gdbstub/test-sve.py
@@ -6,20 +6,10 @@
 #
 
 import gdb
-import sys
+from test_gdbstub import main, report
 
 MAGIC = 0xDEADBEEF
 
-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
 
 def run_test():
     "Run through the tests one by one"
@@ -54,24 +44,5 @@ def run_test():
             report(str(v.type) == "uint64_t", "size of %s" % (reg))
             report(int(v) == MAGIC, "%s is 0x%x" % (reg, MAGIC))
 
-#
-# This runs as the script it sourced (via -x, via run-test.py)
-#
-try:
-    inferior = gdb.selected_inferior()
-    arch = inferior.architecture()
-    report(arch.name() == "aarch64", "connected to aarch64")
-except (gdb.error, AttributeError):
-    print("SKIPPING (not connected)", file=sys.stderr)
-    exit(0)
-
-try:
-    # Run the actual tests
-    run_test()
-except:
-    print ("GDB Exception: %s" % (sys.exc_info()[0]))
-    failcount += 1
-
-print("All tests complete: %d failures" % failcount)
 
-exit(failcount)
+main(run_test, expected_arch="aarch64")
diff --git a/tests/tcg/multiarch/gdbstub/interrupt.py b/tests/tcg/multiarch/gdbstub/interrupt.py
index c016e7afbbf..90a45b5140a 100644
--- a/tests/tcg/multiarch/gdbstub/interrupt.py
+++ b/tests/tcg/multiarch/gdbstub/interrupt.py
@@ -8,19 +8,7 @@
 #
 
 import gdb
-import sys
-
-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
+from test_gdbstub import main, report
 
 
 def check_interrupt(thread):
@@ -59,6 +47,9 @@ def run_test():
     Test if interrupting the code always lands us on the same thread when
     running with scheduler-lock enabled.
     """
+    if len(gdb.selected_inferior().threads()) == 1:
+        print("SKIP: set to run on a single thread")
+        exit(0)
 
     gdb.execute("set scheduler-locking on")
     for thread in gdb.selected_inferior().threads():
@@ -66,32 +57,4 @@ def run_test():
                "thread %d resumes correctly on interrupt" % thread.num)
 
 
-#
-# 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)
-if len(gdb.selected_inferior().threads()) == 1:
-    print("SKIP: set to run on a single thread")
-    exit(0)
-
-try:
-    # Run the actual tests
-    run_test()
-except (gdb.error):
-    print("GDB Exception: %s" % (sys.exc_info()[0]))
-    failcount += 1
-    pass
-
-# Finally kill the inferior and exit gdb with a count of failures
-gdb.execute("kill")
-exit(failcount)
+main(run_test)
diff --git a/tests/tcg/multiarch/gdbstub/memory.py b/tests/tcg/multiarch/gdbstub/memory.py
index fb1d06b7bb7..532b92e7fb3 100644
--- a/tests/tcg/multiarch/gdbstub/memory.py
+++ b/tests/tcg/multiarch/gdbstub/memory.py
@@ -9,18 +9,7 @@
 
 import gdb
 import sys
-
-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
+from test_gdbstub import main, report
 
 
 def check_step():
@@ -99,29 +88,5 @@ def run_test():
 
     report(cbp.hit_count == 0, "didn't reach backstop")
 
-#
-# 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 the actual tests
-    run_test()
-except (gdb.error):
-    print("GDB Exception: %s" % (sys.exc_info()[0]))
-    failcount += 1
-    pass
-
-# Finally kill the inferior and exit gdb with a count of failures
-gdb.execute("kill")
-exit(failcount)
+
+main(run_test)
diff --git a/tests/tcg/multiarch/gdbstub/registers.py b/tests/tcg/multiarch/gdbstub/registers.py
index 688c0611072..b3d13cb077f 100644
--- a/tests/tcg/multiarch/gdbstub/registers.py
+++ b/tests/tcg/multiarch/gdbstub/registers.py
@@ -7,20 +7,11 @@
 # SPDX-License-Identifier: GPL-2.0-or-later
 
 import gdb
-import sys
 import xml.etree.ElementTree as ET
+from test_gdbstub import main, report
 
-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
+initial_vlen = 0
 
 
 def fetch_xml_regmap():
@@ -75,6 +66,7 @@ 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
@@ -84,6 +76,7 @@ def get_register_by_regnum(reg_map, regnum):
             return entry
     return None
 
+
 def crosscheck_remote_xml(reg_map):
     """
     Cross-check the list of remote-registers with the XML info.
@@ -144,6 +137,7 @@ def crosscheck_remote_xml(reg_map):
         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
@@ -214,27 +208,4 @@ def run_test():
         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)
+main(run_test)
diff --git a/tests/tcg/multiarch/gdbstub/sha1.py b/tests/tcg/multiarch/gdbstub/sha1.py
index 416728415f9..1ce711a402c 100644
--- a/tests/tcg/multiarch/gdbstub/sha1.py
+++ b/tests/tcg/multiarch/gdbstub/sha1.py
@@ -7,19 +7,11 @@
 #
 
 import gdb
-import sys
+from test_gdbstub import main, report
+
 
 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
 
 def check_break(sym_name):
     "Setup breakpoint, continue and check we stopped."
@@ -35,6 +27,7 @@ def check_break(sym_name):
 
     bp.delete()
 
+
 def run_test():
     "Run through the tests one by one"
 
@@ -57,28 +50,5 @@ def run_test():
     # finally check we don't barf inspecting registers
     gdb.execute("info registers")
 
-#
-# 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 the actual tests
-    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)
+
+main(run_test)
diff --git a/tests/tcg/multiarch/gdbstub/test-proc-mappings.py b/tests/tcg/multiarch/gdbstub/test-proc-mappings.py
index 04ec61d2197..564613fabf0 100644
--- a/tests/tcg/multiarch/gdbstub/test-proc-mappings.py
+++ b/tests/tcg/multiarch/gdbstub/test-proc-mappings.py
@@ -3,20 +3,7 @@
 This runs as a sourced script (via -x, via run-test.py)."""
 from __future__ import print_function
 import gdb
-import sys
-
-
-n_failures = 0
-
-
-def report(cond, msg):
-    """Report success/fail of a test"""
-    if cond:
-        print("PASS: {}".format(msg))
-    else:
-        print("FAIL: {}".format(msg))
-        global n_failures
-        n_failures += 1
+from test_gdbstub import main, report
 
 
 def run_test():
@@ -37,26 +24,4 @@ def run_test():
     # report("/sha1" in mappings, "Found the test binary name in the mappings")
 
 
-def main():
-    """Prepare the environment and run through the tests"""
-    try:
-        inferior = gdb.selected_inferior()
-        print("ATTACHED: {}".format(inferior.architecture().name()))
-    except (gdb.error, AttributeError):
-        print("SKIPPING (not connected)")
-        exit(0)
-
-    if gdb.parse_and_eval('$pc') == 0:
-        print("SKIP: PC not set")
-        exit(0)
-
-    try:
-        # Run the actual tests
-        run_test()
-    except gdb.error:
-        report(False, "GDB Exception: {}".format(sys.exc_info()[0]))
-    print("All tests complete: %d failures" % n_failures)
-    exit(n_failures)
-
-
-main()
+main(run_test)
diff --git a/tests/tcg/multiarch/gdbstub/test-qxfer-auxv-read.py b/tests/tcg/multiarch/gdbstub/test-qxfer-auxv-read.py
index 926fa962b77..00c26ab4a95 100644
--- a/tests/tcg/multiarch/gdbstub/test-qxfer-auxv-read.py
+++ b/tests/tcg/multiarch/gdbstub/test-qxfer-auxv-read.py
@@ -6,18 +6,8 @@
 #
 
 import gdb
-import sys
+from test_gdbstub import main, report
 
-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
 
 def run_test():
     "Run through the tests one by one"
@@ -26,28 +16,5 @@ def run_test():
     report(isinstance(auxv, str), "Fetched auxv from inferior")
     report(auxv.find("sha1"), "Found test binary name in auxv")
 
-#
-# 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 the actual tests
-    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)
+main(run_test)
diff --git a/tests/tcg/multiarch/gdbstub/test-thread-breakpoint.py b/tests/tcg/multiarch/gdbstub/test-thread-breakpoint.py
index e57d2a8db8b..4d6b6b9fbe7 100644
--- a/tests/tcg/multiarch/gdbstub/test-thread-breakpoint.py
+++ b/tests/tcg/multiarch/gdbstub/test-thread-breakpoint.py
@@ -6,18 +6,8 @@
 #
 
 import gdb
-import sys
+from test_gdbstub import main, report
 
-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
 
 def run_test():
     "Run through the tests one by one"
@@ -29,28 +19,5 @@ def run_test():
     frame = gdb.selected_frame()
     report(str(frame.function()) == "thread1_func", "break @ %s"%frame)
 
-#
-# 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 the actual tests
-    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)
+main(run_test)
diff --git a/tests/tcg/s390x/gdbstub/test-signals-s390x.py b/tests/tcg/s390x/gdbstub/test-signals-s390x.py
index ca2bbc0b03e..b6b7b39fc46 100644
--- a/tests/tcg/s390x/gdbstub/test-signals-s390x.py
+++ b/tests/tcg/s390x/gdbstub/test-signals-s390x.py
@@ -7,19 +7,7 @@
 #
 
 import gdb
-import sys
-
-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
+from test_gdbstub import main, report
 
 
 def run_test():
@@ -42,31 +30,7 @@ def run_test():
     gdb.Breakpoint("_exit")
     gdb.execute("c")
     status = int(gdb.parse_and_eval("$r2"))
-    report(status == 0, "status == 0");
-
-
-#
-# 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)
+    report(status == 0, "status == 0")
 
-try:
-    # Run the actual tests
-    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)
+main(run_test)
diff --git a/tests/tcg/s390x/gdbstub/test-svc.py b/tests/tcg/s390x/gdbstub/test-svc.py
index 804705fede9..17210b4e020 100644
--- a/tests/tcg/s390x/gdbstub/test-svc.py
+++ b/tests/tcg/s390x/gdbstub/test-svc.py
@@ -3,20 +3,7 @@
 This runs as a sourced script (via -x, via run-test.py)."""
 from __future__ import print_function
 import gdb
-import sys
-
-
-n_failures = 0
-
-
-def report(cond, msg):
-    """Report success/fail of a test"""
-    if cond:
-        print("PASS: {}".format(msg))
-    else:
-        print("FAIL: {}".format(msg))
-        global n_failures
-        n_failures += 1
+from test_gdbstub import main, report
 
 
 def run_test():
@@ -35,26 +22,4 @@ def run_test():
     gdb.execute("si")
 
 
-def main():
-    """Prepare the environment and run through the tests"""
-    try:
-        inferior = gdb.selected_inferior()
-        print("ATTACHED: {}".format(inferior.architecture().name()))
-    except (gdb.error, AttributeError):
-        print("SKIPPING (not connected)")
-        exit(0)
-
-    if gdb.parse_and_eval('$pc') == 0:
-        print("SKIP: PC not set")
-        exit(0)
-
-    try:
-        # Run the actual tests
-        run_test()
-    except gdb.error:
-        report(False, "GDB Exception: {}".format(sys.exc_info()[0]))
-    print("All tests complete: %d failures" % n_failures)
-    exit(n_failures)
-
-
-main()
+main(run_test)
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 3/3] tests/tcg: Add the PROT_NONE gdbstub test
  2024-01-16  0:31 [PATCH v3 0/3] linux-user: Allow gdbstub to ignore page protection Ilya Leoshkevich
  2024-01-16  0:31 ` [PATCH v3 1/3] " Ilya Leoshkevich
  2024-01-16  0:31 ` [PATCH v3 2/3] tests/tcg: Factor out gdbstub test functions Ilya Leoshkevich
@ 2024-01-16  0:31 ` Ilya Leoshkevich
  2024-01-22 15:43   ` Alex Bennée
  2024-01-22 15:54   ` Alex Bennée
  2 siblings, 2 replies; 11+ messages in thread
From: Ilya Leoshkevich @ 2024-01-16  0:31 UTC (permalink / raw)
  To: Richard Henderson, Peter Maydell, Alex Bennée,
	David Hildenbrand
  Cc: Paolo Bonzini, Philippe Mathieu-Daudé, qemu-devel, qemu-arm,
	qemu-s390x, Ilya Leoshkevich

Make sure that qemu gdbstub, like gdbserver, allows reading from and
writing to PROT_NONE pages.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 tests/tcg/multiarch/Makefile.target      |  9 +++++-
 tests/tcg/multiarch/gdbstub/prot-none.py | 22 +++++++++++++
 tests/tcg/multiarch/prot-none.c          | 40 ++++++++++++++++++++++++
 3 files changed, 70 insertions(+), 1 deletion(-)
 create mode 100644 tests/tcg/multiarch/gdbstub/prot-none.py
 create mode 100644 tests/tcg/multiarch/prot-none.c

diff --git a/tests/tcg/multiarch/Makefile.target b/tests/tcg/multiarch/Makefile.target
index d31ba8d6ae4..315a2e13588 100644
--- a/tests/tcg/multiarch/Makefile.target
+++ b/tests/tcg/multiarch/Makefile.target
@@ -101,13 +101,20 @@ run-gdbstub-registers: sha512
 		--bin $< --test $(MULTIARCH_SRC)/gdbstub/registers.py, \
 	checking register enumeration)
 
+run-gdbstub-prot-none: prot-none
+	$(call run-test, $@, env PROT_NONE_PY=1 $(GDB_SCRIPT) \
+		--gdb $(GDB) \
+		--qemu $(QEMU) --qargs "$(QEMU_OPTS)" \
+		--bin $< --test $(MULTIARCH_SRC)/gdbstub/prot-none.py, \
+	accessing PROT_NONE memory)
+
 else
 run-gdbstub-%:
 	$(call skip-test, "gdbstub test $*", "need working gdb with $(patsubst -%,,$(TARGET_NAME)) support")
 endif
 EXTRA_RUNS += run-gdbstub-sha1 run-gdbstub-qxfer-auxv-read \
 	      run-gdbstub-proc-mappings run-gdbstub-thread-breakpoint \
-	      run-gdbstub-registers
+	      run-gdbstub-registers run-gdbstub-prot-none
 
 # ARM Compatible Semi Hosting Tests
 #
diff --git a/tests/tcg/multiarch/gdbstub/prot-none.py b/tests/tcg/multiarch/gdbstub/prot-none.py
new file mode 100644
index 00000000000..f1f1dd82cbe
--- /dev/null
+++ b/tests/tcg/multiarch/gdbstub/prot-none.py
@@ -0,0 +1,22 @@
+"""Test that GDB can access PROT_NONE pages.
+
+This runs as a sourced script (via -x, via run-test.py).
+
+SPDX-License-Identifier: GPL-2.0-or-later
+"""
+from test_gdbstub import main, report
+
+
+def run_test():
+    """Run through the tests one by one"""
+    gdb.Breakpoint("break_here")
+    gdb.execute("continue")
+    val = gdb.parse_and_eval("*(char[2] *)q").string()
+    report(val == "42", "{} == 42".format(val))
+    gdb.execute("set *(char[3] *)q = \"24\"")
+    gdb.execute("continue")
+    exitcode = int(gdb.parse_and_eval("$_exitcode"))
+    report(exitcode == 0, "{} == 0".format(exitcode))
+
+
+main(run_test)
diff --git a/tests/tcg/multiarch/prot-none.c b/tests/tcg/multiarch/prot-none.c
new file mode 100644
index 00000000000..dc56aadb3c5
--- /dev/null
+++ b/tests/tcg/multiarch/prot-none.c
@@ -0,0 +1,40 @@
+/*
+ * Test that GDB can access PROT_NONE pages.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include <assert.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/mman.h>
+#include <unistd.h>
+
+void break_here(void *q)
+{
+}
+
+int main(void)
+{
+    long pagesize = sysconf(_SC_PAGESIZE);
+    void *p, *q;
+    int err;
+
+    p = mmap(NULL, pagesize * 2, PROT_READ | PROT_WRITE,
+             MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+    assert(p != MAP_FAILED);
+    q = p + pagesize - 1;
+    strcpy(q, "42");
+
+    err = mprotect(p, pagesize * 2, PROT_NONE);
+    assert(err == 0);
+
+    break_here(q);
+
+    err = mprotect(p, pagesize * 2, PROT_READ);
+    assert(err == 0);
+    if (getenv("PROT_NONE_PY")) {
+        assert(strcmp(q, "24") == 0);
+    }
+
+    return EXIT_SUCCESS;
+}
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 3/3] tests/tcg: Add the PROT_NONE gdbstub test
  2024-01-16  0:31 ` [PATCH v3 3/3] tests/tcg: Add the PROT_NONE gdbstub test Ilya Leoshkevich
@ 2024-01-22 15:43   ` Alex Bennée
  2024-01-22 15:54   ` Alex Bennée
  1 sibling, 0 replies; 11+ messages in thread
From: Alex Bennée @ 2024-01-22 15:43 UTC (permalink / raw)
  To: Ilya Leoshkevich
  Cc: Richard Henderson, Peter Maydell, David Hildenbrand,
	Paolo Bonzini, Philippe Mathieu-Daudé, qemu-devel, qemu-arm,
	qemu-s390x

Ilya Leoshkevich <iii@linux.ibm.com> writes:

> Make sure that qemu gdbstub, like gdbserver, allows reading from and
> writing to PROT_NONE pages.
>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>

Hmm I'm seeing the test hang and drop to the interactive python shell:

    TEST    basic gdbstub support on aarch64
  Failed to read a valid object file image from memory.
  qemu-aarch64: QEMU: Terminated via GDBstub
    TEST    basic gdbstub qXfer:auxv:read support on aarch64
  Failed to read a valid object file image from memory.
  qemu-aarch64: QEMU: Terminated via GDBstub
    TEST    proc mappings support on aarch64
  Failed to read a valid object file image from memory.
  qemu-aarch64: QEMU: Terminated via GDBstub
    TEST    hitting a breakpoint on non-main thread on aarch64
  Failed to read a valid object file image from memory.
  qemu-aarch64: QEMU: Terminated via GDBstub
    TEST    checking register enumeration on aarch64
  Failed to read a valid object file image from memory.
  qemu-aarch64: QEMU: Terminated via GDBstub
    TEST    accessing PROT_NONE memory on aarch64
  Failed to read a valid object file image from memory.
  Python 3.11.2 (main, Mar 13 2023, 12:18:29) [GCC 12.2.0] on linux
  Type "help", "copyright", "credits" or "license" for more information.
  (InteractiveConsole)

> ---
>  tests/tcg/multiarch/Makefile.target      |  9 +++++-
>  tests/tcg/multiarch/gdbstub/prot-none.py | 22 +++++++++++++
>  tests/tcg/multiarch/prot-none.c          | 40 ++++++++++++++++++++++++
>  3 files changed, 70 insertions(+), 1 deletion(-)
>  create mode 100644 tests/tcg/multiarch/gdbstub/prot-none.py
>  create mode 100644 tests/tcg/multiarch/prot-none.c
>
> diff --git a/tests/tcg/multiarch/Makefile.target b/tests/tcg/multiarch/Makefile.target
> index d31ba8d6ae4..315a2e13588 100644
> --- a/tests/tcg/multiarch/Makefile.target
> +++ b/tests/tcg/multiarch/Makefile.target
> @@ -101,13 +101,20 @@ run-gdbstub-registers: sha512
>  		--bin $< --test $(MULTIARCH_SRC)/gdbstub/registers.py, \
>  	checking register enumeration)
>  
> +run-gdbstub-prot-none: prot-none
> +	$(call run-test, $@, env PROT_NONE_PY=1 $(GDB_SCRIPT) \
> +		--gdb $(GDB) \
> +		--qemu $(QEMU) --qargs "$(QEMU_OPTS)" \
> +		--bin $< --test $(MULTIARCH_SRC)/gdbstub/prot-none.py, \
> +	accessing PROT_NONE memory)
> +
>  else
>  run-gdbstub-%:
>  	$(call skip-test, "gdbstub test $*", "need working gdb with $(patsubst -%,,$(TARGET_NAME)) support")
>  endif
>  EXTRA_RUNS += run-gdbstub-sha1 run-gdbstub-qxfer-auxv-read \
>  	      run-gdbstub-proc-mappings run-gdbstub-thread-breakpoint \
> -	      run-gdbstub-registers
> +	      run-gdbstub-registers run-gdbstub-prot-none
>  
>  # ARM Compatible Semi Hosting Tests
>  #
> diff --git a/tests/tcg/multiarch/gdbstub/prot-none.py b/tests/tcg/multiarch/gdbstub/prot-none.py
> new file mode 100644
> index 00000000000..f1f1dd82cbe
> --- /dev/null
> +++ b/tests/tcg/multiarch/gdbstub/prot-none.py
> @@ -0,0 +1,22 @@
> +"""Test that GDB can access PROT_NONE pages.
> +
> +This runs as a sourced script (via -x, via run-test.py).
> +
> +SPDX-License-Identifier: GPL-2.0-or-later
> +"""
> +from test_gdbstub import main, report
> +
> +
> +def run_test():
> +    """Run through the tests one by one"""
> +    gdb.Breakpoint("break_here")
> +    gdb.execute("continue")
> +    val = gdb.parse_and_eval("*(char[2] *)q").string()
> +    report(val == "42", "{} == 42".format(val))
> +    gdb.execute("set *(char[3] *)q = \"24\"")
> +    gdb.execute("continue")
> +    exitcode = int(gdb.parse_and_eval("$_exitcode"))
> +    report(exitcode == 0, "{} == 0".format(exitcode))
> +
> +
> +main(run_test)
> diff --git a/tests/tcg/multiarch/prot-none.c b/tests/tcg/multiarch/prot-none.c
> new file mode 100644
> index 00000000000..dc56aadb3c5
> --- /dev/null
> +++ b/tests/tcg/multiarch/prot-none.c
> @@ -0,0 +1,40 @@
> +/*
> + * Test that GDB can access PROT_NONE pages.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +#include <assert.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <sys/mman.h>
> +#include <unistd.h>
> +
> +void break_here(void *q)
> +{
> +}
> +
> +int main(void)
> +{
> +    long pagesize = sysconf(_SC_PAGESIZE);
> +    void *p, *q;
> +    int err;
> +
> +    p = mmap(NULL, pagesize * 2, PROT_READ | PROT_WRITE,
> +             MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> +    assert(p != MAP_FAILED);
> +    q = p + pagesize - 1;
> +    strcpy(q, "42");
> +
> +    err = mprotect(p, pagesize * 2, PROT_NONE);
> +    assert(err == 0);
> +
> +    break_here(q);
> +
> +    err = mprotect(p, pagesize * 2, PROT_READ);
> +    assert(err == 0);
> +    if (getenv("PROT_NONE_PY")) {
> +        assert(strcmp(q, "24") == 0);
> +    }
> +
> +    return EXIT_SUCCESS;
> +}

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 3/3] tests/tcg: Add the PROT_NONE gdbstub test
  2024-01-16  0:31 ` [PATCH v3 3/3] tests/tcg: Add the PROT_NONE gdbstub test Ilya Leoshkevich
  2024-01-22 15:43   ` Alex Bennée
@ 2024-01-22 15:54   ` Alex Bennée
  2024-01-22 21:31     ` Ilya Leoshkevich
  1 sibling, 1 reply; 11+ messages in thread
From: Alex Bennée @ 2024-01-22 15:54 UTC (permalink / raw)
  To: Ilya Leoshkevich
  Cc: Richard Henderson, Peter Maydell, David Hildenbrand,
	Paolo Bonzini, Philippe Mathieu-Daudé, qemu-devel, qemu-arm,
	qemu-s390x

Ilya Leoshkevich <iii@linux.ibm.com> writes:

> Make sure that qemu gdbstub, like gdbserver, allows reading from and
> writing to PROT_NONE pages.
>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>  tests/tcg/multiarch/Makefile.target      |  9 +++++-
>  tests/tcg/multiarch/gdbstub/prot-none.py | 22 +++++++++++++
>  tests/tcg/multiarch/prot-none.c          | 40 ++++++++++++++++++++++++
>  3 files changed, 70 insertions(+), 1 deletion(-)
>  create mode 100644 tests/tcg/multiarch/gdbstub/prot-none.py
>  create mode 100644 tests/tcg/multiarch/prot-none.c
>
> diff --git a/tests/tcg/multiarch/Makefile.target b/tests/tcg/multiarch/Makefile.target
> index d31ba8d6ae4..315a2e13588 100644
> --- a/tests/tcg/multiarch/Makefile.target
> +++ b/tests/tcg/multiarch/Makefile.target
> @@ -101,13 +101,20 @@ run-gdbstub-registers: sha512
>  		--bin $< --test $(MULTIARCH_SRC)/gdbstub/registers.py, \
>  	checking register enumeration)
>  
> +run-gdbstub-prot-none: prot-none
> +	$(call run-test, $@, env PROT_NONE_PY=1 $(GDB_SCRIPT) \
> +		--gdb $(GDB) \
> +		--qemu $(QEMU) --qargs "$(QEMU_OPTS)" \
> +		--bin $< --test $(MULTIARCH_SRC)/gdbstub/prot-none.py, \
> +	accessing PROT_NONE memory)
> +
>  else
>  run-gdbstub-%:
>  	$(call skip-test, "gdbstub test $*", "need working gdb with $(patsubst -%,,$(TARGET_NAME)) support")
>  endif
>  EXTRA_RUNS += run-gdbstub-sha1 run-gdbstub-qxfer-auxv-read \
>  	      run-gdbstub-proc-mappings run-gdbstub-thread-breakpoint \
> -	      run-gdbstub-registers
> +	      run-gdbstub-registers run-gdbstub-prot-none
>  
>  # ARM Compatible Semi Hosting Tests
>  #
> diff --git a/tests/tcg/multiarch/gdbstub/prot-none.py b/tests/tcg/multiarch/gdbstub/prot-none.py
> new file mode 100644
> index 00000000000..f1f1dd82cbe
> --- /dev/null
> +++ b/tests/tcg/multiarch/gdbstub/prot-none.py
> @@ -0,0 +1,22 @@
> +"""Test that GDB can access PROT_NONE pages.
> +
> +This runs as a sourced script (via -x, via run-test.py).
> +
> +SPDX-License-Identifier: GPL-2.0-or-later
> +"""
> +from test_gdbstub import main, report
> +
> +
> +def run_test():
> +    """Run through the tests one by one"""
> +    gdb.Breakpoint("break_here")
> +    gdb.execute("continue")
> +    val = gdb.parse_and_eval("*(char[2] *)q").string()

Better traceback:

  Breakpoint 1, break_here (q=0x400000802fff) at /home/alex/lsrc/qemu.git/tests/tcg/multiarch/prot-none.c:14
  14      }
  GDB Exception:
  Traceback (most recent call last):
    File "/home/alex/lsrc/qemu.git/tests/guest-debug/test_gdbstub.py", line 42, in main
      test()
    File "./tests/tcg/multiarch/gdbstub/prot-none.py", line 14, in run_test
      val = gdb.parse_and_eval("*(char[2] *)q").string()
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  gdb.MemoryError: Cannot access memory at address 0x400000802fff
  Python 3.11.2 (main, Mar 13 2023, 12:18:29) [GCC 12.2.0] on linux
  Type "help", "copyright", "credits" or "license" for more information.
  (InteractiveConsole)
  >>> 

> +    report(val == "42", "{} == 42".format(val))
> +    gdb.execute("set *(char[3] *)q = \"24\"")
> +    gdb.execute("continue")
> +    exitcode = int(gdb.parse_and_eval("$_exitcode"))
> +    report(exitcode == 0, "{} == 0".format(exitcode))
> +
> +
> +main(run_test)
> diff --git a/tests/tcg/multiarch/prot-none.c b/tests/tcg/multiarch/prot-none.c
> new file mode 100644
> index 00000000000..dc56aadb3c5
> --- /dev/null
> +++ b/tests/tcg/multiarch/prot-none.c
> @@ -0,0 +1,40 @@
> +/*
> + * Test that GDB can access PROT_NONE pages.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +#include <assert.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <sys/mman.h>
> +#include <unistd.h>
> +
> +void break_here(void *q)
> +{
> +}
> +
> +int main(void)
> +{
> +    long pagesize = sysconf(_SC_PAGESIZE);
> +    void *p, *q;
> +    int err;
> +
> +    p = mmap(NULL, pagesize * 2, PROT_READ | PROT_WRITE,
> +             MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> +    assert(p != MAP_FAILED);
> +    q = p + pagesize - 1;
> +    strcpy(q, "42");
> +
> +    err = mprotect(p, pagesize * 2, PROT_NONE);
> +    assert(err == 0);
> +
> +    break_here(q);
> +
> +    err = mprotect(p, pagesize * 2, PROT_READ);
> +    assert(err == 0);
> +    if (getenv("PROT_NONE_PY")) {
> +        assert(strcmp(q, "24") == 0);
> +    }
> +
> +    return EXIT_SUCCESS;
> +}

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 2/3] tests/tcg: Factor out gdbstub test functions
  2024-01-16  0:31 ` [PATCH v3 2/3] tests/tcg: Factor out gdbstub test functions Ilya Leoshkevich
@ 2024-01-22 16:00   ` Alex Bennée
  2024-01-22 21:08     ` Ilya Leoshkevich
  0 siblings, 1 reply; 11+ messages in thread
From: Alex Bennée @ 2024-01-22 16:00 UTC (permalink / raw)
  To: Ilya Leoshkevich
  Cc: Richard Henderson, Peter Maydell, David Hildenbrand,
	Paolo Bonzini, Philippe Mathieu-Daudé, qemu-devel, qemu-arm,
	qemu-s390x

Ilya Leoshkevich <iii@linux.ibm.com> writes:

> Both the report() function as well as the initial gdbstub test sequence
> are copy-pasted into ~10 files with slight modifications. This
> indicates that they are indeed generic, so factor them out. While
> at it, add a few newlines to make the formatting closer to PEP-8.
>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>  tests/guest-debug/run-test.py                 |  7 ++-
>  tests/guest-debug/test_gdbstub.py             | 58 +++++++++++++++++++
>  tests/tcg/aarch64/gdbstub/test-sve-ioctl.py   | 34 +----------
>  tests/tcg/aarch64/gdbstub/test-sve.py         | 33 +----------
>  tests/tcg/multiarch/gdbstub/interrupt.py      | 47 ++-------------
>  tests/tcg/multiarch/gdbstub/memory.py         | 41 +------------
>  tests/tcg/multiarch/gdbstub/registers.py      | 41 ++-----------
>  tests/tcg/multiarch/gdbstub/sha1.py           | 40 ++-----------
>  .../multiarch/gdbstub/test-proc-mappings.py   | 39 +------------
>  .../multiarch/gdbstub/test-qxfer-auxv-read.py | 37 +-----------
>  .../gdbstub/test-thread-breakpoint.py         | 37 +-----------
>  tests/tcg/s390x/gdbstub/test-signals-s390x.py | 42 +-------------
>  tests/tcg/s390x/gdbstub/test-svc.py           | 39 +------------
>  13 files changed, 98 insertions(+), 397 deletions(-)
>  create mode 100644 tests/guest-debug/test_gdbstub.py
>
> diff --git a/tests/guest-debug/run-test.py b/tests/guest-debug/run-test.py
> index b13b27d4b19..368ff8a8903 100755
> --- a/tests/guest-debug/run-test.py
> +++ b/tests/guest-debug/run-test.py
> @@ -97,7 +97,12 @@ def log(output, msg):
>      sleep(1)
>      log(output, "GDB CMD: %s" % (gdb_cmd))
>  
> -    result = subprocess.call(gdb_cmd, shell=True, stdout=output, stderr=stderr)
> +    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)
> +    result = subprocess.call(gdb_cmd, shell=True, stdout=output, stderr=stderr,
> +                             env=gdb_env)
>  
>      # A result of greater than 128 indicates a fatal signal (likely a
>      # crash due to gdb internal failure). That's a problem for GDB and
> diff --git a/tests/guest-debug/test_gdbstub.py b/tests/guest-debug/test_gdbstub.py
> new file mode 100644
> index 00000000000..a71cdaa915a
> --- /dev/null
> +++ b/tests/guest-debug/test_gdbstub.py
> @@ -0,0 +1,58 @@
> +"""Helper functions for gdbstub testing
> +
> +"""
> +from __future__ import print_function
> +import gdb
> +import sys
> +import traceback
> +
> +fail_count = 0
> +
> +
> +def report(cond, msg):
> +    """Report success/fail of a test"""
> +    if cond:
> +        print("PASS: {}".format(msg))
> +    else:
> +        print("FAIL: {}".format(msg))
> +        global fail_count
> +        fail_count += 1
> +
> +
> +def main(test, expected_arch=None):
> +    """Run a test function
> +
> +    This runs as the script it sourced (via -x, via run-test.py)."""
> +    try:
> +        inferior = gdb.selected_inferior()
> +        arch = inferior.architecture()
> +        print("ATTACHED: {}".format(arch.name()))
> +        if expected_arch is not None:
> +            report(arch.name() == expected_arch,
> +                   "connected to {}".format(expected_arch))
> +    except (gdb.error, AttributeError):
> +        print("SKIP: not connected")
> +        exit(0)
> +
> +    if gdb.parse_and_eval("$pc") == 0:
> +        print("SKIP: PC not set")
> +        exit(0)
> +
> +    try:
> +        test()
> +    except:
> +        print("GDB Exception:")
> +        traceback.print_exc(file=sys.stdout)
> +        global fail_count
> +        fail_count += 1
> +        import code
> +        code.InteractiveConsole(locals=globals()).interact()
> +        raise

While I can see this is useful we don't want to default to an
interactive console as that will hang the test in CI type setups. Can we
make this a option we enable?

> +
> +    try:
> +        gdb.execute("kill")
> +    except gdb.error:
> +        pass
> +
> +    print("All tests complete: {} failures".format(fail_count))
> +    exit(fail_count)
> diff --git a/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py b/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
> index ee8d467e59d..a78a3a2514d 100644
> --- a/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
> +++ b/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
> @@ -8,19 +8,10 @@
>  #
>  
>  import gdb
> -import sys
> +from test_gdbstub import main, report
>  
>  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
>  
>  class TestBreakpoint(gdb.Breakpoint):
>      def __init__(self, sym_name="__sve_ld_done"):
> @@ -64,26 +55,5 @@ def run_test():
>  
>      gdb.execute("c")
>  
> -#
> -# This runs as the script it sourced (via -x, via run-test.py)
> -#
> -try:
> -    inferior = gdb.selected_inferior()
> -    arch = inferior.architecture()
> -    report(arch.name() == "aarch64", "connected to aarch64")
> -except (gdb.error, AttributeError):
> -    print("SKIPPING (not connected)", file=sys.stderr)
> -    exit(0)
> -
> -try:
> -    # Run the actual tests
> -    run_test()
> -except:
> -    print ("GDB Exception: %s" % (sys.exc_info()[0]))
> -    failcount += 1
> -    import code
> -    code.InteractiveConsole(locals=globals()).interact()
> -    raise
>  
> -print("All tests complete: %d failures" % failcount)
> -exit(failcount)
> +main(run_test, expected_arch="aarch64")
> diff --git a/tests/tcg/aarch64/gdbstub/test-sve.py b/tests/tcg/aarch64/gdbstub/test-sve.py
> index afd8ece98dd..84cdcd4a32e 100644
> --- a/tests/tcg/aarch64/gdbstub/test-sve.py
> +++ b/tests/tcg/aarch64/gdbstub/test-sve.py
> @@ -6,20 +6,10 @@
>  #
>  
>  import gdb
> -import sys
> +from test_gdbstub import main, report
>  
>  MAGIC = 0xDEADBEEF
>  
> -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
>  
>  def run_test():
>      "Run through the tests one by one"
> @@ -54,24 +44,5 @@ def run_test():
>              report(str(v.type) == "uint64_t", "size of %s" % (reg))
>              report(int(v) == MAGIC, "%s is 0x%x" % (reg, MAGIC))
>  
> -#
> -# This runs as the script it sourced (via -x, via run-test.py)
> -#
> -try:
> -    inferior = gdb.selected_inferior()
> -    arch = inferior.architecture()
> -    report(arch.name() == "aarch64", "connected to aarch64")
> -except (gdb.error, AttributeError):
> -    print("SKIPPING (not connected)", file=sys.stderr)
> -    exit(0)
> -
> -try:
> -    # Run the actual tests
> -    run_test()
> -except:
> -    print ("GDB Exception: %s" % (sys.exc_info()[0]))
> -    failcount += 1
> -
> -print("All tests complete: %d failures" % failcount)
>  
> -exit(failcount)
> +main(run_test, expected_arch="aarch64")
> diff --git a/tests/tcg/multiarch/gdbstub/interrupt.py b/tests/tcg/multiarch/gdbstub/interrupt.py
> index c016e7afbbf..90a45b5140a 100644
> --- a/tests/tcg/multiarch/gdbstub/interrupt.py
> +++ b/tests/tcg/multiarch/gdbstub/interrupt.py
> @@ -8,19 +8,7 @@
>  #
>  
>  import gdb
> -import sys
> -
> -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
> +from test_gdbstub import main, report
>  
>  
>  def check_interrupt(thread):
> @@ -59,6 +47,9 @@ def run_test():
>      Test if interrupting the code always lands us on the same thread when
>      running with scheduler-lock enabled.
>      """
> +    if len(gdb.selected_inferior().threads()) == 1:
> +        print("SKIP: set to run on a single thread")
> +        exit(0)
>  
>      gdb.execute("set scheduler-locking on")
>      for thread in gdb.selected_inferior().threads():
> @@ -66,32 +57,4 @@ def run_test():
>                 "thread %d resumes correctly on interrupt" % thread.num)
>  
>  
> -#
> -# 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)
> -if len(gdb.selected_inferior().threads()) == 1:
> -    print("SKIP: set to run on a single thread")
> -    exit(0)
> -
> -try:
> -    # Run the actual tests
> -    run_test()
> -except (gdb.error):
> -    print("GDB Exception: %s" % (sys.exc_info()[0]))
> -    failcount += 1
> -    pass
> -
> -# Finally kill the inferior and exit gdb with a count of failures
> -gdb.execute("kill")
> -exit(failcount)
> +main(run_test)
> diff --git a/tests/tcg/multiarch/gdbstub/memory.py b/tests/tcg/multiarch/gdbstub/memory.py
> index fb1d06b7bb7..532b92e7fb3 100644
> --- a/tests/tcg/multiarch/gdbstub/memory.py
> +++ b/tests/tcg/multiarch/gdbstub/memory.py
> @@ -9,18 +9,7 @@
>  
>  import gdb
>  import sys
> -
> -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
> +from test_gdbstub import main, report
>  
>  
>  def check_step():
> @@ -99,29 +88,5 @@ def run_test():
>  
>      report(cbp.hit_count == 0, "didn't reach backstop")
>  
> -#
> -# 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 the actual tests
> -    run_test()
> -except (gdb.error):
> -    print("GDB Exception: %s" % (sys.exc_info()[0]))
> -    failcount += 1
> -    pass
> -
> -# Finally kill the inferior and exit gdb with a count of failures
> -gdb.execute("kill")
> -exit(failcount)
> +
> +main(run_test)
> diff --git a/tests/tcg/multiarch/gdbstub/registers.py b/tests/tcg/multiarch/gdbstub/registers.py
> index 688c0611072..b3d13cb077f 100644
> --- a/tests/tcg/multiarch/gdbstub/registers.py
> +++ b/tests/tcg/multiarch/gdbstub/registers.py
> @@ -7,20 +7,11 @@
>  # SPDX-License-Identifier: GPL-2.0-or-later
>  
>  import gdb
> -import sys
>  import xml.etree.ElementTree as ET
> +from test_gdbstub import main, report
>  
> -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
> +initial_vlen = 0
>  
>  
>  def fetch_xml_regmap():
> @@ -75,6 +66,7 @@ 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
> @@ -84,6 +76,7 @@ def get_register_by_regnum(reg_map, regnum):
>              return entry
>      return None
>  
> +
>  def crosscheck_remote_xml(reg_map):
>      """
>      Cross-check the list of remote-registers with the XML info.
> @@ -144,6 +137,7 @@ def crosscheck_remote_xml(reg_map):
>          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
> @@ -214,27 +208,4 @@ def run_test():
>          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)
> +main(run_test)
> diff --git a/tests/tcg/multiarch/gdbstub/sha1.py b/tests/tcg/multiarch/gdbstub/sha1.py
> index 416728415f9..1ce711a402c 100644
> --- a/tests/tcg/multiarch/gdbstub/sha1.py
> +++ b/tests/tcg/multiarch/gdbstub/sha1.py
> @@ -7,19 +7,11 @@
>  #
>  
>  import gdb
> -import sys
> +from test_gdbstub import main, report
> +
>  
>  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
>  
>  def check_break(sym_name):
>      "Setup breakpoint, continue and check we stopped."
> @@ -35,6 +27,7 @@ def check_break(sym_name):
>  
>      bp.delete()
>  
> +
>  def run_test():
>      "Run through the tests one by one"
>  
> @@ -57,28 +50,5 @@ def run_test():
>      # finally check we don't barf inspecting registers
>      gdb.execute("info registers")
>  
> -#
> -# 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 the actual tests
> -    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)
> +
> +main(run_test)
> diff --git a/tests/tcg/multiarch/gdbstub/test-proc-mappings.py b/tests/tcg/multiarch/gdbstub/test-proc-mappings.py
> index 04ec61d2197..564613fabf0 100644
> --- a/tests/tcg/multiarch/gdbstub/test-proc-mappings.py
> +++ b/tests/tcg/multiarch/gdbstub/test-proc-mappings.py
> @@ -3,20 +3,7 @@
>  This runs as a sourced script (via -x, via run-test.py)."""
>  from __future__ import print_function
>  import gdb
> -import sys
> -
> -
> -n_failures = 0
> -
> -
> -def report(cond, msg):
> -    """Report success/fail of a test"""
> -    if cond:
> -        print("PASS: {}".format(msg))
> -    else:
> -        print("FAIL: {}".format(msg))
> -        global n_failures
> -        n_failures += 1
> +from test_gdbstub import main, report
>  
>  
>  def run_test():
> @@ -37,26 +24,4 @@ def run_test():
>      # report("/sha1" in mappings, "Found the test binary name in the mappings")
>  
>  
> -def main():
> -    """Prepare the environment and run through the tests"""
> -    try:
> -        inferior = gdb.selected_inferior()
> -        print("ATTACHED: {}".format(inferior.architecture().name()))
> -    except (gdb.error, AttributeError):
> -        print("SKIPPING (not connected)")
> -        exit(0)
> -
> -    if gdb.parse_and_eval('$pc') == 0:
> -        print("SKIP: PC not set")
> -        exit(0)
> -
> -    try:
> -        # Run the actual tests
> -        run_test()
> -    except gdb.error:
> -        report(False, "GDB Exception: {}".format(sys.exc_info()[0]))
> -    print("All tests complete: %d failures" % n_failures)
> -    exit(n_failures)
> -
> -
> -main()
> +main(run_test)
> diff --git a/tests/tcg/multiarch/gdbstub/test-qxfer-auxv-read.py b/tests/tcg/multiarch/gdbstub/test-qxfer-auxv-read.py
> index 926fa962b77..00c26ab4a95 100644
> --- a/tests/tcg/multiarch/gdbstub/test-qxfer-auxv-read.py
> +++ b/tests/tcg/multiarch/gdbstub/test-qxfer-auxv-read.py
> @@ -6,18 +6,8 @@
>  #
>  
>  import gdb
> -import sys
> +from test_gdbstub import main, report
>  
> -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
>  
>  def run_test():
>      "Run through the tests one by one"
> @@ -26,28 +16,5 @@ def run_test():
>      report(isinstance(auxv, str), "Fetched auxv from inferior")
>      report(auxv.find("sha1"), "Found test binary name in auxv")
>  
> -#
> -# 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 the actual tests
> -    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)
> +main(run_test)
> diff --git a/tests/tcg/multiarch/gdbstub/test-thread-breakpoint.py b/tests/tcg/multiarch/gdbstub/test-thread-breakpoint.py
> index e57d2a8db8b..4d6b6b9fbe7 100644
> --- a/tests/tcg/multiarch/gdbstub/test-thread-breakpoint.py
> +++ b/tests/tcg/multiarch/gdbstub/test-thread-breakpoint.py
> @@ -6,18 +6,8 @@
>  #
>  
>  import gdb
> -import sys
> +from test_gdbstub import main, report
>  
> -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
>  
>  def run_test():
>      "Run through the tests one by one"
> @@ -29,28 +19,5 @@ def run_test():
>      frame = gdb.selected_frame()
>      report(str(frame.function()) == "thread1_func", "break @ %s"%frame)
>  
> -#
> -# 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 the actual tests
> -    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)
> +main(run_test)
> diff --git a/tests/tcg/s390x/gdbstub/test-signals-s390x.py b/tests/tcg/s390x/gdbstub/test-signals-s390x.py
> index ca2bbc0b03e..b6b7b39fc46 100644
> --- a/tests/tcg/s390x/gdbstub/test-signals-s390x.py
> +++ b/tests/tcg/s390x/gdbstub/test-signals-s390x.py
> @@ -7,19 +7,7 @@
>  #
>  
>  import gdb
> -import sys
> -
> -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
> +from test_gdbstub import main, report
>  
>  
>  def run_test():
> @@ -42,31 +30,7 @@ def run_test():
>      gdb.Breakpoint("_exit")
>      gdb.execute("c")
>      status = int(gdb.parse_and_eval("$r2"))
> -    report(status == 0, "status == 0");
> -
> -
> -#
> -# 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)
> +    report(status == 0, "status == 0")
>  
> -try:
> -    # Run the actual tests
> -    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)
> +main(run_test)
> diff --git a/tests/tcg/s390x/gdbstub/test-svc.py b/tests/tcg/s390x/gdbstub/test-svc.py
> index 804705fede9..17210b4e020 100644
> --- a/tests/tcg/s390x/gdbstub/test-svc.py
> +++ b/tests/tcg/s390x/gdbstub/test-svc.py
> @@ -3,20 +3,7 @@
>  This runs as a sourced script (via -x, via run-test.py)."""
>  from __future__ import print_function
>  import gdb
> -import sys
> -
> -
> -n_failures = 0
> -
> -
> -def report(cond, msg):
> -    """Report success/fail of a test"""
> -    if cond:
> -        print("PASS: {}".format(msg))
> -    else:
> -        print("FAIL: {}".format(msg))
> -        global n_failures
> -        n_failures += 1
> +from test_gdbstub import main, report
>  
>  
>  def run_test():
> @@ -35,26 +22,4 @@ def run_test():
>      gdb.execute("si")
>  
>  
> -def main():
> -    """Prepare the environment and run through the tests"""
> -    try:
> -        inferior = gdb.selected_inferior()
> -        print("ATTACHED: {}".format(inferior.architecture().name()))
> -    except (gdb.error, AttributeError):
> -        print("SKIPPING (not connected)")
> -        exit(0)
> -
> -    if gdb.parse_and_eval('$pc') == 0:
> -        print("SKIP: PC not set")
> -        exit(0)
> -
> -    try:
> -        # Run the actual tests
> -        run_test()
> -    except gdb.error:
> -        report(False, "GDB Exception: {}".format(sys.exc_info()[0]))
> -    print("All tests complete: %d failures" % n_failures)
> -    exit(n_failures)
> -
> -
> -main()
> +main(run_test)

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Re: [PATCH v3 2/3] tests/tcg: Factor out gdbstub test functions
  2024-01-22 16:00   ` Alex Bennée
@ 2024-01-22 21:08     ` Ilya Leoshkevich
  0 siblings, 0 replies; 11+ messages in thread
From: Ilya Leoshkevich @ 2024-01-22 21:08 UTC (permalink / raw)
  To: Alex Bennée
  Cc: Richard Henderson, Peter Maydell, David Hildenbrand,
	Paolo Bonzini, Philippe Mathieu-Daudé, qemu-devel, qemu-arm,
	qemu-s390x

On Mon, Jan 22, 2024 at 04:00:44PM +0000, Alex Bennée wrote:
> Ilya Leoshkevich <iii@linux.ibm.com> writes:
> 
> > Both the report() function as well as the initial gdbstub test sequence
> > are copy-pasted into ~10 files with slight modifications. This
> > indicates that they are indeed generic, so factor them out. While
> > at it, add a few newlines to make the formatting closer to PEP-8.
> >
> > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > ---
> >  tests/guest-debug/run-test.py                 |  7 ++-
> >  tests/guest-debug/test_gdbstub.py             | 58 +++++++++++++++++++
> >  tests/tcg/aarch64/gdbstub/test-sve-ioctl.py   | 34 +----------
> >  tests/tcg/aarch64/gdbstub/test-sve.py         | 33 +----------
> >  tests/tcg/multiarch/gdbstub/interrupt.py      | 47 ++-------------
> >  tests/tcg/multiarch/gdbstub/memory.py         | 41 +------------
> >  tests/tcg/multiarch/gdbstub/registers.py      | 41 ++-----------
> >  tests/tcg/multiarch/gdbstub/sha1.py           | 40 ++-----------
> >  .../multiarch/gdbstub/test-proc-mappings.py   | 39 +------------
> >  .../multiarch/gdbstub/test-qxfer-auxv-read.py | 37 +-----------
> >  .../gdbstub/test-thread-breakpoint.py         | 37 +-----------
> >  tests/tcg/s390x/gdbstub/test-signals-s390x.py | 42 +-------------
> >  tests/tcg/s390x/gdbstub/test-svc.py           | 39 +------------
> >  13 files changed, 98 insertions(+), 397 deletions(-)
> >  create mode 100644 tests/guest-debug/test_gdbstub.py

[...]

> > +    if gdb.parse_and_eval("$pc") == 0:
> > +        print("SKIP: PC not set")
> > +        exit(0)
> > +
> > +    try:
> > +        test()
> > +    except:
> > +        print("GDB Exception:")
> > +        traceback.print_exc(file=sys.stdout)
> > +        global fail_count
> > +        fail_count += 1
> > +        import code
> > +        code.InteractiveConsole(locals=globals()).interact()
> > +        raise
> 
> While I can see this is useful we don't want to default to an
> interactive console as that will hang the test in CI type setups. Can we
> make this a option we enable?

Would something like `export QEMU_TEST_INTERACTIVE=1` be okay?

[...]


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Re: [PATCH v3 3/3] tests/tcg: Add the PROT_NONE gdbstub test
  2024-01-22 15:54   ` Alex Bennée
@ 2024-01-22 21:31     ` Ilya Leoshkevich
  2024-01-22 23:19       ` Alex Bennée
  0 siblings, 1 reply; 11+ messages in thread
From: Ilya Leoshkevich @ 2024-01-22 21:31 UTC (permalink / raw)
  To: Alex Bennée
  Cc: Richard Henderson, Peter Maydell, David Hildenbrand,
	Paolo Bonzini, Philippe Mathieu-Daudé, qemu-devel, qemu-arm,
	qemu-s390x

On Mon, Jan 22, 2024 at 03:54:32PM +0000, Alex Bennée wrote:
> Ilya Leoshkevich <iii@linux.ibm.com> writes:
> 
> > Make sure that qemu gdbstub, like gdbserver, allows reading from and
> > writing to PROT_NONE pages.
> >
> > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > ---
> >  tests/tcg/multiarch/Makefile.target      |  9 +++++-
> >  tests/tcg/multiarch/gdbstub/prot-none.py | 22 +++++++++++++
> >  tests/tcg/multiarch/prot-none.c          | 40 ++++++++++++++++++++++++
> >  3 files changed, 70 insertions(+), 1 deletion(-)
> >  create mode 100644 tests/tcg/multiarch/gdbstub/prot-none.py
> >  create mode 100644 tests/tcg/multiarch/prot-none.c

[...]

> > +def run_test():
> > +    """Run through the tests one by one"""
> > +    gdb.Breakpoint("break_here")
> > +    gdb.execute("continue")
> > +    val = gdb.parse_and_eval("*(char[2] *)q").string()
> 
> Better traceback:
> 
>   Breakpoint 1, break_here (q=0x400000802fff) at /home/alex/lsrc/qemu.git/tests/tcg/multiarch/prot-none.c:14
>   14      }
>   GDB Exception:
>   Traceback (most recent call last):
>     File "/home/alex/lsrc/qemu.git/tests/guest-debug/test_gdbstub.py", line 42, in main
>       test()
>     File "./tests/tcg/multiarch/gdbstub/prot-none.py", line 14, in run_test
>       val = gdb.parse_and_eval("*(char[2] *)q").string()
>             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>   gdb.MemoryError: Cannot access memory at address 0x400000802fff
>   Python 3.11.2 (main, Mar 13 2023, 12:18:29) [GCC 12.2.0] on linux
>   Type "help", "copyright", "credits" or "license" for more information.
>   (InteractiveConsole)
>   >>> 

Thanks for the debug output. This shows that the feature being tested
doesn't work (the value of `q` looks sane to me). May I ask what host
distro is this? I tried on x86_64 Fedora 38 and x86_64 Ubuntu 22.04 so
far, and the test was successful.

[...]


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 3/3] tests/tcg: Add the PROT_NONE gdbstub test
  2024-01-22 21:31     ` Ilya Leoshkevich
@ 2024-01-22 23:19       ` Alex Bennée
  2024-01-25  2:39         ` Ilya Leoshkevich
  0 siblings, 1 reply; 11+ messages in thread
From: Alex Bennée @ 2024-01-22 23:19 UTC (permalink / raw)
  To: Ilya Leoshkevich
  Cc: Richard Henderson, Peter Maydell, David Hildenbrand,
	Paolo Bonzini, Philippe Mathieu-Daudé, qemu-devel, qemu-arm,
	qemu-s390x

Ilya Leoshkevich <iii@linux.ibm.com> writes:

> On Mon, Jan 22, 2024 at 03:54:32PM +0000, Alex Bennée wrote:
>> Ilya Leoshkevich <iii@linux.ibm.com> writes:
>> 
>> > Make sure that qemu gdbstub, like gdbserver, allows reading from and
>> > writing to PROT_NONE pages.
>> >
>> > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
>> > ---
>> >  tests/tcg/multiarch/Makefile.target      |  9 +++++-
>> >  tests/tcg/multiarch/gdbstub/prot-none.py | 22 +++++++++++++
>> >  tests/tcg/multiarch/prot-none.c          | 40 ++++++++++++++++++++++++
>> >  3 files changed, 70 insertions(+), 1 deletion(-)
>> >  create mode 100644 tests/tcg/multiarch/gdbstub/prot-none.py
>> >  create mode 100644 tests/tcg/multiarch/prot-none.c
>
> [...]
>
>> > +def run_test():
>> > +    """Run through the tests one by one"""
>> > +    gdb.Breakpoint("break_here")
>> > +    gdb.execute("continue")
>> > +    val = gdb.parse_and_eval("*(char[2] *)q").string()
>> 
>> Better traceback:
>> 
>>   Breakpoint 1, break_here (q=0x400000802fff) at /home/alex/lsrc/qemu.git/tests/tcg/multiarch/prot-none.c:14
>>   14      }
>>   GDB Exception:
>>   Traceback (most recent call last):
>>     File "/home/alex/lsrc/qemu.git/tests/guest-debug/test_gdbstub.py", line 42, in main
>>       test()
>>     File "./tests/tcg/multiarch/gdbstub/prot-none.py", line 14, in run_test
>>       val = gdb.parse_and_eval("*(char[2] *)q").string()
>>             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>   gdb.MemoryError: Cannot access memory at address 0x400000802fff
>>   Python 3.11.2 (main, Mar 13 2023, 12:18:29) [GCC 12.2.0] on linux
>>   Type "help", "copyright", "credits" or "license" for more information.
>>   (InteractiveConsole)
>>   >>> 
>
> Thanks for the debug output. This shows that the feature being tested
> doesn't work (the value of `q` looks sane to me). May I ask what host
> distro is this? I tried on x86_64 Fedora 38 and x86_64 Ubuntu 22.04 so
> far, and the test was successful.

Debian Bookworm (x86_64) with gdb-multiarch installed.
>
> [...]

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 3/3] tests/tcg: Add the PROT_NONE gdbstub test
  2024-01-22 23:19       ` Alex Bennée
@ 2024-01-25  2:39         ` Ilya Leoshkevich
  0 siblings, 0 replies; 11+ messages in thread
From: Ilya Leoshkevich @ 2024-01-25  2:39 UTC (permalink / raw)
  To: Alex Bennée
  Cc: Richard Henderson, Peter Maydell, David Hildenbrand,
	Paolo Bonzini, Philippe Mathieu-Daudé, qemu-devel, qemu-arm,
	qemu-s390x

On Mon, Jan 22, 2024 at 11:19:05PM +0000, Alex Bennée wrote:
> Ilya Leoshkevich <iii@linux.ibm.com> writes:
> 
> > On Mon, Jan 22, 2024 at 03:54:32PM +0000, Alex Bennée wrote:
> >> Ilya Leoshkevich <iii@linux.ibm.com> writes:
> >> 
> >> > Make sure that qemu gdbstub, like gdbserver, allows reading from and
> >> > writing to PROT_NONE pages.
> >> >
> >> > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> >> > ---
> >> >  tests/tcg/multiarch/Makefile.target      |  9 +++++-
> >> >  tests/tcg/multiarch/gdbstub/prot-none.py | 22 +++++++++++++
> >> >  tests/tcg/multiarch/prot-none.c          | 40 ++++++++++++++++++++++++
> >> >  3 files changed, 70 insertions(+), 1 deletion(-)
> >> >  create mode 100644 tests/tcg/multiarch/gdbstub/prot-none.py
> >> >  create mode 100644 tests/tcg/multiarch/prot-none.c
> >
> > [...]
> >
> >> > +def run_test():
> >> > +    """Run through the tests one by one"""
> >> > +    gdb.Breakpoint("break_here")
> >> > +    gdb.execute("continue")
> >> > +    val = gdb.parse_and_eval("*(char[2] *)q").string()
> >> 
> >> Better traceback:
> >> 
> >>   Breakpoint 1, break_here (q=0x400000802fff) at /home/alex/lsrc/qemu.git/tests/tcg/multiarch/prot-none.c:14
> >>   14      }
> >>   GDB Exception:
> >>   Traceback (most recent call last):
> >>     File "/home/alex/lsrc/qemu.git/tests/guest-debug/test_gdbstub.py", line 42, in main
> >>       test()
> >>     File "./tests/tcg/multiarch/gdbstub/prot-none.py", line 14, in run_test
> >>       val = gdb.parse_and_eval("*(char[2] *)q").string()
> >>             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >>   gdb.MemoryError: Cannot access memory at address 0x400000802fff
> >>   Python 3.11.2 (main, Mar 13 2023, 12:18:29) [GCC 12.2.0] on linux
> >>   Type "help", "copyright", "credits" or "license" for more information.
> >>   (InteractiveConsole)
> >>   >>> 
> >
> > Thanks for the debug output. This shows that the feature being tested
> > doesn't work (the value of `q` looks sane to me). May I ask what host
> > distro is this? I tried on x86_64 Fedora 38 and x86_64 Ubuntu 22.04 so
> > far, and the test was successful.
> 
> Debian Bookworm (x86_64) with gdb-multiarch installed.
> >
> > [...]
> 
> -- 
> Alex Bennée
> Virtualisation Tech Lead @ Linaro

Hm, I tried that (in a VM, in case the kernel is somehow involved) and
that worked too:

  Breakpoint 1, break_here (q=0x400000802fff) at /qemu/tests/tcg/multiarch/prot-none.c:14
  14      }
  PASS: 42 == 42

I wonder what else can be different. Can it be that in your case the
test runs without /proc? Or perhaps some additional LSM is enabled?


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2024-01-25  2:41 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-16  0:31 [PATCH v3 0/3] linux-user: Allow gdbstub to ignore page protection Ilya Leoshkevich
2024-01-16  0:31 ` [PATCH v3 1/3] " Ilya Leoshkevich
2024-01-16  0:31 ` [PATCH v3 2/3] tests/tcg: Factor out gdbstub test functions Ilya Leoshkevich
2024-01-22 16:00   ` Alex Bennée
2024-01-22 21:08     ` Ilya Leoshkevich
2024-01-16  0:31 ` [PATCH v3 3/3] tests/tcg: Add the PROT_NONE gdbstub test Ilya Leoshkevich
2024-01-22 15:43   ` Alex Bennée
2024-01-22 15:54   ` Alex Bennée
2024-01-22 21:31     ` Ilya Leoshkevich
2024-01-22 23:19       ` Alex Bennée
2024-01-25  2:39         ` Ilya Leoshkevich

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).