qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Matheus Branco Borella
Subject: [PATCH] gdbstub: fixes cases where wrong threads were reported to GDB on SIGINT
Date: Fri, 23 Jun 2023 15:12:57 -0300

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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]