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
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);
-- 
2.46.0.469.g59c65b2a67-goog

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(-)
>

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

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