[PATCH] gdbstub: fixes cases where wrong threads were reported to GDB on SIGINT

Matheus Branco Borella posted 1 patch 10 months, 4 weeks ago
Failed in applying to current master (apply log)
There is a newer version of this series
gdbstub/gdbstub.c | 31 ++++++++++++++++++++++++++++++-
1 file changed, 30 insertions(+), 1 deletion(-)
[PATCH] gdbstub: fixes cases where wrong threads were reported to GDB on SIGINT
Posted by Matheus Branco Borella 10 months, 4 weeks ago
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1725

This fix is implemented by having the vCont handler set the value of
`gdbserver_state.c_cpu` if any threads are to be resumed. The specific CPU
is picked arbitrarily from the ones to be resumed, but it should be okay, as all
GDB cares about is that it is a resumed thread.

Keep in mind that because this patch overwrites `c_cpu`, it breaks cases where
$vCont is used together with $Hc, so there might be more work to be done here.
It might also be the case that it breaking this, specifically, isn't of
consequence, seeing as single stepping with $vCont already overwrites `c_cpu`
anyway, so you could say the implementation already behaves oddly as far as
mixing $vCont and $Hc is concerned.
---
 gdbstub/gdbstub.c | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index be18568d0a..4f7ac5ddfe 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -595,6 +595,15 @@ static int gdb_handle_vcont(const char *p)
      *  or incorrect parameters passed.
      */
     res = 0;
+    
+    /* 
+     * target_count and last_target keep track of how many CPUs we are going to
+     * step or resume, and a pointer to the state structure of one of them, 
+     * respectivelly
+     */
+    int target_count = 0;
+    CPUState *last_target = NULL;
+
     while (*p) {
         if (*p++ != ';') {
             res = -ENOTSUP;
@@ -639,8 +648,10 @@ static int gdb_handle_vcont(const char *p)
             while (cpu) {
                 if (newstates[cpu->cpu_index] == 1) {
                     newstates[cpu->cpu_index] = cur_action;
-                }
 
+                    target_count++;
+                    last_target = cpu;
+                }
                 cpu = gdb_next_attached_cpu(cpu);
             }
             break;
@@ -657,6 +668,9 @@ static int gdb_handle_vcont(const char *p)
             while (cpu) {
                 if (newstates[cpu->cpu_index] == 1) {
                     newstates[cpu->cpu_index] = cur_action;
+                    
+                    target_count++;
+                    last_target = cpu;
                 }
 
                 cpu = gdb_next_cpu_in_process(cpu);
@@ -675,10 +689,25 @@ static int gdb_handle_vcont(const char *p)
             /* only use if no previous match occourred */
             if (newstates[cpu->cpu_index] == 1) {
                 newstates[cpu->cpu_index] = cur_action;
+
+                target_count++;
+                last_target = cpu;
             }
             break;
         }
     }
+
+    /* 
+     * if we're about to resume a specific set of CPUs/threads, make it so that 
+     * in case execution gets interrupted, we can send GDB a stop reply with a
+     * correct value. it doesn't really matter which CPU we tell GDB the signal 
+     * happened in (VM pauses stop all of them anyway), so long as it is one of
+     * the ones we resumed/single stepped here.
+     */
+    if (target_count > 0) {
+        gdbserver_state.c_cpu = last_target;
+    }
+
     gdbserver_state.signal = signal;
     gdb_continue_partial(newstates);
 
-- 
2.40.1
Re: [PATCH] gdbstub: fixes cases where wrong threads were reported to GDB on SIGINT
Posted by Alex Bennée 10 months, 3 weeks ago
Matheus Branco Borella <dark.ryu.550@gmail.com> writes:

> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1725
>
> This fix is implemented by having the vCont handler set the value of
> `gdbserver_state.c_cpu` if any threads are to be resumed. The specific CPU
> is picked arbitrarily from the ones to be resumed, but it should be okay, as all
> GDB cares about is that it is a resumed thread.
>
> Keep in mind that because this patch overwrites `c_cpu`, it breaks cases where
> $vCont is used together with $Hc, so there might be more work to be
> done here.

That doesn't sound good. Is that a possible case or an invalid one
because we shouldn't see gdbs using both?

> It might also be the case that it breaking this, specifically, isn't of
> consequence, seeing as single stepping with $vCont already overwrites `c_cpu`
> anyway, so you could say the implementation already behaves oddly as far as
> mixing $vCont and $Hc is concerned.

It would be nice to have some unit tests for this behaviour to defend
it. See the various tests in tests/tcg that call $(GDB_SCRIPT) for
examples.

BTW you are missing a Signed-off-by: tag which we will need to take a
patch submission. See:

  https://qemu.readthedocs.io/en/latest/devel/submitting-a-patch.html


> ---
>  gdbstub/gdbstub.c | 31 ++++++++++++++++++++++++++++++-
>  1 file changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
> index be18568d0a..4f7ac5ddfe 100644
> --- a/gdbstub/gdbstub.c
> +++ b/gdbstub/gdbstub.c
> @@ -595,6 +595,15 @@ static int gdb_handle_vcont(const char *p)
>       *  or incorrect parameters passed.
>       */
>      res = 0;
> +    
> +    /* 
> +     * target_count and last_target keep track of how many CPUs we are going to
> +     * step or resume, and a pointer to the state structure of one of them, 
> +     * respectivelly
> +     */
> +    int target_count = 0;
> +    CPUState *last_target = NULL;
> +
>      while (*p) {
>          if (*p++ != ';') {
>              res = -ENOTSUP;
> @@ -639,8 +648,10 @@ static int gdb_handle_vcont(const char *p)
>              while (cpu) {
>                  if (newstates[cpu->cpu_index] == 1) {
>                      newstates[cpu->cpu_index] = cur_action;
> -                }
>  
> +                    target_count++;
> +                    last_target = cpu;
> +                }
>                  cpu = gdb_next_attached_cpu(cpu);
>              }
>              break;
> @@ -657,6 +668,9 @@ static int gdb_handle_vcont(const char *p)
>              while (cpu) {
>                  if (newstates[cpu->cpu_index] == 1) {
>                      newstates[cpu->cpu_index] = cur_action;
> +                    
> +                    target_count++;
> +                    last_target = cpu;
>                  }
>  
>                  cpu = gdb_next_cpu_in_process(cpu);
> @@ -675,10 +689,25 @@ static int gdb_handle_vcont(const char *p)
>              /* only use if no previous match occourred */
>              if (newstates[cpu->cpu_index] == 1) {
>                  newstates[cpu->cpu_index] = cur_action;
> +
> +                target_count++;
> +                last_target = cpu;
>              }
>              break;
>          }
>      }
> +
> +    /* 
> +     * if we're about to resume a specific set of CPUs/threads, make it so that 
> +     * in case execution gets interrupted, we can send GDB a stop reply with a
> +     * correct value. it doesn't really matter which CPU we tell GDB the signal 
> +     * happened in (VM pauses stop all of them anyway), so long as it is one of
> +     * the ones we resumed/single stepped here.
> +     */
> +    if (target_count > 0) {
> +        gdbserver_state.c_cpu = last_target;
> +    }
> +
>      gdbserver_state.signal = signal;
>      gdb_continue_partial(newstates);

Looks reasonable at first glance but I would like some tests.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro
[PATCH v2] gdbstub: fixes cases where wrong threads were reported to GDB on SIGINT
Posted by Matheus Branco Borella 9 months, 2 weeks ago
Alex Bennée <alex.bennee@linaro.org> writes:
> Can gdb switch which packet sequence it uses to halt and restart
> threads?

Yes, but the way it does it does not trigger the behavior I was concerned 
about. GDB falls back to the old sequence when either (1) the target does not
support the vCont command it's trying to send or (2) you step backwards. In both
cases, though, whenever it does fall back, it will first send an Hc packet 
before continuing or stepping, which means we won't ever see a sequence such as
["Hc", "vCont;c:*", "c"]. This means, in short, that, while the shortcoming does
exist in the code, GDB never actually triggers it.

> The test I would like see is pretty much your test case
> 
>  - load a multi-threaded program
>  - wait until threads running
>  - pause
>  - resume thread
>  - check resumed thread was the right one

What I have here should be pretty much that. 

Is there something else you think I'm missing?

---
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1725

This fix is implemented by having the vCont handler set the value of
`gdbserver_state.c_cpu` if any threads are to be resumed. The specific CPU
picked is arbitrarily from the ones to be resumed, but it should be okay, as all
GDB cares about is that it is a resumed thread.

Signed-off-by: Matheus Branco Borella <dark.ryu.550@gmail.com>
---
 gdbstub/gdbstub.c                             | 29 ++++++
 tests/tcg/multiarch/gdbstub/interrupt.py      | 99 +++++++++++++++++++
 .../multiarch/system/Makefile.softmmu-target  | 12 ++-
 tests/tcg/multiarch/system/interrupt.c        | 25 +++++
 4 files changed, 163 insertions(+), 2 deletions(-)
 create mode 100644 tests/tcg/multiarch/gdbstub/interrupt.py
 create mode 100644 tests/tcg/multiarch/system/interrupt.c

diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 6911b73c07..5763399f8b 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -597,6 +597,15 @@ static int gdb_handle_vcont(const char *p)
      *  or incorrect parameters passed.
      */
     res = 0;
+
+    /*
+     * target_count and last_target keep track of how many CPUs we are going to
+     * step or resume, and a pointer to the state structure of one of them,
+     * respectivelly
+     */
+    int target_count = 0;
+    CPUState *last_target = NULL;
+
     while (*p) {
         if (*p++ != ';') {
             return -ENOTSUP;
@@ -637,6 +646,9 @@ static int gdb_handle_vcont(const char *p)
             while (cpu) {
                 if (newstates[cpu->cpu_index] == 1) {
                     newstates[cpu->cpu_index] = cur_action;
+
+                    target_count++;
+                    last_target = cpu;
                 }
 
                 cpu = gdb_next_attached_cpu(cpu);
@@ -654,6 +666,9 @@ static int gdb_handle_vcont(const char *p)
             while (cpu) {
                 if (newstates[cpu->cpu_index] == 1) {
                     newstates[cpu->cpu_index] = cur_action;
+
+                    target_count++;
+                    last_target = cpu;
                 }
 
                 cpu = gdb_next_cpu_in_process(cpu);
@@ -671,11 +686,25 @@ static int gdb_handle_vcont(const char *p)
             /* only use if no previous match occourred */
             if (newstates[cpu->cpu_index] == 1) {
                 newstates[cpu->cpu_index] = cur_action;
+
+                target_count++;
+                last_target = cpu;
             }
             break;
         }
     }
 
+    /*
+     * if we're about to resume a specific set of CPUs/threads, make it so that
+     * in case execution gets interrupted, we can send GDB a stop reply with a
+     * correct value. it doesn't really matter which CPU we tell GDB the signal
+     * happened in (VM pauses stop all of them anyway), so long as it is one of
+     * the ones we resumed/single stepped here.
+     */
+    if (target_count > 0) {
+        gdbserver_state.c_cpu = last_target;
+    }
+
     gdbserver_state.signal = signal;
     gdb_continue_partial(newstates);
     return res;
diff --git a/tests/tcg/multiarch/gdbstub/interrupt.py b/tests/tcg/multiarch/gdbstub/interrupt.py
new file mode 100644
index 0000000000..0b0b5a30f6
--- /dev/null
+++ b/tests/tcg/multiarch/gdbstub/interrupt.py
@@ -0,0 +1,99 @@
+from __future__ import print_function
+#
+# Test some of the softmmu debug features with the multiarch memory
+# test. It is a port of the original vmlinux focused test case but
+# using the "memory" test instead.
+#
+# This is launched via tests/guest-debug/run-test.py
+#
+
+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
+
+def check_interrupt(thread):
+    """
+    Check that, if thread is resumed, we go back to the same thread when the
+    program gets interrupted.
+    """
+
+    # Switch to the thread we're going to be running the test in.
+    print("thread ", thread.num)
+    gdb.execute("thr %d" % thread.num)
+
+    # Enter the loop() function on this thread.
+    #
+    # While there are cleaner ways to do this, we want to minimize the number of
+    # side effects on the gdbstub's internal state, since those may mask bugs.
+    # Ideally, there should be no difference between what we're doing here and
+    # the program reaching the loop() function on its own.
+    #
+    # For this to be safe, we only need the prologue of loop() to not have
+    # instructions that may have problems with what we're doing here. We don't
+    # have to worry about anything else, as this function never returns.
+    gdb.execute("set $pc = loop")
+
+    # Continue and then interrupt the task.
+    gdb.post_event(lambda: gdb.execute("interrupt"))
+    gdb.execute("c")
+
+    # Check whether the thread we're in after the interruption is the same we
+    # ran continue from.
+    return (thread.num == gdb.selected_thread().num)
+
+def run_test():
+    """
+    Test if interrupting the code always lands us on the same thread when
+    running with scheduler-lock enabled.
+    """
+
+    gdb.execute("set scheduler-locking on")
+    for thread in gdb.selected_inferior().threads():
+        report(
+            check_interrupt(thread), 
+            "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:
+	# These are not very useful in scripts
+	gdb.execute("set pagination off")
+
+	# 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)
diff --git a/tests/tcg/multiarch/system/Makefile.softmmu-target b/tests/tcg/multiarch/system/Makefile.softmmu-target
index fe40195d39..ad3b727ffc 100644
--- a/tests/tcg/multiarch/system/Makefile.softmmu-target
+++ b/tests/tcg/multiarch/system/Makefile.softmmu-target
@@ -27,7 +27,15 @@ run-gdbstub-memory: memory
 		"-monitor none -display none -chardev file$(COMMA)path=$<.out$(COMMA)id=output $(QEMU_OPTS)" \
 		--bin $< --test $(MULTIARCH_SRC)/gdbstub/memory.py, \
 	softmmu gdbstub support)
-
+run-gdbstub-interrupt: interrupt
+	$(call run-test, $@, $(GDB_SCRIPT) \
+		--gdb $(HAVE_GDB_BIN) \
+		--qemu $(QEMU) \
+		--output $<.gdb.out \
+		--qargs \
+		"-smp 2 -monitor none -display none -chardev file$(COMMA)path=$<.out$(COMMA)id=output $(QEMU_OPTS)" \
+		--bin $< --test $(MULTIARCH_SRC)/gdbstub/interrupt.py, \
+	softmmu gdbstub support)
 run-gdbstub-untimely-packet: hello
 	$(call run-test, $@, $(GDB_SCRIPT) \
 		--gdb $(HAVE_GDB_BIN) \
@@ -50,4 +58,4 @@ run-gdbstub-%:
 	$(call skip-test, "gdbstub test $*", "need working gdb")
 endif
 
-MULTIARCH_RUNS += run-gdbstub-memory run-gdbstub-untimely-packet
+MULTIARCH_RUNS += run-gdbstub-memory run-gdbstub-interrupt run-gdbstub-untimely-packet
diff --git a/tests/tcg/multiarch/system/interrupt.c b/tests/tcg/multiarch/system/interrupt.c
new file mode 100644
index 0000000000..64899caf09
--- /dev/null
+++ b/tests/tcg/multiarch/system/interrupt.c
@@ -0,0 +1,25 @@
+/*
+ * External interruption test. This test is structured in such a way that it
+ * passes the cases that require it to exit, but we can make it enter an
+ * infinite loop from GDB.
+ *
+ * We don't have the benefit of libc, just builtin C primitives and
+ * whatever is in minilib.
+ */
+
+#include <minilib.h>
+
+void loop(void)
+{
+    while(1)
+        /* Loop forever. Just make sure the condition is always a constant
+         * expression, so that this loop is not UB, as per the C standard. */
+        ;
+}
+
+int main(void)
+{
+    return 0;
+}
+
+
-- 
2.41.0


Re: [PATCH v2] gdbstub: fixes cases where wrong threads were reported to GDB on SIGINT
Posted by Alex Bennée 9 months, 1 week ago
Matheus Branco Borella <dark.ryu.550@gmail.com> writes:

> Alex Bennée <alex.bennee@linaro.org> writes:
>> Can gdb switch which packet sequence it uses to halt and restart
>> threads?
>
> Yes, but the way it does it does not trigger the behavior I was concerned 
> about. GDB falls back to the old sequence when either (1) the target does not
> support the vCont command it's trying to send or (2) you step backwards. In both
> cases, though, whenever it does fall back, it will first send an Hc packet 
> before continuing or stepping, which means we won't ever see a sequence such as
> ["Hc", "vCont;c:*", "c"]. This means, in short, that, while the shortcoming does
> exist in the code, GDB never actually triggers it.
>
>> The test I would like see is pretty much your test case
>> 
>>  - load a multi-threaded program
>>  - wait until threads running
>>  - pause
>>  - resume thread
>>  - check resumed thread was the right one
>
> What I have here should be pretty much that. 
>
> Is there something else you think I'm missing?
>
> ---
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1725
>
> This fix is implemented by having the vCont handler set the value of
> `gdbserver_state.c_cpu` if any threads are to be resumed. The specific CPU
> picked is arbitrarily from the ones to be resumed, but it should be okay, as all
> GDB cares about is that it is a resumed thread.
>
> Signed-off-by: Matheus Branco Borella <dark.ryu.550@gmail.com>

Arg the commit message is in the --- discard section.

Queued to for-8.1/misc-fixes, thanks.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro
[PATCH] gdbstub: fixes cases where wrong threads were reported to GDB on SIGINT
Posted by Matheus Branco Borella 10 months, 2 weeks ago
Alex Bennée <alex.bennee@linaro.org> writes:
> Can gdb switch which packet sequence it uses to halt and restart
> threads?

Yes, but the way it does it does not trigger the behavior I was concerned 
about. GDB falls back to the old sequence when either (1) the target does not
support the vCont command it's trying to send or (2) you step backwards. In both
cases, though, whenever it does fall back, it will first send an Hc packet 
before continuing or stepping, which means we won't ever see a sequence such as
["Hc", "vCont;c:*", "c"]. This means, in short, that, while the shortcoming does
exist in the code, GDB never actually triggers it.

> The test I would like see is pretty much your test case
> 
>  - load a multi-threaded program
>  - wait until threads running
>  - pause
>  - resume thread
>  - check resumed thread was the right one

What I have here should be pretty much that. 

Is there something else you think I'm missing?

---
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1725

This fix is implemented by having the vCont handler set the value of
`gdbserver_state.c_cpu` if any threads are to be resumed. The specific CPU
picked is arbitrarily from the ones to be resumed, but it should be okay, as all
GDB cares about is that it is a resumed thread.

Signed-off-by: Matheus Branco Borella <dark.ryu.550@gmail.com>
---
 gdbstub/gdbstub.c                             | 29 ++++++
 tests/tcg/multiarch/gdbstub/interrupt.py      | 99 +++++++++++++++++++
 .../multiarch/system/Makefile.softmmu-target  | 12 ++-
 tests/tcg/multiarch/system/interrupt.c        | 25 +++++
 4 files changed, 163 insertions(+), 2 deletions(-)
 create mode 100644 tests/tcg/multiarch/gdbstub/interrupt.py
 create mode 100644 tests/tcg/multiarch/system/interrupt.c

diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 6911b73c07..6a5a400348 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -597,6 +597,15 @@ static int gdb_handle_vcont(const char *p)
      *  or incorrect parameters passed.
      */
     res = 0;
+
+    /* 
+     * target_count and last_target keep track of how many CPUs we are going to
+     * step or resume, and a pointer to the state structure of one of them, 
+     * respectivelly
+     */
+    int target_count = 0;
+    CPUState *last_target = NULL;
+
     while (*p) {
         if (*p++ != ';') {
             return -ENOTSUP;
@@ -637,6 +646,9 @@ static int gdb_handle_vcont(const char *p)
             while (cpu) {
                 if (newstates[cpu->cpu_index] == 1) {
                     newstates[cpu->cpu_index] = cur_action;
+
+                    target_count++;
+                    last_target = cpu;
                 }
 
                 cpu = gdb_next_attached_cpu(cpu);
@@ -654,6 +666,9 @@ static int gdb_handle_vcont(const char *p)
             while (cpu) {
                 if (newstates[cpu->cpu_index] == 1) {
                     newstates[cpu->cpu_index] = cur_action;
+
+                    target_count++;
+                    last_target = cpu;
                 }
 
                 cpu = gdb_next_cpu_in_process(cpu);
@@ -671,11 +686,25 @@ static int gdb_handle_vcont(const char *p)
             /* only use if no previous match occourred */
             if (newstates[cpu->cpu_index] == 1) {
                 newstates[cpu->cpu_index] = cur_action;
+
+                target_count++;
+                last_target = cpu;
             }
             break;
         }
     }
 
+    /* 
+     * if we're about to resume a specific set of CPUs/threads, make it so that 
+     * in case execution gets interrupted, we can send GDB a stop reply with a
+     * correct value. it doesn't really matter which CPU we tell GDB the signal 
+     * happened in (VM pauses stop all of them anyway), so long as it is one of
+     * the ones we resumed/single stepped here.
+     */
+    if (target_count > 0) {
+        gdbserver_state.c_cpu = last_target;
+    }
+
     gdbserver_state.signal = signal;
     gdb_continue_partial(newstates);
     return res;
diff --git a/tests/tcg/multiarch/gdbstub/interrupt.py b/tests/tcg/multiarch/gdbstub/interrupt.py
new file mode 100644
index 0000000000..042d6fede9
--- /dev/null
+++ b/tests/tcg/multiarch/gdbstub/interrupt.py
@@ -0,0 +1,99 @@
+from __future__ import print_function
+#
+# This is launched via tests/guest-debug/run-test.py
+#
+
+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
+
+def check_interrupt(thread):
+    """
+    Check that, if thread is resumed, we go back to the same thread when the
+    program gets interrupted.
+    """
+
+    # Switch to the thread we're going to be running the test in.
+    print("thread ", thread.num)
+    gdb.execute("thr %d" % thread.num)
+
+    # Enter the loop() function on this thread.
+    #
+    # While there are cleaner ways to do this, we want to minimize the number of
+    # side effects on the gdbstub's internal state, since those may mask bugs. 
+    # Ideally, there should be no difference between what we're doing here and
+    # the program reaching the loop() function on its own.
+    #
+    # For this to be safe, we only need the prologue of loop() to not have
+    # instructions that may have problems with what we're doing here. We don't
+    # have to worry about anything else, as this function never returns.
+    gdb.execute("set $pc = loop")
+
+    # Continue and then interrupt the task.
+    gdb.post_event(lambda: gdb.execute("interrupt"))
+    gdb.execute("c")
+
+    # Check whether the thread we're in after the interruption is the same we 
+    # ran continue from.
+    return (thread.num == gdb.selected_thread().num)
+
+def run_test():
+    """
+    Test if interrupting the code always lands us on the same thread when
+    running with scheduler-lock enabled.
+    """
+
+    gdb.execute("set scheduler-locking on")
+    for thread in gdb.selected_inferior().threads():
+        report(
+            check_interrupt(thread), 
+            "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:
+	# These are not very useful in scripts
+	gdb.execute("set pagination off")
+
+	# 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)
diff --git a/tests/tcg/multiarch/system/Makefile.softmmu-target b/tests/tcg/multiarch/system/Makefile.softmmu-target
index fe40195d39..ad3b727ffc 100644
--- a/tests/tcg/multiarch/system/Makefile.softmmu-target
+++ b/tests/tcg/multiarch/system/Makefile.softmmu-target
@@ -27,7 +27,15 @@ run-gdbstub-memory: memory
 		"-monitor none -display none -chardev file$(COMMA)path=$<.out$(COMMA)id=output $(QEMU_OPTS)" \
 		--bin $< --test $(MULTIARCH_SRC)/gdbstub/memory.py, \
 	softmmu gdbstub support)
-
+run-gdbstub-interrupt: interrupt
+	$(call run-test, $@, $(GDB_SCRIPT) \
+		--gdb $(HAVE_GDB_BIN) \
+		--qemu $(QEMU) \
+		--output $<.gdb.out \
+		--qargs \
+		"-smp 2 -monitor none -display none -chardev file$(COMMA)path=$<.out$(COMMA)id=output $(QEMU_OPTS)" \
+		--bin $< --test $(MULTIARCH_SRC)/gdbstub/interrupt.py, \
+	softmmu gdbstub support)
 run-gdbstub-untimely-packet: hello
 	$(call run-test, $@, $(GDB_SCRIPT) \
 		--gdb $(HAVE_GDB_BIN) \
@@ -50,4 +58,4 @@ run-gdbstub-%:
 	$(call skip-test, "gdbstub test $*", "need working gdb")
 endif
 
-MULTIARCH_RUNS += run-gdbstub-memory run-gdbstub-untimely-packet
+MULTIARCH_RUNS += run-gdbstub-memory run-gdbstub-interrupt run-gdbstub-untimely-packet
diff --git a/tests/tcg/multiarch/system/interrupt.c b/tests/tcg/multiarch/system/interrupt.c
new file mode 100644
index 0000000000..19fd831a5c
--- /dev/null
+++ b/tests/tcg/multiarch/system/interrupt.c
@@ -0,0 +1,25 @@
+/*
+ * External interruption test. This test is structured in such a way that it
+ * passes the cases that require it to exit, but we can make it enter an 
+ * infinite loop from GDB.
+ *
+ * We don't have the benefit of libc, just builtin C primitives and
+ * whatever is in minilib.
+ */
+
+#include <minilib.h>
+
+void loop(void)
+{
+    while(1)
+        /* Loop forever. Just make sure the condition is always a constant 
+         * expression, so that this loop is not UB, as per the C standard. */
+        ;
+}
+
+int main(void)
+{
+    return 0;
+}
+
+
-- 
2.40.1