| Message ID | 20240906225451.1039718-2-roqueh@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix for multi-process gdbstub breakpoints | expand |
Roque Arcudia Hernandez <roqueh@google.com> writes: > 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. Can we better comment: CPUState *c_cpu; /* current CPU for step/continue ops */ CPUState *g_cpu; /* current CPU for other ops */ in GDBState? > > 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. */ > > Google-Bug-Id: 355027002 > Signed-off-by: Roque Arcudia Hernandez <roqueh@google.com> > --- > gdbstub/gdbstub.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c > index d08568cea0..98574eba68 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);
Hi Roque, On 6/9/24 19:54, Roque Arcudia Hernandez wrote: > 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. */ > > Google-Bug-Id: 355027002 Where can we find more information on this bug ID? I tried various query in the Google public tracker but couldn't find anything. (i.e. https://issuetracker.google.com/issues?q=canonicalid:355027002) > Signed-off-by: Roque Arcudia Hernandez <roqueh@google.com> > --- > gdbstub/gdbstub.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-)
Hello Philippe, The Google-Bug-Id is an internal ID and should have been removed. The information in the bug was mostly transcribed in the cover letter of this patch series. Thanks Roque On Mon, Oct 7, 2024 at 1:06 PM Philippe Mathieu-Daudé <philmd@linaro.org> wrote: > > Hi Roque, > > On 6/9/24 19:54, Roque Arcudia Hernandez wrote: > > 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. */ > > > > Google-Bug-Id: 355027002 > > Where can we find more information on this bug ID? > I tried various query in the Google public tracker but > couldn't find anything. > (i.e. https://issuetracker.google.com/issues?q=canonicalid:355027002) > > > Signed-off-by: Roque Arcudia Hernandez <roqueh@google.com> > > --- > > gdbstub/gdbstub.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) >
I'm adding extra documentation to those fields in a new patchset. On Fri, Oct 4, 2024 at 1:46 PM Alex Bennée <alex.bennee@linaro.org> wrote: > > Roque Arcudia Hernandez <roqueh@google.com> writes: > > > 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. > > Can we better comment: > > CPUState *c_cpu; /* current CPU for step/continue ops */ > CPUState *g_cpu; /* current CPU for other ops */ > > in GDBState? > > > > > 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. */ > > > > Google-Bug-Id: 355027002 > > Signed-off-by: Roque Arcudia Hernandez <roqueh@google.com> > > --- > > gdbstub/gdbstub.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c > > index d08568cea0..98574eba68 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); > > -- > Alex Bennée > Virtualisation Tech Lead @ Linaro
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c index d08568cea0..98574eba68 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);