[v2,1/2] gdbstub: Fix wrong CPUState pointer in breakpoint functions

Commit Message

Roque Arcudia Hernandez Oct. 17, 2024, 3:37 p.m. UTC

In the context of using the remote gdb with multiple
processes/inferiors (multiple cluster machine) a given breakpoint
will target an specific inferior. If needed the remote protocol will
use the packet 'H op thread-id' with op = 'g' to change focus to the
inferior we want to insert/remove the breakpoint to, for instance
'Hgp3.3' and not 'Hcp3.3'.

This is supported by the documentation of the H packets:

 > 'H op thread-id'
 > Set thread for subsequent operations ('m', 'M', 'g', 'G',
 > et.al.). Depending on the operation to be performed, op should be
 > 'c' for step and continue operations (note that this is
 > deprecated, supporting the 'vCont' command is a better option),
 > and 'g' for other operations.

This can also be verified in the GDB source code file gdb/remote.c.
Functions remote_target::insert_breakpoint and
remote_target::remove_breakpoint will eventually call
remote_target::set_general_thread if it needs to change the process
focus and not remote_target::set_continue_thread.

This can be seen around a comment that says:

      /* Make sure the remote is pointing at the right process, if
         necessary.  */

Signed-off-by: Roque Arcudia Hernandez <[e-mail]>
---
 gdbstub/gdbstub.c   |  4 ++--
 gdbstub/internals.h | 13 +++++++++++--
 2 files changed, 13 insertions(+), 4 deletions(-)

Patch

diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index b1def7e71d..64ad81e53f 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -1148,7 +1148,7 @@  static void handle_insert_bp(GArray *params, void *user_ctx)
         return;
     }
 
-    res = gdb_breakpoint_insert(gdbserver_state.c_cpu,
+    res = gdb_breakpoint_insert(gdbserver_state.g_cpu,
                                 gdb_get_cmd_param(params, 0)->val_ul,
                                 gdb_get_cmd_param(params, 1)->val_ull,
                                 gdb_get_cmd_param(params, 2)->val_ull);
@@ -1172,7 +1172,7 @@  static void handle_remove_bp(GArray *params, void *user_ctx)
         return;
     }
 
-    res = gdb_breakpoint_remove(gdbserver_state.c_cpu,
+    res = gdb_breakpoint_remove(gdbserver_state.g_cpu,
                                 gdb_get_cmd_param(params, 0)->val_ul,
                                 gdb_get_cmd_param(params, 1)->val_ull,
                                 gdb_get_cmd_param(params, 2)->val_ull);
diff --git a/gdbstub/internals.h b/gdbstub/internals.h
index bf5a5c6302..e67fb6fe37 100644
--- a/gdbstub/internals.h
+++ b/gdbstub/internals.h
@@ -48,8 +48,17 @@  enum RSState {
 
 typedef struct GDBState {
     bool init;       /* have we been initialised? */
-    CPUState *c_cpu; /* current CPU for step/continue ops */
-    CPUState *g_cpu; /* current CPU for other ops */
+    /*
+     * Current CPU for step/continue ops. Updated by the remote packet
+     * 'Hc thread-id'
+     */
+    CPUState *c_cpu;
+    /*
+     * Current CPU for other ops such as memory accesses ('m'/'M'), general
+     * register accesses ('g'/'G'), breakpoint management ('z'/'Z'), etc.
+     * Updated by the remote packet 'Hg thread-id'
+     */
+    CPUState *g_cpu;
     CPUState *query_cpu; /* for q{f|s}ThreadInfo */
     enum RSState state; /* parsing state */
     char line_buf[MAX_PACKET_LENGTH];