qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Matheus Branco Borella <dark.ryu.550@gmail.com>
To: qemu-devel@nongnu.org
Cc: alex.bennee@linaro.org, Matheus Branco Borella <dark.ryu.550@gmail.com>
Subject: [PATCH] gdbstub: fixes cases where wrong threads were reported to GDB on SIGINT
Date: Fri, 23 Jun 2023 15:12:57 -0300	[thread overview]
Message-ID: <20230623181256.2596-1-dark.ryu.550@gmail.com> (raw)

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	other threads:[~2023-06-23 20:30 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-23 18:12 Matheus Branco Borella [this message]
2023-06-27 10:39 ` [PATCH] gdbstub: fixes cases where wrong threads were reported to GDB on SIGINT Alex Bennée
2023-07-06 23:50   ` Matheus Branco Borella
2023-08-04 18:26   ` [PATCH v2] " Matheus Branco Borella
2023-08-10 17:30     ` Alex Bennée

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230623181256.2596-1-dark.ryu.550@gmail.com \
    --to=dark.ryu.550@gmail.com \
    --cc=alex.bennee@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).