From: Roque Arcudia Hernandez <roq...@google.com>
In the context of using the remote gdb with multiple
processes/inferiors (multi cluster machine) a given breakpoint will
target an specific inferior. Current implementation of
tcg_insert_breakpoint and tcg_remove_breakpoint apply a given
breakpoint to all the cpus available in the system.
This is not how gdb expects the remote protocol to behave. If we
refer to the current source of gdb, in the function
remote_target::insert_breakpoint in the file gdb/remote.c we can see
the intention to have the right process selected before breakpoint
insertion:
int
remote_target::insert_breakpoint (struct gdbarch *gdbarch,
struct bp_target_info *bp_tgt)
{
...
/* Make sure the remote is pointing at the right process, if
necessary. */
if (!gdbarch_has_global_breakpoints (current_inferior ()->arch ()))
set_general_process ();
...
}
Since most platforms do not have global breakpoints, this typically
will result in an 'Hg' Packet sent before 'z/Z', if gdb is not
pointing to the desired process.
For instance this is a concrete example obtained with a trace dump:
gdbstub_io_command Received: Hgp2.2
gdbstub_io_command Received: Z0,4000ded0,4
Only the CPUs associated with the selected process ID should insert
or remove the breakpoint. It is important to apply it to all the CPUs
in the process ID regardless of the particular thread selected by the
'Hg' packet because even in the case of a thread specific breakpoint.
A breakpoint on a specific thread is treated as a conditional break
similar to a 'break if'. This can be read in the code and comments of
function bpstat_check_breakpoint_conditions in the file
gdb/breakpoint.c
/* For breakpoints that are currently marked as telling gdb to stop,
check conditions (condition proper, frame, thread and ignore count)
of breakpoint referred to by BS. If we should not stop for this
breakpoint, set BS->stop to 0. */
static void
bpstat_check_breakpoint_conditions (bpstat *bs, thread_info *thread)
The patch needs to expose the currently private function
gdb_get_cpu_pid to the TCG and also expose the value of
gdbserver_state.multiprocess. The PID filtering will only be
applicable to multiprocess gdb because the PIDs are only defined in
that context.
Signed-off-by: Roque Arcudia Hernandez <roq...@google.com>
Signed-off-by: Nabih Estefan <nabiheste...@google.com>
---
accel/tcg/tcg-accel-ops.c | 37 +++++++++++++++++++++++--------------
gdbstub/gdbstub.c | 6 ++++++
include/exec/gdbstub.h | 12 ++++++++++++
3 files changed, 41 insertions(+), 14 deletions(-)
diff --git a/accel/tcg/tcg-accel-ops.c b/accel/tcg/tcg-accel-ops.c
index b24d6a7562..aca476cdf5 100644
--- a/accel/tcg/tcg-accel-ops.c
+++ b/accel/tcg/tcg-accel-ops.c
@@ -34,6 +34,7 @@
#include "qemu/guest-random.h"
#include "qemu/timer.h"
#include "exec/cputlb.h"
+#include "exec/gdbstub.h"
#include "exec/hwaddr.h"
#include "exec/tb-flush.h"
#include "exec/translation-block.h"
@@ -139,9 +140,11 @@ static int tcg_insert_breakpoint(CPUState *cs, int type,
vaddr addr, vaddr len)
case GDB_BREAKPOINT_SW:
case GDB_BREAKPOINT_HW:
CPU_FOREACH(cpu) {
- err = cpu_breakpoint_insert(cpu, addr, BP_GDB, NULL);
- if (err) {
- break;
+ if (gdb_cpu_in_source_group(cs, cpu)) {
+ err = cpu_breakpoint_insert(cpu, addr, BP_GDB, NULL);
+ if (err) {
+ break;
+ }
}
}
return err;
@@ -149,10 +152,12 @@ static int tcg_insert_breakpoint(CPUState *cs, int type,
vaddr addr, vaddr len)
case GDB_WATCHPOINT_READ:
case GDB_WATCHPOINT_ACCESS:
CPU_FOREACH(cpu) {
- err = cpu_watchpoint_insert(cpu, addr, len,
- xlat_gdb_type(cpu, type), NULL);
- if (err) {
- break;
+ if (gdb_cpu_in_source_group(cs, cpu)) {
+ err = cpu_watchpoint_insert(cpu, addr, len,
+ xlat_gdb_type(cpu, type), NULL);
+ if (err) {
+ break;
+ }
}
}
return err;
@@ -170,9 +175,11 @@ static int tcg_remove_breakpoint(CPUState *cs, int type,
vaddr addr, vaddr len)
case GDB_BREAKPOINT_SW:
case GDB_BREAKPOINT_HW:
CPU_FOREACH(cpu) {
- err = cpu_breakpoint_remove(cpu, addr, BP_GDB);
- if (err) {
- break;
+ if (gdb_cpu_in_source_group(cs, cpu)) {
+ err = cpu_breakpoint_remove(cpu, addr, BP_GDB);
+ if (err) {
+ break;
+ }
}
}
return err;
@@ -180,10 +187,12 @@ static int tcg_remove_breakpoint(CPUState *cs, int type,
vaddr addr, vaddr len)
case GDB_WATCHPOINT_READ:
case GDB_WATCHPOINT_ACCESS:
CPU_FOREACH(cpu) {
- err = cpu_watchpoint_remove(cpu, addr, len,
- xlat_gdb_type(cpu, type));
- if (err) {
- break;
+ if (gdb_cpu_in_source_group(cs, cpu)) {
+ err = cpu_watchpoint_remove(cpu, addr, len,
+ xlat_gdb_type(cpu, type));
+ if (err) {
+ break;
+ }
}
}
return err;
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 0e2e10fbaa..652e0e768f 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -492,6 +492,12 @@ const GDBFeature *gdb_find_static_feature(const char
*xmlname)
g_assert_not_reached();
}
+bool gdb_cpu_in_source_group(CPUState *cs, CPUState *cpu)
+{
+ return !gdbserver_state.multiprocess ||
+ (gdb_get_cpu_pid(cs) == gdb_get_cpu_pid(cpu));
+}
+
GArray *gdb_get_register_list(CPUState *cpu)
{
GArray *results = g_array_new(true, true, sizeof(GDBRegDesc));
diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index 0675b0b646..f92641793c 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -114,6 +114,18 @@ void gdb_feature_builder_end(const GDBFeatureBuilder
*builder);
*/
const GDBFeature *gdb_find_static_feature(const char *xmlname);
+/**
+ * Tests if the CPUs belong to the same group for the purposes of breakpoint
+ * insertion and deletion when running multiprocesses gdb. The test is only
+ * valid for multiprocess gdb and should not affect the insertion or deletion
of
+ * breakpoints when we are not running in that mode.
+ * @cs: The CPU used as reference
+ * @cpu: The CPU to test
+ *
+ * Return: True if they belong to the same group or it is not a multiprocess
gdb
+ */
+bool gdb_cpu_in_source_group(CPUState *cs, CPUState *cpu);
+
/**
* gdb_read_register() - Read a register associated with a CPU.
* @cpu: The CPU associated with the register.
--
2.49.0.1015.ga840276032-goog
