Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
gdbstub: fixes cases where wrong threads were reported to GDB on SIGINT
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>
Message-Id: <20230804182633.47300-2-dark.ryu.550@gmail.com>
[AJB: style and whitespace fixes]
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1725
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20230829161528.2707696-9-alex.bennee@linaro.org>
  • Loading branch information
mbrla0 authored and stsquad committed Aug 30, 2023
1 parent a8fea70 commit 761e3c1
Show file tree
Hide file tree
Showing 4 changed files with 164 additions and 2 deletions.
29 changes: 29 additions & 0 deletions gdbstub/gdbstub.c
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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;
Expand Down
97 changes: 97 additions & 0 deletions tests/tcg/multiarch/gdbstub/interrupt.py
@@ -0,0 +1,97 @@
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:
# 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)
12 changes: 10 additions & 2 deletions tests/tcg/multiarch/system/Makefile.softmmu-target
Expand Up @@ -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) \
Expand All @@ -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
28 changes: 28 additions & 0 deletions tests/tcg/multiarch/system/interrupt.c
@@ -0,0 +1,28 @@
/*
* 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)
{
do {
/*
* Loop forever. Just make sure the condition is always a constant
* expression, so that this loop is not UB, as per the C
* standard.
*/
} while (1);
}

int main(void)
{
return 0;
}


0 comments on commit 761e3c1

Please sign in to comment.