* [PATCH v3 5/5] Support software breakpoints for ARM linux in GDBServer.
2015-10-20 16:48 [PATCH v3 0/5] Software breakpoints support for ARM linux in GDBServer Antoine Tremblay
@ 2015-10-20 16:48 ` Antoine Tremblay
2015-10-21 13:12 ` Pedro Alves
2015-10-20 16:48 ` [PATCH v3 1/5] Add the target_ops needed for software breakpoints " Antoine Tremblay
` (4 subsequent siblings)
5 siblings, 1 reply; 20+ messages in thread
From: Antoine Tremblay @ 2015-10-20 16:48 UTC (permalink / raw)
To: gdb-patches; +Cc: Antoine Tremblay
In this v3:
* Removed NEWS entry
-
This patch enables software breakpoints via GDB's Z0 packets on ARM.
No regressions, tested on ubuntu 14.04 ARMv7 and x86.
With gdbserver-{native,extended} / { -marm -mthumb }
gdb/gdbserver/ChangeLog:
* linux-arm-low.c (arm_supports_z_point_type): Add software
breakpoint support.
---
gdb/gdbserver/linux-arm-low.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/gdb/gdbserver/linux-arm-low.c b/gdb/gdbserver/linux-arm-low.c
index 62e88f0..0f88b87 100644
--- a/gdb/gdbserver/linux-arm-low.c
+++ b/gdb/gdbserver/linux-arm-low.c
@@ -545,6 +545,7 @@ arm_supports_z_point_type (char z_type)
{
switch (z_type)
{
+ case Z_PACKET_SW_BP:
case Z_PACKET_HW_BP:
case Z_PACKET_WRITE_WP:
case Z_PACKET_READ_WP:
--
1.9.1
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v3 0/5] Software breakpoints support for ARM linux in GDBServer.
@ 2015-10-20 16:48 Antoine Tremblay
2015-10-20 16:48 ` [PATCH v3 5/5] Support software breakpoints " Antoine Tremblay
` (5 more replies)
0 siblings, 6 replies; 20+ messages in thread
From: Antoine Tremblay @ 2015-10-20 16:48 UTC (permalink / raw)
To: gdb-patches
In this v3 :
The main change is that the functions breakpoint_from_pc and
breakpoint_from_kind are now called : breakpoint_kind_from_pc and
sw_breakpoint_from_kind.
As suggested in : https://sourceware.org/ml/gdb-patches/2015-10/msg00287.html
Thank you Pedro the patch set is really more clear now.
Thus the patchset is restructured for that, patches have been renamed and
reordered.
Patches that have changed have their own v3 comment.
Original comment is updated :
-----
This patch series adds software breakpoint support for ARM on linux in
GDBServer.
This is a subset of a previous patchset :
https://sourceware.org/ml/gdb-patches/2015-09/msg00221.html
Other patches in that previous patchset will be sent after.
The problematic with ARM software breakpoints is that it can have 3 different
breakpoints one for each instruction set : arm, thumb, thumb2.
So we need to be able to set different kinds of breakpoints when GDBServer sets
its own breakpoints or when a breakpoint is set using GDB's Z0 packet.
In order to allow that the patches :
[PATCH v3 1/5] Add the target_ops needed for software breakpoints in GDBServer.
Adds new target operations to replace the static breakpoint and breakpoint_len
operation so that the target can return the proper GDBServer breakpoint.
[PATCH v3 2/5] Support breakpoint kinds for software breakpoints in GDBServer.
This is the main patch of the set, it handles GDBServer's software
breakpoints by calling the breakpoint_kind_from_pc operation and
sw_breakpoint_from_kind.
[PATCH v3 3/5] Implement breakpoint_kind_from_pc and sw_breakpoint_from_kind for
ARM in GDBServer.
This patch adds the ARM implementation of these target operation, selecting the
thumb, thumb2, or arm breakpoint based on the PC address and the flags encoded
in it.
[PATCH v3 4/5] Refactor the breakpoint definitions in linux-arm-low.c.
This patch refactors the breakpoint definitions a bit to be more clear
.
[PATCH v3 5/5] Support software breakpoints for ARM linux in GDBServer.
And finally software breakpoints via Z0 packets are enabled for ARM.
This patchset has no regressions observed, tested on ubuntu 14.04 ARMv7 and x86.
With gdbserver-{native,extended} / { -marm -mthumb }
Note also that while I could not test directly thumbv1 instructions with gcc
-marmv4t , manual testing of the software breakpoints was done for thumbv1
instructions.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v3 1/5] Add the target_ops needed for software breakpoints in GDBServer.
2015-10-20 16:48 [PATCH v3 0/5] Software breakpoints support for ARM linux in GDBServer Antoine Tremblay
2015-10-20 16:48 ` [PATCH v3 5/5] Support software breakpoints " Antoine Tremblay
@ 2015-10-20 16:48 ` Antoine Tremblay
2015-10-21 11:22 ` Pedro Alves
2015-10-20 16:49 ` [PATCH v3 2/5] Support breakpoint kinds " Antoine Tremblay
` (3 subsequent siblings)
5 siblings, 1 reply; 20+ messages in thread
From: Antoine Tremblay @ 2015-10-20 16:48 UTC (permalink / raw)
To: gdb-patches; +Cc: Antoine Tremblay
In this v3 :
* use gdb_byte.
* Fixed missing comments.
* Fixed style issues.
* Fixed casts.
* Use new target operations as per Pedro's comments.
-
This patch is in preparation for software breakpoints on ARM linux. It
refactors breakpoint and breakpoint_len into breakpoint_kind_from_pc and
sw_breakpoint_from kind to prepare the case where we have multiple types of
breakpoints.
Kind is the type of breakpoint (hardware or software) to be inserted, usually it
is the lenght of the software breakpoint but can be something else depending on
the target.
This patch introduces the linux_target_ops breakpoint_kind_from_pc and
sw_breakpoint_from_kind.
breakpoint_kind_from_pc returns the breakpoint kind and adjusts the PC to the
real memory location in case a flag was present in the PC. E.g the instruction
mode on ARM.
sw_breakpoint_from_kind returns the software breakpoint for this kind as a
string of bytes, the length of the breakpoint is adjusted for the breakpoint's
size in memory.
For targets that have only one kind of breakpoint, the default value 0 is
returned by linux_breakpoint_kind_from_pc so that not all targets need to
implement the breakpoint_kind_from_pc operation.
No regressions, tested on Ubuntu 14.04 on ARMv7 and x86
With gdbserver-{native,extended} / { -marm -mthumb }
Also since the target_ops have been changed compilation was tested on
affected archs namely : aarch64, arm, bfin, cris, crisv32, m32r,
m68k, mips, nios2, ppc, s390, sparc, tic6x, tile, x86, steins.
Not tested : sh
gdb/gdbserver/ChangeLog:
* linux-aarch64-low.c (aarch64_sw_breakpoint_from_kind): New function.
(struct linux_target_ops) <breakpoint>: Remove.
(struct linux_target_ops) <breakpoint_len>: Remove.
(struct linux_target_ops) <breakpoint_kind_from_pc>: Initialize field.
(struct linux_target_ops) <sw_breakpoint_from_kind>: Initialize field.
* linux-arm-low.c (arm_breakpoint_kind_from_pc): New function.
(arm_sw_breakpoint_from_kind): New function.
* linux-bfin-low.c (bfin_sw_breakpoint_from_kind): New function.
(struct linux_target_ops) <breakpoint>: Remove.
(struct linux_target_ops) <breakpoint_len>: Remove.
(struct linux_target_ops) <breakpoint_kind_from_pc>: Initialize field.
(struct linux_target_ops) <sw_breakpoint_from_kind>: Initialize field.
* linux-cris-low.c (cris_sw_breakpoint_from_kind): New function.
(struct linux_target_ops) <breakpoint>: Remove.
(struct linux_target_ops) <breakpoint_len>: Remove.
(struct linux_target_ops) <breakpoint_kind_from_pc>: Initialize field.
(struct linux_target_ops) <sw_breakpoint_from_kind>: Initialize field.
* linux-crisv32-low.c (cris_sw_breakpoint_from_kind): New function.
(struct linux_target_ops) <breakpoint>: Remove.
(struct linux_target_ops) <breakpoint_len>: Remove.
(struct linux_target_ops) <breakpoint_kind_from_pc>: Initialize field.
(struct linux_target_ops) <sw_breakpoint_from_kind>: Initialize field.
* linux-low.c (linux_wait_1): Call breakpoint_kind_from_pc
and sw_breakpoint_from_kind to increment the pc.
(linux_breakpoint_kind_from_pc): New function.
(linux_sw_breakpoint_from_kind): New function.
(struct target_ops) <sw_breakpoint_from_kind>: Initialize field.
(initialize_low): Call breakpoint_kind_from_pc and
sw_breakpoint_from_kind to replace breakpoint_data/len.
* linux-low.h (struct linux_target_ops) <breakpoint_kind_from_pc>:
New field.
(struct linux_target_ops) <sw_breakpoint_from_kind>: Likewise.
* linux-m32r-low.c (m32r_sw_breakpoint_from_kind): New function.
(struct linux_target_ops) <breakpoint>: Remove.
(struct linux_target_ops) <breakpoint_len>: Remove.
(struct linux_target_ops) <breakpoint_kind_from_pc>: Initialize field.
(struct linux_target_ops) <sw_breakpoint_from_kind>: Initialize field.
* linux-m68k-low.c (m68k_sw_breakpoint_from_kind): New function.
(struct linux_target_ops) <breakpoint>: Remove.
(struct linux_target_ops) <breakpoint_len>: Remove.
(struct linux_target_ops) <breakpoint_kind_from_pc>: Initialize field.
(struct linux_target_ops) <sw_breakpoint_from_kind>: Initialize field.
* linux-mips-low.c (mips_sw_breakpoint_from_kind): New function.
(struct linux_target_ops) <breakpoint>: Remove.
(struct linux_target_ops) <breakpoint_len>: Remove.
(struct linux_target_ops) <breakpoint_kind_from_pc>: Initialize field.
(struct linux_target_ops) <sw_breakpoint_from_kind>: Initialize field.
* linux-nios2-low.c (nios2_sw_breakpoint_from_kind): New function.
(struct linux_target_ops) <breakpoint>: Remove.
(struct linux_target_ops) <breakpoint_len>: Remove.
(struct linux_target_ops) <breakpoint_kind_from_pc>: Initialize field.
(struct linux_target_ops) <sw_breakpoint_from_kind>: Initialize field.
* linux-ppc-low.c (ppc_sw_breakpoint_from_kind): New function.
(struct linux_target_ops) <breakpoint>: Remove.
(struct linux_target_ops) <breakpoint_len>: Remove.
(struct linux_target_ops) <breakpoint_kind_from_pc>: Initialize field.
(struct linux_target_ops) <sw_breakpoint_from_kind>: Initialize field.
* linux-s390-low.c (s390_sw_breakpoint_from_kind): New function.
(struct linux_target_ops) <breakpoint>: Remove.
(struct linux_target_ops) <breakpoint_len>: Remove.
(struct linux_target_ops) <breakpoint_kind_from_pc>: Initialize field.
(struct linux_target_ops) <sw_breakpoint_from_kind>: Initialize field.
* linux-sh-low.c (sh_sw_breakpoint_from_kind): New function.
(struct linux_target_ops) <breakpoint>: Remove.
(struct linux_target_ops) <breakpoint_len>: Remove.
(struct linux_target_ops) <breakpoint_kind_from_pc>: Initialize field.
(struct linux_target_ops) <sw_breakpoint_from_kind>: Initialize field.
* linux-sparc-low.c (sparc_sw_breakpoint_from_kind): New function.
(struct linux_target_ops) <breakpoint>: Remove.
(struct linux_target_ops) <breakpoint_len>: Remove.
(struct linux_target_ops) <breakpoint_kind_from_pc>: Initialize field.
(struct linux_target_ops) <sw_breakpoint_from_kind>: Initialize field.
* linux-tic6x-low.c (tic6x_sw_breakpoint_from_kind): New function.
(struct linux_target_ops) <breakpoint>: Remove.
(struct linux_target_ops) <breakpoint_len>: Remove.
(struct linux_target_ops) <breakpoint_kind_from_pc>: Initialize field.
(struct linux_target_ops) <sw_breakpoint_from_kind>: Initialize field.
* linux-tile-low.c (tile_sw_breakpoint_from_kind): New function.
* linux-x86-low.c (x86_sw_breakpoint_from_kind): New function.
(struct linux_target_ops) <breakpoint>: Remove.
(struct linux_target_ops) <breakpoint_len>: Remove.
(struct linux_target_ops) <breakpoint_kind_from_pc>: Initialize field.
(struct linux_target_ops) <sw_breakpoint_from_kind>: Initialize field.
* linux-xtensa-low.c (xtensa_sw_breakpoint_from_kind) New function.
(struct linux_target_ops) <breakpoint>: Remove.
(struct linux_target_ops) <breakpoint_len>: Remove.
(struct linux_target_ops) <breakpoint_kind_from_pc>: Initialize field.
(struct linux_target_ops) <sw_breakpoint_from_kind>: Initialize field.
---
gdb/gdbserver/linux-aarch64-low.c | 13 +++++++++--
gdb/gdbserver/linux-arm-low.c | 30 ++++++++++++++++----------
gdb/gdbserver/linux-bfin-low.c | 15 ++++++++++---
gdb/gdbserver/linux-cris-low.c | 13 +++++++++--
gdb/gdbserver/linux-crisv32-low.c | 13 +++++++++--
gdb/gdbserver/linux-low.c | 45 ++++++++++++++++++++++++++++++++++++---
gdb/gdbserver/linux-low.h | 13 +++++++++--
gdb/gdbserver/linux-m32r-low.c | 13 +++++++++--
gdb/gdbserver/linux-m68k-low.c | 15 ++++++++++---
gdb/gdbserver/linux-mips-low.c | 13 +++++++++--
gdb/gdbserver/linux-nios2-low.c | 24 ++++++++++++++-------
gdb/gdbserver/linux-ppc-low.c | 13 +++++++++--
gdb/gdbserver/linux-s390-low.c | 15 ++++++++++---
gdb/gdbserver/linux-sh-low.c | 13 +++++++++--
gdb/gdbserver/linux-sparc-low.c | 14 +++++++++---
gdb/gdbserver/linux-tic6x-low.c | 16 ++++++++++----
gdb/gdbserver/linux-tile-low.c | 13 +++++++++--
gdb/gdbserver/linux-x86-low.c | 15 ++++++++++---
gdb/gdbserver/linux-xtensa-low.c | 15 ++++++++++---
gdb/gdbserver/target.h | 10 +++++++++
20 files changed, 269 insertions(+), 62 deletions(-)
diff --git a/gdb/gdbserver/linux-aarch64-low.c b/gdb/gdbserver/linux-aarch64-low.c
index 780c5e3..cb49a04 100644
--- a/gdb/gdbserver/linux-aarch64-low.c
+++ b/gdb/gdbserver/linux-aarch64-low.c
@@ -2931,6 +2931,15 @@ aarch64_supports_range_stepping (void)
return 1;
}
+/* Implementation of linux_target_ops method "sw_breakpoint_from_kind". */
+
+static const gdb_byte *
+aarch64_sw_breakpoint_from_kind (int kind, int *size)
+{
+ *size = aarch64_breakpoint_len;
+ return aarch64_breakpoint;
+}
+
struct linux_target_ops the_low_target =
{
aarch64_arch_setup,
@@ -2940,8 +2949,8 @@ struct linux_target_ops the_low_target =
NULL, /* fetch_register */
aarch64_get_pc,
aarch64_set_pc,
- (const unsigned char *) &aarch64_breakpoint,
- aarch64_breakpoint_len,
+ NULL, /* breakpoint_kind_from_pc */
+ aarch64_sw_breakpoint_from_kind,
NULL, /* breakpoint_reinsert_addr */
0, /* decr_pc_after_break */
aarch64_breakpoint_at,
diff --git a/gdb/gdbserver/linux-arm-low.c b/gdb/gdbserver/linux-arm-low.c
index a277bb6..3a56620 100644
--- a/gdb/gdbserver/linux-arm-low.c
+++ b/gdb/gdbserver/linux-arm-low.c
@@ -913,26 +913,34 @@ arm_regs_info (void)
return ®s_info_arm;
}
-struct linux_target_ops the_low_target = {
- arm_arch_setup,
- arm_regs_info,
- arm_cannot_fetch_register,
- arm_cannot_store_register,
- NULL, /* fetch_register */
- arm_get_pc,
- arm_set_pc,
+/* Implementation of linux_target_ops method "sw_breakpoint_from_kind". */
+static const gdb_byte *
+arm_sw_breakpoint_from_kind (int kind , int *size)
+{
+ *size = arm_breakpoint_len;
/* Define an ARM-mode breakpoint; we only set breakpoints in the C
library, which is most likely to be ARM. If the kernel supports
clone events, we will never insert a breakpoint, so even a Thumb
C library will work; so will mixing EABI/non-EABI gdbserver and
application. */
#ifndef __ARM_EABI__
- (const unsigned char *) &arm_breakpoint,
+ return (const gdb_byte *) &arm_breakpoint;
#else
- (const unsigned char *) &arm_eabi_breakpoint,
+ return (const gdb_byte *) &arm_eabi_breakpoint;
#endif
- arm_breakpoint_len,
+}
+
+struct linux_target_ops the_low_target = {
+ arm_arch_setup,
+ arm_regs_info,
+ arm_cannot_fetch_register,
+ arm_cannot_store_register,
+ NULL, /* fetch_register */
+ arm_get_pc,
+ arm_set_pc,
+ NULL, /* breakpoint_kind_from_pc */
+ arm_sw_breakpoint_from_kind,
arm_reinsert_addr,
0,
arm_breakpoint_at,
diff --git a/gdb/gdbserver/linux-bfin-low.c b/gdb/gdbserver/linux-bfin-low.c
index 4002f22..d3b83fc 100644
--- a/gdb/gdbserver/linux-bfin-low.c
+++ b/gdb/gdbserver/linux-bfin-low.c
@@ -73,7 +73,16 @@ bfin_set_pc (struct regcache *regcache, CORE_ADDR pc)
}
#define bfin_breakpoint_len 2
-static const unsigned char bfin_breakpoint[bfin_breakpoint_len] = {0xa1, 0x00};
+static const gdb_byte bfin_breakpoint[bfin_breakpoint_len] = {0xa1, 0x00};
+
+/* Implementation of linux_target_ops method "sw_breakpoint_from_kind". */
+
+static const gdb_byte *
+bfin_sw_breakpoint_from_kind (int kind, int *size)
+{
+ *size = bfin_breakpoint_len;
+ return bfin_breakpoint;
+}
static int
bfin_breakpoint_at (CORE_ADDR where)
@@ -122,8 +131,8 @@ struct linux_target_ops the_low_target = {
NULL, /* fetch_register */
bfin_get_pc,
bfin_set_pc,
- bfin_breakpoint,
- bfin_breakpoint_len,
+ NULL, /* breakpoint_kind_from_pc */
+ bfin_sw_breakpoint_from_kind,
NULL, /* breakpoint_reinsert_addr */
2,
bfin_breakpoint_at,
diff --git a/gdb/gdbserver/linux-cris-low.c b/gdb/gdbserver/linux-cris-low.c
index e0bfa1a..0265238 100644
--- a/gdb/gdbserver/linux-cris-low.c
+++ b/gdb/gdbserver/linux-cris-low.c
@@ -81,6 +81,15 @@ cris_set_pc (struct regcache *regcache, CORE_ADDR pc)
static const unsigned short cris_breakpoint = 0xe938;
#define cris_breakpoint_len 2
+/* Implementation of linux_target_ops method "sw_breakpoint_from_kind". */
+
+static const gdb_byte *
+cris_sw_breakpoint_from_kind (int kind, int *size)
+{
+ *size = cris_breakpoint_len;
+ return (const gdb_byte *) &cris_breakpoint;
+}
+
static int
cris_breakpoint_at (CORE_ADDR where)
{
@@ -140,8 +149,8 @@ struct linux_target_ops the_low_target = {
NULL, /* fetch_register */
cris_get_pc,
cris_set_pc,
- (const unsigned char *) &cris_breakpoint,
- cris_breakpoint_len,
+ NULL, /* breakpoint_kind_from_pc */
+ cris_sw_breakpoint_from_kind,
cris_reinsert_addr,
0,
cris_breakpoint_at,
diff --git a/gdb/gdbserver/linux-crisv32-low.c b/gdb/gdbserver/linux-crisv32-low.c
index 5120863..f97122e 100644
--- a/gdb/gdbserver/linux-crisv32-low.c
+++ b/gdb/gdbserver/linux-crisv32-low.c
@@ -77,6 +77,15 @@ cris_set_pc (struct regcache *regcache, CORE_ADDR pc)
static const unsigned short cris_breakpoint = 0xe938;
#define cris_breakpoint_len 2
+/* Implementation of linux_target_ops method "sw_breakpoint_from_kind". */
+
+static const gdb_byte *
+cris_sw_breakpoint_from_kind (int kind, int *size)
+{
+ *size = cris_breakpoint_len;
+ return (const gdb_byte *) &cris_breakpoint;
+}
+
static int
cris_breakpoint_at (CORE_ADDR where)
{
@@ -420,8 +429,8 @@ struct linux_target_ops the_low_target = {
NULL, /* fetch_register */
cris_get_pc,
cris_set_pc,
- (const unsigned char *) &cris_breakpoint,
- cris_breakpoint_len,
+ NULL, /* breakpoint_kind_from_pc */
+ cris_sw_breakpoint_from_kind,
cris_reinsert_addr,
0,
cris_breakpoint_at,
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 3a1a6ae..398c0aa 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -3012,7 +3012,12 @@ linux_wait_1 (ptid_t ptid,
if (!ptid_equal (step_over_bkpt, null_ptid)
&& event_child->stop_reason == TARGET_STOPPED_BY_SW_BREAKPOINT)
{
- unsigned int increment_pc = the_low_target.breakpoint_len;
+ int increment_pc = 0;
+ int breakpoint_kind = 0;
+ CORE_ADDR stop_pc = event_child->stop_pc;
+
+ breakpoint_kind = the_target->breakpoint_kind_from_pc (&stop_pc);
+ the_target->sw_breakpoint_from_kind (breakpoint_kind, &increment_pc);
if (debug_threads)
{
@@ -6932,6 +6937,28 @@ current_lwp_ptid (void)
return ptid_of (current_thread);
}
+/* Implementation of the target_ops method "breakpoint_kind_from_pc". */
+
+static int
+linux_breakpoint_kind_from_pc (CORE_ADDR *pcptr)
+{
+ if (the_low_target.breakpoint_kind_from_pc != NULL)
+ return (*the_low_target.breakpoint_kind_from_pc) (pcptr);
+ else
+ /* Default breakpoint kind value. */
+ return 0;
+}
+
+/* Implementation of the target_ops method "sw_breakpoint_from_kind". */
+
+static const gdb_byte *
+linux_sw_breakpoint_from_kind (int kind, int *size)
+{
+ gdb_assert (the_low_target.sw_breakpoint_from_kind != NULL);
+
+ return (*the_low_target.sw_breakpoint_from_kind) (kind, size);
+}
+
static struct target_ops linux_target_ops = {
linux_create_inferior,
linux_arch_setup,
@@ -7026,6 +7053,8 @@ static struct target_ops linux_target_ops = {
linux_mntns_open_cloexec,
linux_mntns_unlink,
linux_mntns_readlink,
+ linux_breakpoint_kind_from_pc,
+ linux_sw_breakpoint_from_kind
};
static void
@@ -7053,10 +7082,20 @@ void
initialize_low (void)
{
struct sigaction sigchld_action;
+ int breakpoint_kind = 0;
+ int breakpoint_size = 0;
+ const gdb_byte *breakpoint = NULL;
+
memset (&sigchld_action, 0, sizeof (sigchld_action));
set_target_ops (&linux_target_ops);
- set_breakpoint_data (the_low_target.breakpoint,
- the_low_target.breakpoint_len);
+
+ breakpoint_kind = the_target->breakpoint_kind_from_pc (NULL);
+ breakpoint = the_target->sw_breakpoint_from_kind (breakpoint_kind,
+ &breakpoint_size);
+
+ set_breakpoint_data (breakpoint,
+ breakpoint_size);
+
linux_init_signals ();
linux_ptrace_init_warnings ();
diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h
index f8f6e78..5ae09fa 100644
--- a/gdb/gdbserver/linux-low.h
+++ b/gdb/gdbserver/linux-low.h
@@ -141,8 +141,17 @@ struct linux_target_ops
CORE_ADDR (*get_pc) (struct regcache *regcache);
void (*set_pc) (struct regcache *regcache, CORE_ADDR newpc);
- const unsigned char *breakpoint;
- int breakpoint_len;
+
+ /* Return the breakpoint kind for this target based on PC. The PCPTR is
+ adjusted to the real memory location in case a flag (e.g., the Thumb bit on
+ ARM) was present in the PC. */
+ int (*breakpoint_kind_from_pc) (CORE_ADDR *pcptr);
+
+ /* Return the software breakpoint from KIND. KIND can have target
+ specific meaning like the z0 kind parameter.
+ SIZE is set to the software breakpoint's length in memory. */
+ const gdb_byte *(*sw_breakpoint_from_kind) (int kind, int *size);
+
CORE_ADDR (*breakpoint_reinsert_addr) (void);
int decr_pc_after_break;
diff --git a/gdb/gdbserver/linux-m32r-low.c b/gdb/gdbserver/linux-m32r-low.c
index 8ffeda2..bb1002f 100644
--- a/gdb/gdbserver/linux-m32r-low.c
+++ b/gdb/gdbserver/linux-m32r-low.c
@@ -73,6 +73,15 @@ m32r_set_pc (struct regcache *regcache, CORE_ADDR pc)
static const unsigned short m32r_breakpoint = 0x10f1;
#define m32r_breakpoint_len 2
+/* Implementation of linux_target_ops method "sw_breakpoint_from_kind". */
+
+static const gdb_byte *
+m32r_sw_breakpoint_from_kind (int kind, int *size)
+{
+ *size = m32r_breakpoint_len;
+ return (const gdb_byte *) &m32r_breakpoint;
+}
+
static int
m32r_breakpoint_at (CORE_ADDR where)
{
@@ -120,8 +129,8 @@ struct linux_target_ops the_low_target = {
NULL, /* fetch_register */
m32r_get_pc,
m32r_set_pc,
- (const unsigned char *) &m32r_breakpoint,
- m32r_breakpoint_len,
+ NULL, /* breakpoint_from_pc */
+ m32r_sw_breakpoint_from_kind,
NULL,
0,
m32r_breakpoint_at,
diff --git a/gdb/gdbserver/linux-m68k-low.c b/gdb/gdbserver/linux-m68k-low.c
index 39c9cc5..ba8e5e9 100644
--- a/gdb/gdbserver/linux-m68k-low.c
+++ b/gdb/gdbserver/linux-m68k-low.c
@@ -122,9 +122,18 @@ static struct regset_info m68k_regsets[] = {
{ 0, 0, 0, -1, -1, NULL, NULL }
};
-static const unsigned char m68k_breakpoint[] = { 0x4E, 0x4F };
+static const gdb_byte m68k_breakpoint[] = { 0x4E, 0x4F };
#define m68k_breakpoint_len 2
+/* Implementation of linux_target_ops method "sw_breakpoint_from_kind". */
+
+static const gdb_byte *
+m68k_sw_breakpoint_from_kind (int kind, int *size)
+{
+ *size = m68k_breakpoint_len;
+ return m68k_breakpoint;
+}
+
static CORE_ADDR
m68k_get_pc (struct regcache *regcache)
{
@@ -215,8 +224,8 @@ struct linux_target_ops the_low_target = {
NULL, /* fetch_register */
m68k_get_pc,
m68k_set_pc,
- m68k_breakpoint,
- m68k_breakpoint_len,
+ NULL, /* breakpoint_kind_from_pc */
+ m68k_sw_breakpoint_from_kind,
NULL,
2,
m68k_breakpoint_at,
diff --git a/gdb/gdbserver/linux-mips-low.c b/gdb/gdbserver/linux-mips-low.c
index d1181b6..344d7a5 100644
--- a/gdb/gdbserver/linux-mips-low.c
+++ b/gdb/gdbserver/linux-mips-low.c
@@ -266,6 +266,15 @@ mips_set_pc (struct regcache *regcache, CORE_ADDR pc)
static const unsigned int mips_breakpoint = 0x0005000d;
#define mips_breakpoint_len 4
+/* Implementation of linux_target_ops method "sw_breakpoint_from_kind". */
+
+static const gdb_byte *
+mips_sw_breakpoint_from_kind (int kind, int *size)
+{
+ *size = mips_breakpoint_len;
+ return (const gdb_byte *) &mips_breakpoint;
+}
+
/* We only place breakpoints in empty marker functions, and thread locking
is outside of the function. So rather than importing software single-step,
we can just run until exit. */
@@ -881,8 +890,8 @@ struct linux_target_ops the_low_target = {
NULL, /* fetch_register */
mips_get_pc,
mips_set_pc,
- (const unsigned char *) &mips_breakpoint,
- mips_breakpoint_len,
+ NULL, /* breakpoint_kind_from_pc */
+ mips_sw_breakpoint_from_kind,
mips_reinsert_addr,
0,
mips_breakpoint_at,
diff --git a/gdb/gdbserver/linux-nios2-low.c b/gdb/gdbserver/linux-nios2-low.c
index 71542b4..95a2d9e 100644
--- a/gdb/gdbserver/linux-nios2-low.c
+++ b/gdb/gdbserver/linux-nios2-low.c
@@ -127,9 +127,23 @@ nios2_set_pc (struct regcache *regcache, CORE_ADDR pc)
#define NIOS2_BREAKPOINT 0x003b6ffa
#endif
+/* We only register the 4-byte breakpoint, even on R2 targets which also
+ support 2-byte breakpoints. Since there is no supports_z_point_type
+ function provided, gdbserver never inserts software breakpoints itself
+ and instead relies on GDB to insert the breakpoint of the correct length
+ via a memory write. */
static const unsigned int nios2_breakpoint = NIOS2_BREAKPOINT;
#define nios2_breakpoint_len 4
+/* Implementation of linux_target_ops method "sw_breakpoint_from_kind". */
+
+static const gdb_byte *
+nios2_sw_breakpoint_from_kind (int kind, int *size)
+{
+ *size = nios2_breakpoint_len;
+ return (const gdb_byte *) &nios2_breakpoint;
+}
+
/* Implement the breakpoint_reinsert_addr linux_target_ops method. */
static CORE_ADDR
@@ -263,14 +277,8 @@ struct linux_target_ops the_low_target =
NULL,
nios2_get_pc,
nios2_set_pc,
-
- /* We only register the 4-byte breakpoint, even on R2 targets which also
- support 2-byte breakpoints. Since there is no supports_z_point_type
- function provided, gdbserver never inserts software breakpoints itself
- and instead relies on GDB to insert the breakpoint of the correct length
- via a memory write. */
- (const unsigned char *) &nios2_breakpoint,
- nios2_breakpoint_len,
+ NULL, /* breakpoint_kind_from_pc */
+ nios2_sw_breakpoint_from_kind,
nios2_reinsert_addr,
0,
nios2_breakpoint_at,
diff --git a/gdb/gdbserver/linux-ppc-low.c b/gdb/gdbserver/linux-ppc-low.c
index 188fac0..6e6a936 100644
--- a/gdb/gdbserver/linux-ppc-low.c
+++ b/gdb/gdbserver/linux-ppc-low.c
@@ -486,6 +486,15 @@ ppc_arch_setup (void)
static const unsigned int ppc_breakpoint = 0x7d821008;
#define ppc_breakpoint_len 4
+/* Implementation of linux_target_ops method "sw_breakpoint_from_kind". */
+
+static const gdb_byte *
+ppc_sw_breakpoint_from_kind (int kind, int *size)
+{
+ *size = ppc_breakpoint_len;
+ return (const gdb_byte *) &ppc_breakpoint;
+}
+
static int
ppc_breakpoint_at (CORE_ADDR where)
{
@@ -685,8 +694,8 @@ struct linux_target_ops the_low_target = {
NULL, /* fetch_register */
ppc_get_pc,
ppc_set_pc,
- (const unsigned char *) &ppc_breakpoint,
- ppc_breakpoint_len,
+ NULL, /* breakpoint_kind_from_pc */
+ ppc_sw_breakpoint_from_kind,
NULL,
0,
ppc_breakpoint_at,
diff --git a/gdb/gdbserver/linux-s390-low.c b/gdb/gdbserver/linux-s390-low.c
index 8a0a689..2ba1221 100644
--- a/gdb/gdbserver/linux-s390-low.c
+++ b/gdb/gdbserver/linux-s390-low.c
@@ -394,9 +394,18 @@ static struct regset_info s390_regsets[] = {
};
-static const unsigned char s390_breakpoint[] = { 0, 1 };
+static const gdb_byte s390_breakpoint[] = { 0, 1 };
#define s390_breakpoint_len 2
+/* Implementation of linux_target_ops method "sw_breakpoint_from_kind". */
+
+static const gdb_byte *
+s390_sw_breakpoint_from_kind (int kind, int *size)
+{
+ *size = s390_breakpoint_len;
+ return s390_breakpoint;
+}
+
static CORE_ADDR
s390_get_pc (struct regcache *regcache)
{
@@ -665,8 +674,8 @@ struct linux_target_ops the_low_target = {
NULL, /* fetch_register */
s390_get_pc,
s390_set_pc,
- s390_breakpoint,
- s390_breakpoint_len,
+ NULL, /* breakpoint_kind_from_pc */
+ s390_sw_breakpoint_from_kind,
NULL,
s390_breakpoint_len,
s390_breakpoint_at,
diff --git a/gdb/gdbserver/linux-sh-low.c b/gdb/gdbserver/linux-sh-low.c
index 218d4d3..f7f3239 100644
--- a/gdb/gdbserver/linux-sh-low.c
+++ b/gdb/gdbserver/linux-sh-low.c
@@ -77,6 +77,15 @@ sh_set_pc (struct regcache *regcache, CORE_ADDR pc)
static const unsigned short sh_breakpoint = 0xc3c3;
#define sh_breakpoint_len 2
+/* Implementation of linux_target_ops method "sw_breakpoint_from_kind". */
+
+static const gdb_byte *
+sh_sw_breakpoint_from_kind (int kind, int *size)
+{
+ *size = sh_breakpoint_len;
+ return (const gdb_byte *) &sh_breakpoint;
+}
+
static int
sh_breakpoint_at (CORE_ADDR where)
{
@@ -148,8 +157,8 @@ struct linux_target_ops the_low_target = {
NULL, /* fetch_register */
sh_get_pc,
sh_set_pc,
- (const unsigned char *) &sh_breakpoint,
- sh_breakpoint_len,
+ NULL, /* breakpoint_kind_from_pc */
+ sh_sw_breakpoint_from_kind,
NULL,
0,
sh_breakpoint_at,
diff --git a/gdb/gdbserver/linux-sparc-low.c b/gdb/gdbserver/linux-sparc-low.c
index 796af8a..0691867 100644
--- a/gdb/gdbserver/linux-sparc-low.c
+++ b/gdb/gdbserver/linux-sparc-low.c
@@ -235,11 +235,19 @@ sparc_get_pc (struct regcache *regcache)
return pc;
}
-static const unsigned char sparc_breakpoint[INSN_SIZE] = {
+static const gdb_byte sparc_breakpoint[INSN_SIZE] = {
0x91, 0xd0, 0x20, 0x01
};
#define sparc_breakpoint_len INSN_SIZE
+/* Implementation of linux_target_ops method "sw_breakpoint_from_kind". */
+
+static const unsigned char *
+sparc_sw_breakpoint_from_kind (int kind, int *size)
+{
+ *size = sparc_breakpoint_len;
+ return sparc_breakpoint;
+}
static int
sparc_breakpoint_at (CORE_ADDR where)
@@ -323,8 +331,8 @@ struct linux_target_ops the_low_target = {
sparc_get_pc,
/* No sparc_set_pc is needed. */
NULL,
- (const unsigned char *) sparc_breakpoint,
- sparc_breakpoint_len,
+ NULL, /* breakpoint_kind_from_pc */
+ sparc_sw_breakpoint_from_kind,
sparc_reinsert_addr,
0,
sparc_breakpoint_at,
diff --git a/gdb/gdbserver/linux-tic6x-low.c b/gdb/gdbserver/linux-tic6x-low.c
index a2ac3ee..d9476fd 100644
--- a/gdb/gdbserver/linux-tic6x-low.c
+++ b/gdb/gdbserver/linux-tic6x-low.c
@@ -171,6 +171,16 @@ extern struct linux_target_ops the_low_target;
static int *tic6x_regmap;
static unsigned int tic6x_breakpoint;
+#define tic6x_breakpoint_len 4
+
+/* Implementation of linux_target_ops method "sw_breakpoint_from_kind". */
+
+static const gdb_byte *
+tic6x_sw_breakpoint_from_kind (int kind, int *size)
+{
+ *size = tic6x_breakpoint_len;
+ return (const gdb_byte *) &tic6x_breakpoint;
+}
/* Forward definition. */
static struct usrregs_info tic6x_usrregs_info;
@@ -247,8 +257,6 @@ tic6x_set_pc (struct regcache *regcache, CORE_ADDR pc)
supply_register_by_name (regcache, "PC", newpc.buf);
}
-#define tic6x_breakpoint_len 4
-
static int
tic6x_breakpoint_at (CORE_ADDR where)
{
@@ -367,8 +375,8 @@ struct linux_target_ops the_low_target = {
NULL, /* fetch_register */
tic6x_get_pc,
tic6x_set_pc,
- (const unsigned char *) &tic6x_breakpoint,
- tic6x_breakpoint_len,
+ NULL, /* breakpoint_kind_from_pc */
+ tic6x_sw_breakpoint_from_kind,
NULL,
0,
tic6x_breakpoint_at,
diff --git a/gdb/gdbserver/linux-tile-low.c b/gdb/gdbserver/linux-tile-low.c
index 6aaea6a..e31a620 100644
--- a/gdb/gdbserver/linux-tile-low.c
+++ b/gdb/gdbserver/linux-tile-low.c
@@ -88,6 +88,15 @@ tile_set_pc (struct regcache *regcache, CORE_ADDR pc)
static uint64_t tile_breakpoint = 0x400b3cae70166000ULL;
#define tile_breakpoint_len 8
+/* Implementation of linux_target_ops method "sw_breakpoint_from_kind". */
+
+static const gdb_byte *
+tile_sw_breakpoint_from_kind (int kind, int *size)
+{
+ *size = tile_breakpoint_len;
+ return (const gdb_byte *) &tile_breakpoint;
+}
+
static int
tile_breakpoint_at (CORE_ADDR where)
{
@@ -182,8 +191,8 @@ struct linux_target_ops the_low_target =
NULL,
tile_get_pc,
tile_set_pc,
- (const unsigned char *) &tile_breakpoint,
- tile_breakpoint_len,
+ NULL, /* breakpoint_kind_from_pc */
+ tile_sw_breakpoint_from_kind,
NULL,
0,
tile_breakpoint_at,
diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c
index 20d4257..406d552 100644
--- a/gdb/gdbserver/linux-x86-low.c
+++ b/gdb/gdbserver/linux-x86-low.c
@@ -502,7 +502,7 @@ x86_set_pc (struct regcache *regcache, CORE_ADDR pc)
}
}
\f
-static const unsigned char x86_breakpoint[] = { 0xCC };
+static const gdb_byte x86_breakpoint[] = { 0xCC };
#define x86_breakpoint_len 1
static int
@@ -3243,6 +3243,15 @@ x86_emit_ops (void)
return &i386_emit_ops;
}
+/* Implementation of linux_target_ops method "sw_breakpoint_from_kind". */
+
+static const gdb_byte *
+x86_sw_breakpoint_from_kind (int kind, int *size)
+{
+ *size = x86_breakpoint_len;
+ return x86_breakpoint;
+}
+
static int
x86_supports_range_stepping (void)
{
@@ -3261,8 +3270,8 @@ struct linux_target_ops the_low_target =
NULL, /* fetch_register */
x86_get_pc,
x86_set_pc,
- x86_breakpoint,
- x86_breakpoint_len,
+ NULL, /* breakpoint_kind_from_pc */
+ x86_sw_breakpoint_from_kind,
NULL,
1,
x86_breakpoint_at,
diff --git a/gdb/gdbserver/linux-xtensa-low.c b/gdb/gdbserver/linux-xtensa-low.c
index debe467..fa6f418 100644
--- a/gdb/gdbserver/linux-xtensa-low.c
+++ b/gdb/gdbserver/linux-xtensa-low.c
@@ -151,9 +151,18 @@ static struct regset_info xtensa_regsets[] = {
#define XTENSA_BREAKPOINT {0x2d,0xf0}
#endif
-static const unsigned char xtensa_breakpoint[] = XTENSA_BREAKPOINT;
+static const gdb_byte xtensa_breakpoint[] = XTENSA_BREAKPOINT;
#define xtensa_breakpoint_len 2
+/* Implementation of linux_target_ops method "sw_breakpoint_from_kind". */
+
+static const gdb_byte *
+xtensa_sw_breakpoint_from_kind (int kind, int *size)
+{
+ *size = xtensa_breakpoint_len;
+ return xtensa_breakpoint;
+}
+
static CORE_ADDR
xtensa_get_pc (struct regcache *regcache)
{
@@ -234,8 +243,8 @@ struct linux_target_ops the_low_target = {
NULL, /* fetch_register */
xtensa_get_pc,
xtensa_set_pc,
- xtensa_breakpoint,
- xtensa_breakpoint_len,
+ NULL, /* breakpoint_kind_from_pc */
+ xtensa_sw_breakpoint_from_kind,
NULL,
0,
xtensa_breakpoint_at,
diff --git a/gdb/gdbserver/target.h b/gdb/gdbserver/target.h
index a2842b4..640d976 100644
--- a/gdb/gdbserver/target.h
+++ b/gdb/gdbserver/target.h
@@ -441,6 +441,16 @@ struct target_ops
readlink(2). */
ssize_t (*multifs_readlink) (int pid, const char *filename,
char *buf, size_t bufsiz);
+
+ /* Return the breakpoint kind for this target based on PC. The PCPTR is
+ adjusted to the real memory location in case a flag (e.g., the Thumb bit on
+ ARM) was present in the PC. */
+ int (*breakpoint_kind_from_pc) (CORE_ADDR *pcptr);
+
+ /* Return the software breakpoint from KIND. KIND can have target
+ specific meaning like the z0 kind parameter.
+ SIZE is set to the software breakpoint's length in memory. */
+ const gdb_byte *(*sw_breakpoint_from_kind) (int kind, int *size);
};
extern struct target_ops *the_target;
--
1.9.1
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v3 4/5] Refactor the breakpoint definitions in linux-arm-low.c.
2015-10-20 16:48 [PATCH v3 0/5] Software breakpoints support for ARM linux in GDBServer Antoine Tremblay
` (2 preceding siblings ...)
2015-10-20 16:49 ` [PATCH v3 2/5] Support breakpoint kinds " Antoine Tremblay
@ 2015-10-20 16:49 ` Antoine Tremblay
2015-10-21 13:04 ` Pedro Alves
2015-10-20 16:55 ` [PATCH v3 3/5] Implement breakpoint_kind_from_pc and sw_breakpoint_from_kind for ARM in GDBServer Antoine Tremblay
2015-10-21 13:34 ` [PATCH v3 0/5] Software breakpoints support for ARM linux " Pedro Alves
5 siblings, 1 reply; 20+ messages in thread
From: Antoine Tremblay @ 2015-10-20 16:49 UTC (permalink / raw)
To: gdb-patches; +Cc: Antoine Tremblay
Before arm_breakpoint_from_pc would use an #ifdef to return the right
arm_breakpoint from the abi or eabi breakpoint type.
arm_breakpoint_at would also check for the arm_breakpoint ||
arm_eabi_breakpoint.
Thus the selected arm_breakpoint would be what arm_breakpoint_from_pc returned
and arm_breakpoint was arm_abi_breakpoint.
This patch makes it more clear by naming those for what they are : 2 separate
entities: arm_abi_breakpoint and arm_eabi_breakpoint and set the current used
one as arm_breakpoint.
This allows a cleaner arm_breakpoint_from_pc as it just returns arm_breakpoint
rather than having the #ifdef in that function.
Any other reference to the arm_breakpoint can now also be clear of #ifdefs...
No regressions on Ubuntu 14.04 on ARMv7 and x86.
With gdbserver-{native,extended} / { -marm -mthumb }
gdb/gdbserver/ChangeLog:
* linux-arm-low.c: Refactor breakpoint definitions.
(arm_breakpoint_at): Adjust for arm_abi_breakpoint.
(arm_breakpoint_from_pc): Adjust for arm_breakpoint.
---
gdb/gdbserver/linux-arm-low.c | 27 +++++++++++++++------------
1 file changed, 15 insertions(+), 12 deletions(-)
diff --git a/gdb/gdbserver/linux-arm-low.c b/gdb/gdbserver/linux-arm-low.c
index 9b0975f..62e88f0 100644
--- a/gdb/gdbserver/linux-arm-low.c
+++ b/gdb/gdbserver/linux-arm-low.c
@@ -244,18 +244,25 @@ arm_set_pc (struct regcache *regcache, CORE_ADDR pc)
}
/* Correct in either endianness. */
-static const unsigned long arm_breakpoint = 0xef9f0001;
-#define arm_breakpoint_len 4
-static const unsigned short thumb_breakpoint = 0xde01;
-#define thumb_breakpoint_len 2
-static const unsigned short thumb2_breakpoint[] = { 0xf7f0, 0xa000 };
-#define thumb2_breakpoint_len 4
+#define arm_abi_breakpoint 0xef9f0001UL
/* For new EABI binaries. We recognize it regardless of which ABI
is used for gdbserver, so single threaded debugging should work
OK, but for multi-threaded debugging we only insert the current
ABI's breakpoint instruction. For now at least. */
-static const unsigned long arm_eabi_breakpoint = 0xe7f001f0;
+#define arm_eabi_breakpoint 0xe7f001f0UL
+
+#ifndef __ARM_EABI__
+static const unsigned long arm_breakpoint = arm_abi_breakpoint;
+#else
+static const unsigned long arm_breakpoint = arm_eabi_breakpoint;
+#endif
+
+#define arm_breakpoint_len 4
+static const unsigned short thumb_breakpoint = 0xde01;
+#define thumb_breakpoint_len 2
+static const unsigned short thumb2_breakpoint[] = { 0xf7f0, 0xa000 };
+#define thumb2_breakpoint_len 4
static int
arm_breakpoint_at (CORE_ADDR where)
@@ -287,7 +294,7 @@ arm_breakpoint_at (CORE_ADDR where)
unsigned long insn;
(*the_target->read_memory) (where, (unsigned char *) &insn, 4);
- if (insn == arm_breakpoint)
+ if (insn == arm_abi_breakpoint)
return 1;
if (insn == arm_eabi_breakpoint)
@@ -982,11 +989,7 @@ arm_sw_breakpoint_from_kind (int kind , int *size)
return (gdb_byte *) &thumb2_breakpoint;
case ARM_BP_KIND_ARM:
*size = arm_breakpoint_len;
-#ifndef __ARM_EABI__
return (const gdb_byte *) &arm_breakpoint;
-#else
- return (const gdb_byte *) &arm_eabi_breakpoint;
-#endif
default:
return NULL;
}
--
1.9.1
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v3 2/5] Support breakpoint kinds for software breakpoints in GDBServer.
2015-10-20 16:48 [PATCH v3 0/5] Software breakpoints support for ARM linux in GDBServer Antoine Tremblay
2015-10-20 16:48 ` [PATCH v3 5/5] Support software breakpoints " Antoine Tremblay
2015-10-20 16:48 ` [PATCH v3 1/5] Add the target_ops needed for software breakpoints " Antoine Tremblay
@ 2015-10-20 16:49 ` Antoine Tremblay
2015-10-21 11:49 ` Pedro Alves
2015-10-22 15:17 ` Ulrich Weigand
2015-10-20 16:49 ` [PATCH v3 4/5] Refactor the breakpoint definitions in linux-arm-low.c Antoine Tremblay
` (2 subsequent siblings)
5 siblings, 2 replies; 20+ messages in thread
From: Antoine Tremblay @ 2015-10-20 16:49 UTC (permalink / raw)
To: gdb-patches; +Cc: Antoine Tremblay
In this v3:
* Address Pedro's comments about the breakpoint struct usage.
* Fixed formatting issues.
-
There's two ways to set breakpoints in GDBServer.
- GDBServer setting its own breakpoints, through API set_breakpoint_at.
- GDBServer setting breakpoints according to the information in Z
packets, through API set_gdb_breakpoint.
Before this patch the breakpoint kinds were a concept unique to GDB and Z
packets, as GDBServer never had to set different kinds of breakpoint on its
own.
This patch teaches GDBServer to handle breakpoint kinds for its own
breakpoints. It generalizes the breakpoint kind as per Z packets to
represent different kinds of breakpoints directly set by GDBServer also.
GDBServer now querys breakpoint_kind_from_pc to know what breakpoint kind to
set on its own.
As the kind is now a differentiating factor equivalent to size for the
breakpoint struct and that it's size can be queryed using
sw_breakpoint_from_kind, the size field has been replaced with the kind field.
All references to size are now replaced by kind or a call to bp_size that wraps
sw_breakpoing_from_kind and returns the size of the breakpoint in memory.
To fetch the software breakpoint data bp_opcode is called and wraps the
sw_breakpoint_from_kind call.
No regressions on Ubuntu 14.04 on ARMv7 and x86.
With gdbserver-{native,extended} / { -marm -mthumb }
gdb/gdbserver/ChangeLog:
* linux-low.c (initialize_low): Ajdust for breakpoint global variables
removal.
* mem-break.c : Remove breakpoint_data/breakpoint_len global variables.
(struct raw_breakpoint) <size>: Remove.
(struct raw_breakpoint) <kind>: Add.
(bp_size): New function.
(bp_opcode): Likewise.
(find_raw_breakpoint_at): Adjust for kind.
(insert_memory_breakpoint): Ajust for kind call bp_size,bp_opcode.
(remove_memory_breakpoint): Ajust for kind call bp_size.
(set_raw_breakpoint_at): Adjust for kind.
(set_breakpoint): Likewise.
(set_breakpoint_at): Call breakpoint_kind_from_pc.
(delete_raw_breakpoint): Adjust for kind.
(delete_breakpoint): Likewise.
(find_gdb_breakpoint): Likewise.
(set_gdb_breakpoint_1): Likewise.
(set_gdb_breakpoint): Likewise.
(delete_gdb_breakpoint_1): Likewise.
(delete_gdb_breakpoint): Likewise.
(uninsert_raw_breakpoint): Likewise.
(reinsert_raw_breakpoint): Likewise.
(set_breakpoint_data): Remove.
(validate_inserted_breakpoint): Adjust for kind call bp_size,bp_opcode.
(check_mem_read): Adjust for kind call bp_size.
(check_mem_write): Adjust for kind call bp_size,bp_opcode.
(clone_one_breakpoint): Adjust for kind.
* mem-break.h (set_gdb_breakpoint): Likewise.
(delete_gdb_breakpoint): Likewise.
* server.c (process_serial_event): Likewise.
---
gdb/gdbserver/linux-low.c | 10 ----
gdb/gdbserver/mem-break.c | 150 +++++++++++++++++++++++-----------------------
gdb/gdbserver/mem-break.h | 8 +--
gdb/gdbserver/server.c | 8 +--
4 files changed, 83 insertions(+), 93 deletions(-)
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 398c0aa..c20e257 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -7082,20 +7082,10 @@ void
initialize_low (void)
{
struct sigaction sigchld_action;
- int breakpoint_kind = 0;
- int breakpoint_size = 0;
- const gdb_byte *breakpoint = NULL;
memset (&sigchld_action, 0, sizeof (sigchld_action));
set_target_ops (&linux_target_ops);
- breakpoint_kind = the_target->breakpoint_kind_from_pc (NULL);
- breakpoint = the_target->sw_breakpoint_from_kind (breakpoint_kind,
- &breakpoint_size);
-
- set_breakpoint_data (breakpoint,
- breakpoint_size);
-
linux_init_signals ();
linux_ptrace_init_warnings ();
diff --git a/gdb/gdbserver/mem-break.c b/gdb/gdbserver/mem-break.c
index f077497..85ef7b0 100644
--- a/gdb/gdbserver/mem-break.c
+++ b/gdb/gdbserver/mem-break.c
@@ -21,8 +21,6 @@
#include "server.h"
#include "regcache.h"
#include "ax.h"
-const unsigned char *breakpoint_data;
-int breakpoint_len;
#define MAX_BREAKPOINT_LEN 8
@@ -100,8 +98,8 @@ struct raw_breakpoint
breakpoint for a given PC. */
CORE_ADDR pc;
- /* The breakpoint's size. */
- int size;
+ /* The breakpoint's kind. */
+ int kind;
/* The breakpoint's shadow memory. */
unsigned char old_data[MAX_BREAKPOINT_LEN];
@@ -189,6 +187,27 @@ struct breakpoint
int (*handler) (CORE_ADDR);
};
+/* Return the breakpoint size from its kind. */
+
+static int
+bp_size (struct raw_breakpoint *bp)
+{
+ int size = 0;
+
+ the_target->sw_breakpoint_from_kind (bp->kind, &size);
+ return size;
+}
+
+/* Return the breakpoint opcode from its kind. */
+
+static const gdb_byte *
+bp_opcode (struct raw_breakpoint *bp)
+{
+ int size = 0;
+
+ return the_target->sw_breakpoint_from_kind (bp->kind, &size);
+}
+
/* See mem-break.h. */
enum target_hw_bp_type
@@ -281,13 +300,13 @@ find_enabled_raw_code_breakpoint_at (CORE_ADDR addr, enum raw_bkpt_type type)
NULL if not found. */
static struct raw_breakpoint *
-find_raw_breakpoint_at (CORE_ADDR addr, enum raw_bkpt_type type, int size)
+find_raw_breakpoint_at (CORE_ADDR addr, enum raw_bkpt_type type, int kind)
{
struct process_info *proc = current_process ();
struct raw_breakpoint *bp;
for (bp = proc->raw_breakpoints; bp != NULL; bp = bp->next)
- if (bp->pc == addr && bp->raw_type == type && bp->size == size)
+ if (bp->pc == addr && bp->raw_type == type && bp->kind == kind)
return bp;
return NULL;
@@ -301,24 +320,10 @@ insert_memory_breakpoint (struct raw_breakpoint *bp)
unsigned char buf[MAX_BREAKPOINT_LEN];
int err;
- if (breakpoint_data == NULL)
- return 1;
-
- /* If the architecture treats the size field of Z packets as a
- 'kind' field, then we'll need to be able to know which is the
- breakpoint instruction too. */
- if (bp->size != breakpoint_len)
- {
- if (debug_threads)
- debug_printf ("Don't know how to insert breakpoints of size %d.\n",
- bp->size);
- return -1;
- }
-
/* Note that there can be fast tracepoint jumps installed in the
same memory range, so to get at the original memory, we need to
use read_inferior_memory, which masks those out. */
- err = read_inferior_memory (bp->pc, buf, breakpoint_len);
+ err = read_inferior_memory (bp->pc, buf, bp_size (bp));
if (err != 0)
{
if (debug_threads)
@@ -328,10 +333,10 @@ insert_memory_breakpoint (struct raw_breakpoint *bp)
}
else
{
- memcpy (bp->old_data, buf, breakpoint_len);
+ memcpy (bp->old_data, buf, bp_size (bp));
- err = (*the_target->write_memory) (bp->pc, breakpoint_data,
- breakpoint_len);
+ err = (*the_target->write_memory) (bp->pc, bp_opcode (bp),
+ bp_size (bp));
if (err != 0)
{
if (debug_threads)
@@ -358,8 +363,8 @@ remove_memory_breakpoint (struct raw_breakpoint *bp)
note that we need to pass the current shadow contents, because
write_inferior_memory updates any shadow memory with what we pass
here, and we want that to be a nop. */
- memcpy (buf, bp->old_data, breakpoint_len);
- err = write_inferior_memory (bp->pc, buf, breakpoint_len);
+ memcpy (buf, bp->old_data, bp_size (bp));
+ err = write_inferior_memory (bp->pc, buf, bp_size (bp));
if (err != 0)
{
if (debug_threads)
@@ -370,12 +375,12 @@ remove_memory_breakpoint (struct raw_breakpoint *bp)
return err != 0 ? -1 : 0;
}
-/* Set a RAW breakpoint of type TYPE and size SIZE at WHERE. On
+/* Set a RAW breakpoint of type TYPE and kind KIND at WHERE. On
success, a pointer to the new breakpoint is returned. On failure,
returns NULL and writes the error code to *ERR. */
static struct raw_breakpoint *
-set_raw_breakpoint_at (enum raw_bkpt_type type, CORE_ADDR where, int size,
+set_raw_breakpoint_at (enum raw_bkpt_type type, CORE_ADDR where, int kind,
int *err)
{
struct process_info *proc = current_process ();
@@ -384,19 +389,19 @@ set_raw_breakpoint_at (enum raw_bkpt_type type, CORE_ADDR where, int size,
if (type == raw_bkpt_type_sw || type == raw_bkpt_type_hw)
{
bp = find_enabled_raw_code_breakpoint_at (where, type);
- if (bp != NULL && bp->size != size)
+ if (bp != NULL && bp->kind != kind)
{
- /* A different size than previously seen. The previous
+ /* A different kind than previously seen. The previous
breakpoint must be gone then. */
if (debug_threads)
- debug_printf ("Inconsistent breakpoint size? Was %d, now %d.\n",
- bp->size, size);
+ debug_printf ("Inconsistent breakpoint kind? Was %d, now %d.\n",
+ bp->kind, kind);
bp->inserted = -1;
bp = NULL;
}
}
else
- bp = find_raw_breakpoint_at (where, type, size);
+ bp = find_raw_breakpoint_at (where, type, kind);
if (bp != NULL)
{
@@ -406,11 +411,11 @@ set_raw_breakpoint_at (enum raw_bkpt_type type, CORE_ADDR where, int size,
bp = XCNEW (struct raw_breakpoint);
bp->pc = where;
- bp->size = size;
+ bp->kind = kind;
bp->refcount = 1;
bp->raw_type = type;
- *err = the_target->insert_point (bp->raw_type, bp->pc, bp->size, bp);
+ *err = the_target->insert_point (bp->raw_type, bp->pc, bp->kind, bp);
if (*err != 0)
{
if (debug_threads)
@@ -732,7 +737,7 @@ reinsert_fast_tracepoint_jumps_at (CORE_ADDR where)
}
/* Set a high-level breakpoint of type TYPE, with low level type
- RAW_TYPE and size SIZE, at WHERE. On success, a pointer to the new
+ RAW_TYPE and kind KIND, at WHERE. On success, a pointer to the new
breakpoint is returned. On failure, returns NULL and writes the
error code to *ERR. HANDLER is called when the breakpoint is hit.
HANDLER should return 1 if the breakpoint should be deleted, 0
@@ -740,14 +745,14 @@ reinsert_fast_tracepoint_jumps_at (CORE_ADDR where)
static struct breakpoint *
set_breakpoint (enum bkpt_type type, enum raw_bkpt_type raw_type,
- CORE_ADDR where, int size,
+ CORE_ADDR where, int kind,
int (*handler) (CORE_ADDR), int *err)
{
struct process_info *proc = current_process ();
struct breakpoint *bp;
struct raw_breakpoint *raw;
- raw = set_raw_breakpoint_at (raw_type, where, size, err);
+ raw = set_raw_breakpoint_at (raw_type, where, kind, err);
if (raw == NULL)
{
@@ -773,9 +778,11 @@ struct breakpoint *
set_breakpoint_at (CORE_ADDR where, int (*handler) (CORE_ADDR))
{
int err_ignored;
+ CORE_ADDR placed_address = where;
+ int breakpoint_kind = the_target->breakpoint_kind_from_pc (&placed_address);
return set_breakpoint (other_breakpoint, raw_bkpt_type_sw,
- where, breakpoint_len, handler,
+ placed_address, breakpoint_kind, handler,
&err_ignored);
}
@@ -799,7 +806,7 @@ delete_raw_breakpoint (struct process_info *proc, struct raw_breakpoint *todel)
*bp_link = bp->next;
- ret = the_target->remove_point (bp->raw_type, bp->pc, bp->size,
+ ret = the_target->remove_point (bp->raw_type, bp->pc, bp->kind,
bp);
if (ret != 0)
{
@@ -891,12 +898,12 @@ delete_breakpoint (struct breakpoint *todel)
return delete_breakpoint_1 (proc, todel);
}
-/* Locate a GDB breakpoint of type Z_TYPE and size SIZE placed at
- address ADDR and return a pointer to its structure. If SIZE is -1,
- the breakpoints' sizes are ignored. */
+/* Locate a GDB breakpoint of type Z_TYPE and kind KIND placed at
+ address ADDR and return a pointer to its structure. If KIND is -1,
+ the breakpoint's kind are ignored. */
static struct breakpoint *
-find_gdb_breakpoint (char z_type, CORE_ADDR addr, int size)
+find_gdb_breakpoint (char z_type, CORE_ADDR addr, int kind)
{
struct process_info *proc = current_process ();
struct breakpoint *bp;
@@ -904,7 +911,7 @@ find_gdb_breakpoint (char z_type, CORE_ADDR addr, int size)
for (bp = proc->breakpoints; bp != NULL; bp = bp->next)
if (bp->type == type && bp->raw->pc == addr
- && (size == -1 || bp->raw->size == size))
+ && (kind == -1 || bp->raw->kind == kind))
return bp;
return NULL;
@@ -918,13 +925,13 @@ z_type_supported (char z_type)
&& the_target->supports_z_point_type (z_type));
}
-/* Create a new GDB breakpoint of type Z_TYPE at ADDR with size SIZE.
+/* Create a new GDB breakpoint of type Z_TYPE at ADDR with kind KIND.
Returns a pointer to the newly created breakpoint on success. On
failure returns NULL and sets *ERR to either -1 for error, or 1 if
Z_TYPE breakpoints are not supported on this target. */
static struct breakpoint *
-set_gdb_breakpoint_1 (char z_type, CORE_ADDR addr, int size, int *err)
+set_gdb_breakpoint_1 (char z_type, CORE_ADDR addr, int kind, int *err)
{
struct breakpoint *bp;
enum bkpt_type type;
@@ -952,9 +959,9 @@ set_gdb_breakpoint_1 (char z_type, CORE_ADDR addr, int size, int *err)
if (bp != NULL)
{
- if (bp->raw->size != size)
+ if (bp->raw->kind != kind)
{
- /* A different size than previously seen. The previous
+ /* A different kind than previously seen. The previous
breakpoint must be gone then. */
bp->raw->inserted = -1;
delete_breakpoint (bp);
@@ -975,10 +982,10 @@ set_gdb_breakpoint_1 (char z_type, CORE_ADDR addr, int size, int *err)
}
else
{
- /* Data breakpoints for the same address but different size are
+ /* Data breakpoints for the same address but different kind are
expected. GDB doesn't merge these. The backend gets to do
that if it wants/can. */
- bp = find_gdb_breakpoint (z_type, addr, size);
+ bp = find_gdb_breakpoint (z_type, addr, kind);
}
if (bp != NULL)
@@ -993,7 +1000,7 @@ set_gdb_breakpoint_1 (char z_type, CORE_ADDR addr, int size, int *err)
raw_type = Z_packet_to_raw_bkpt_type (z_type);
type = Z_packet_to_bkpt_type (z_type);
- return set_breakpoint (type, raw_type, addr, size, NULL, err);
+ return set_breakpoint (type, raw_type, addr, kind, NULL, err);
}
static int
@@ -1024,7 +1031,7 @@ check_gdb_bp_preconditions (char z_type, int *err)
knows to prepare to access memory for Z0 breakpoints. */
struct breakpoint *
-set_gdb_breakpoint (char z_type, CORE_ADDR addr, int size, int *err)
+set_gdb_breakpoint (char z_type, CORE_ADDR addr, int kind, int *err)
{
struct breakpoint *bp;
@@ -1040,7 +1047,7 @@ set_gdb_breakpoint (char z_type, CORE_ADDR addr, int size, int *err)
return NULL;
}
- bp = set_gdb_breakpoint_1 (z_type, addr, size, err);
+ bp = set_gdb_breakpoint_1 (z_type, addr, kind, err);
if (z_type == Z_PACKET_SW_BP)
done_accessing_memory ();
@@ -1048,18 +1055,18 @@ set_gdb_breakpoint (char z_type, CORE_ADDR addr, int size, int *err)
return bp;
}
-/* Delete a GDB breakpoint of type Z_TYPE and size SIZE previously
+/* Delete a GDB breakpoint of type Z_TYPE and kind KIND previously
inserted at ADDR with set_gdb_breakpoint_at. Returns 0 on success,
-1 on error, and 1 if Z_TYPE breakpoints are not supported on this
target. */
static int
-delete_gdb_breakpoint_1 (char z_type, CORE_ADDR addr, int size)
+delete_gdb_breakpoint_1 (char z_type, CORE_ADDR addr, int kind)
{
struct breakpoint *bp;
int err;
- bp = find_gdb_breakpoint (z_type, addr, size);
+ bp = find_gdb_breakpoint (z_type, addr, kind);
if (bp == NULL)
return -1;
@@ -1077,7 +1084,7 @@ delete_gdb_breakpoint_1 (char z_type, CORE_ADDR addr, int size)
knows to prepare to access memory for Z0 breakpoints. */
int
-delete_gdb_breakpoint (char z_type, CORE_ADDR addr, int size)
+delete_gdb_breakpoint (char z_type, CORE_ADDR addr, int kind)
{
int ret;
@@ -1095,7 +1102,7 @@ delete_gdb_breakpoint (char z_type, CORE_ADDR addr, int size)
return -1;
}
- ret = delete_gdb_breakpoint_1 (z_type, addr, size);
+ ret = delete_gdb_breakpoint_1 (z_type, addr, kind);
if (z_type == Z_PACKET_SW_BP)
done_accessing_memory ();
@@ -1438,7 +1445,7 @@ uninsert_raw_breakpoint (struct raw_breakpoint *bp)
bp->inserted = 0;
- err = the_target->remove_point (bp->raw_type, bp->pc, bp->size, bp);
+ err = the_target->remove_point (bp->raw_type, bp->pc, bp->kind, bp);
if (err != 0)
{
bp->inserted = 1;
@@ -1500,7 +1507,7 @@ reinsert_raw_breakpoint (struct raw_breakpoint *bp)
if (bp->inserted)
error ("Breakpoint already inserted at reinsert time.");
- err = the_target->insert_point (bp->raw_type, bp->pc, bp->size, bp);
+ err = the_target->insert_point (bp->raw_type, bp->pc, bp->kind, bp);
if (err == 0)
bp->inserted = 1;
else if (debug_threads)
@@ -1588,13 +1595,6 @@ check_breakpoints (CORE_ADDR stop_pc)
}
}
-void
-set_breakpoint_data (const unsigned char *bp_data, int bp_len)
-{
- breakpoint_data = bp_data;
- breakpoint_len = bp_len;
-}
-
int
breakpoint_here (CORE_ADDR addr)
{
@@ -1669,9 +1669,9 @@ validate_inserted_breakpoint (struct raw_breakpoint *bp)
gdb_assert (bp->inserted);
gdb_assert (bp->raw_type == raw_bkpt_type_sw);
- buf = (unsigned char *) alloca (breakpoint_len);
- err = (*the_target->read_memory) (bp->pc, buf, breakpoint_len);
- if (err || memcmp (buf, breakpoint_data, breakpoint_len) != 0)
+ buf = (unsigned char *) alloca (bp_size (bp));
+ err = (*the_target->read_memory) (bp->pc, buf, bp_size (bp));
+ if (err || memcmp (buf, bp_opcode (bp), bp_size (bp)) != 0)
{
/* Tag it as gone. */
bp->inserted = -1;
@@ -1762,7 +1762,7 @@ check_mem_read (CORE_ADDR mem_addr, unsigned char *buf, int mem_len)
for (; bp != NULL; bp = bp->next)
{
- CORE_ADDR bp_end = bp->pc + breakpoint_len;
+ CORE_ADDR bp_end = bp->pc + bp_size (bp);
CORE_ADDR start, end;
int copy_offset, copy_len, buf_offset;
@@ -1851,7 +1851,7 @@ check_mem_write (CORE_ADDR mem_addr, unsigned char *buf,
for (; bp != NULL; bp = bp->next)
{
- CORE_ADDR bp_end = bp->pc + breakpoint_len;
+ CORE_ADDR bp_end = bp->pc + bp_size (bp);
CORE_ADDR start, end;
int copy_offset, copy_len, buf_offset;
@@ -1882,7 +1882,7 @@ check_mem_write (CORE_ADDR mem_addr, unsigned char *buf,
if (bp->inserted > 0)
{
if (validate_inserted_breakpoint (bp))
- memcpy (buf + buf_offset, breakpoint_data + copy_offset, copy_len);
+ memcpy (buf + buf_offset, bp_opcode (bp) + copy_offset, copy_len);
else
disabled_one = 1;
}
@@ -1963,7 +1963,7 @@ clone_one_breakpoint (const struct breakpoint *src)
dest_raw->raw_type = src->raw->raw_type;
dest_raw->refcount = src->raw->refcount;
dest_raw->pc = src->raw->pc;
- dest_raw->size = src->raw->size;
+ dest_raw->kind = src->raw->kind;
memcpy (dest_raw->old_data, src->raw->old_data, MAX_BREAKPOINT_LEN);
dest_raw->inserted = src->raw->inserted;
diff --git a/gdb/gdbserver/mem-break.h b/gdb/gdbserver/mem-break.h
index b5a3208..fa9d300 100644
--- a/gdb/gdbserver/mem-break.h
+++ b/gdb/gdbserver/mem-break.h
@@ -65,20 +65,20 @@ enum raw_bkpt_type Z_packet_to_raw_bkpt_type (char z_type);
enum target_hw_bp_type raw_bkpt_type_to_target_hw_bp_type
(enum raw_bkpt_type raw_type);
-/* Create a new GDB breakpoint of type Z_TYPE at ADDR with size SIZE.
+/* Create a new GDB breakpoint of type Z_TYPE at ADDR with kind KIND.
Returns a pointer to the newly created breakpoint on success. On
failure returns NULL and sets *ERR to either -1 for error, or 1 if
Z_TYPE breakpoints are not supported on this target. */
-struct breakpoint *set_gdb_breakpoint (char z_type, CORE_ADDR addr, int size,
+struct breakpoint *set_gdb_breakpoint (char z_type, CORE_ADDR addr, int kind,
int *err);
-/* Delete a GDB breakpoint of type Z_TYPE and size SIZE previously
+/* Delete a GDB breakpoint of type Z_TYPE and kind KIND previously
inserted at ADDR with set_gdb_breakpoint_at. Returns 0 on success,
-1 on error, and 1 if Z_TYPE breakpoints are not supported on this
target. */
-int delete_gdb_breakpoint (char z_type, CORE_ADDR addr, int size);
+int delete_gdb_breakpoint (char z_type, CORE_ADDR addr, int kind);
/* Returns TRUE if there's a software or hardware (code) breakpoint at
ADDR in our tables, inserted, or not. */
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index e25b7c7..ad6626e 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -4069,20 +4069,20 @@ process_serial_event (void)
{
char *dataptr;
ULONGEST addr;
- int len;
+ int kind;
char type = own_buf[1];
int res;
const int insert = ch == 'Z';
char *p = &own_buf[3];
p = unpack_varlen_hex (p, &addr);
- len = strtol (p + 1, &dataptr, 16);
+ kind = strtol (p + 1, &dataptr, 16);
if (insert)
{
struct breakpoint *bp;
- bp = set_gdb_breakpoint (type, addr, len, &res);
+ bp = set_gdb_breakpoint (type, addr, kind, &res);
if (bp != NULL)
{
res = 0;
@@ -4097,7 +4097,7 @@ process_serial_event (void)
}
}
else
- res = delete_gdb_breakpoint (type, addr, len);
+ res = delete_gdb_breakpoint (type, addr, kind);
if (res == 0)
write_ok (own_buf);
--
1.9.1
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v3 3/5] Implement breakpoint_kind_from_pc and sw_breakpoint_from_kind for ARM in GDBServer.
2015-10-20 16:48 [PATCH v3 0/5] Software breakpoints support for ARM linux in GDBServer Antoine Tremblay
` (3 preceding siblings ...)
2015-10-20 16:49 ` [PATCH v3 4/5] Refactor the breakpoint definitions in linux-arm-low.c Antoine Tremblay
@ 2015-10-20 16:55 ` Antoine Tremblay
2015-10-21 12:04 ` Pedro Alves
2015-10-21 13:34 ` [PATCH v3 0/5] Software breakpoints support for ARM linux " Pedro Alves
5 siblings, 1 reply; 20+ messages in thread
From: Antoine Tremblay @ 2015-10-20 16:55 UTC (permalink / raw)
To: gdb-patches; +Cc: Antoine Tremblay
In this v3:
* Include common-defs.h
-
ARM can have multiple breakpoint types based on the instruction set
it's currently in: arm, thumb or thumb2.
GDBServer needs to know what breakpoint is to be inserted at location
when inserting a breakpoint.
This is handled by the breakpoint_kind_from_pc and sw_breakpoint_from_kind
target ops introduced in a previous patch, this patch adds the
arm_breakpoint_kind_from_pc and arm_sw_breakpoint_from_kind implementation so
that the proper breakpoint type is returned based on the pc.
Also in order to share some code with GDB a new file called arm.c have been
introduced in arch/.
While this file does not contain much yet future patches will add more
to it thus the inclusion at this stage.
No regressions on Ubuntu 14.04 on ARMv7 and x86.
With gdbserver-{native,extended} / { -marm -mthumb }
gdb/ChangeLog:
* Makefile.in: Add arm.c/o.
* arch/arm.c: New file.
* arch/arm.h: (IS_THUMB_ADDR): Move macro from arm-tdep.c.
(MAKE_THUMB_ADDR): Likewise.
(UNMAKE_THUMB_ADDR): Likewise.
* arm-tdep.c (int thumb_insn_size): Move to arm.c.
(IS_THUMB_ADDR): Move to arm.h.
(MAKE_THUMB_ADDR): Likewise.
(UNMAKE_THUMB_ADDR): Likewise.
* configure.tgt: Add arm.o to all ARM configs.
gdb/gdbserver/ChangeLog:
* Makefile.in: Add arm.c/o.
* configure.srv: Likewise.
* linux-arm-low.c (arm_breakpoint_kinds): New enum.
(arm_breakpoint_kind_from_pc): New function.
(arm_sw_breakpoint_from_kind): Return proper kind.
(struct linux_target_ops) <breakpoint_kind_from_pc>: Initialize.
---
gdb/Makefile.in | 13 +++++++-
gdb/arch/arm.c | 33 ++++++++++++++++++++
gdb/arch/arm.h | 12 +++++++-
gdb/arm-tdep.c | 21 +------------
gdb/configure.tgt | 16 +++++-----
gdb/gdbserver/Makefile.in | 9 +++++-
gdb/gdbserver/configure.srv | 1 +
gdb/gdbserver/linux-arm-low.c | 70 ++++++++++++++++++++++++++++++++++++++++---
8 files changed, 140 insertions(+), 35 deletions(-)
create mode 100644 gdb/arch/arm.c
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index d5ca2ee..14ad405 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -657,7 +657,7 @@ ALL_64_TARGET_OBS = \
# All other target-dependent objects files (used with --enable-targets=all).
ALL_TARGET_OBS = \
- armbsd-tdep.o arm-linux-tdep.o arm-symbian-tdep.o \
+ armbsd-tdep.o arm.o arm-linux-tdep.o arm-symbian-tdep.o \
armnbsd-tdep.o armobsd-tdep.o \
arm-tdep.o arm-wince-tdep.o \
avr-tdep.o \
@@ -1660,6 +1660,7 @@ ALLDEPFILES = \
amd64-dicos-tdep.c \
amd64-linux-nat.c amd64-linux-tdep.c \
amd64-sol2-tdep.c \
+ arm.c \
arm-linux-nat.c arm-linux-tdep.c arm-symbian-tdep.c arm-tdep.c \
armnbsd-nat.c armbsd-tdep.c armnbsd-tdep.c armobsd-tdep.c \
avr-tdep.c \
@@ -2275,6 +2276,16 @@ waitstatus.o: ${srcdir}/target/waitstatus.c
$(COMPILE) $(srcdir)/target/waitstatus.c
$(POSTCOMPILE)
+#
+# gdb/arch/ dependencies
+#
+# Need to explicitly specify the compile rule as make will do nothing
+# or try to compile the object file into the sub-directory.
+
+arm.o: ${srcdir}/arch/arm.c
+ $(COMPILE) $(srcdir)/arch/arm.c
+ $(POSTCOMPILE)
+
# gdb/nat/ dependencies
#
# Need to explicitly specify the compile rule as make will do nothing
diff --git a/gdb/arch/arm.c b/gdb/arch/arm.c
new file mode 100644
index 0000000..b11c684
--- /dev/null
+++ b/gdb/arch/arm.c
@@ -0,0 +1,33 @@
+/* Common target dependent code for GDB on ARM systems.
+
+ Copyright (C) 1988-2015 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include "common-defs.h"
+#include "arm.h"
+
+/* Return the size in bytes of the complete Thumb instruction whose
+ first halfword is INST1. */
+
+int
+thumb_insn_size (unsigned short inst1)
+{
+ if ((inst1 & 0xe000) == 0xe000 && (inst1 & 0x1800) != 0)
+ return 4;
+ else
+ return 2;
+}
diff --git a/gdb/arch/arm.h b/gdb/arch/arm.h
index e0eed60..a054776 100644
--- a/gdb/arch/arm.h
+++ b/gdb/arch/arm.h
@@ -1,5 +1,5 @@
/* Common target dependent code for GDB on ARM systems.
- Copyright (C) 2002-2015 Free Software Foundation, Inc.
+ Copyright (C) 1988-2015 Free Software Foundation, Inc.
This file is part of GDB.
@@ -58,4 +58,14 @@ enum gdb_regnum {
ARM_LAST_FP_ARG_REGNUM = ARM_F3_REGNUM
};
+/* Addresses for calling Thumb functions have the bit 0 set.
+ Here are some macros to test, set, or clear bit 0 of addresses. */
+#define IS_THUMB_ADDR(addr) ((addr) & 1)
+#define MAKE_THUMB_ADDR(addr) ((addr) | 1)
+#define UNMAKE_THUMB_ADDR(addr) ((addr) & ~1)
+
+/* Return the size in bytes of the complete Thumb instruction whose
+ first halfword is INST1. */
+int thumb_insn_size (unsigned short inst1);
+
#endif
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 249e1d1..3a6c6d8 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -45,6 +45,7 @@
#include "user-regs.h"
#include "observer.h"
+#include "arch/arm.h"
#include "arm-tdep.h"
#include "gdb/sim-arm.h"
@@ -235,8 +236,6 @@ static void arm_neon_quad_write (struct gdbarch *gdbarch,
struct regcache *regcache,
int regnum, const gdb_byte *buf);
-static int thumb_insn_size (unsigned short inst1);
-
struct arm_prologue_cache
{
/* The stack pointer at the time this frame was created; i.e. the
@@ -267,12 +266,6 @@ static CORE_ADDR arm_analyze_prologue (struct gdbarch *gdbarch,
#define DISPLACED_STEPPING_ARCH_VERSION 5
-/* Addresses for calling Thumb functions have the bit 0 set.
- Here are some macros to test, set, or clear bit 0 of addresses. */
-#define IS_THUMB_ADDR(addr) ((addr) & 1)
-#define MAKE_THUMB_ADDR(addr) ((addr) | 1)
-#define UNMAKE_THUMB_ADDR(addr) ((addr) & ~1)
-
/* Set to true if the 32-bit mode is in use. */
int arm_apcs_32 = 1;
@@ -4364,18 +4357,6 @@ bitcount (unsigned long val)
return nbits;
}
-/* Return the size in bytes of the complete Thumb instruction whose
- first halfword is INST1. */
-
-static int
-thumb_insn_size (unsigned short inst1)
-{
- if ((inst1 & 0xe000) == 0xe000 && (inst1 & 0x1800) != 0)
- return 4;
- else
- return 2;
-}
-
static int
thumb_advance_itstate (unsigned int itstate)
{
diff --git a/gdb/configure.tgt b/gdb/configure.tgt
index 33d4cfc..2e824ad 100644
--- a/gdb/configure.tgt
+++ b/gdb/configure.tgt
@@ -44,7 +44,7 @@ aarch64*-*-elf)
aarch64*-*-linux*)
# Target: AArch64 linux
gdb_target_obs="aarch64-tdep.o aarch64-linux-tdep.o aarch64-insn.o \
- arm-tdep.o arm-linux-tdep.o \
+ arm.o arm-tdep.o arm-linux-tdep.o \
glibc-tdep.o linux-tdep.o solib-svr4.o \
symfile-mem.o linux-record.o"
build_gdbserver=yes
@@ -84,31 +84,31 @@ am33_2.0*-*-linux*)
arm*-wince-pe | arm*-*-mingw32ce*)
# Target: ARM based machine running Windows CE (win32)
- gdb_target_obs="arm-tdep.o arm-wince-tdep.o windows-tdep.o"
+ gdb_target_obs="arm.o arm-tdep.o arm-wince-tdep.o windows-tdep.o"
build_gdbserver=yes
;;
arm*-*-linux*)
# Target: ARM based machine running GNU/Linux
- gdb_target_obs="arm-tdep.o arm-linux-tdep.o glibc-tdep.o \
+ gdb_target_obs="arm.o arm-tdep.o arm-linux-tdep.o glibc-tdep.o \
solib-svr4.o symfile-mem.o linux-tdep.o linux-record.o"
build_gdbserver=yes
;;
arm*-*-netbsd* | arm*-*-knetbsd*-gnu)
# Target: NetBSD/arm
- gdb_target_obs="arm-tdep.o armnbsd-tdep.o solib-svr4.o"
+ gdb_target_obs="arm.o arm-tdep.o armnbsd-tdep.o solib-svr4.o"
;;
arm*-*-openbsd*)
# Target: OpenBSD/arm
- gdb_target_obs="arm-tdep.o armbsd-tdep.o armobsd-tdep.o obsd-tdep.o \
- solib-svr4.o"
+ gdb_target_obs="arm.o arm-tdep.o armbsd-tdep.o armobsd-tdep.o \
+ obsd-tdep.o solib-svr4.o"
;;
arm*-*-symbianelf*)
# Target: SymbianOS/arm
- gdb_target_obs="arm-tdep.o arm-symbian-tdep.o"
+ gdb_target_obs="arm.o arm-tdep.o arm-symbian-tdep.o"
;;
arm*-*-*)
# Target: ARM embedded system
- gdb_target_obs="arm-tdep.o"
+ gdb_target_obs="arm.o arm-tdep.o"
gdb_sim=../sim/arm/libsim.a
;;
diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index cd146f4..97e1e62 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -180,7 +180,8 @@ SFILES= $(srcdir)/gdbreplay.c $(srcdir)/inferiors.c $(srcdir)/dll.c \
$(srcdir)/common/common-debug.c $(srcdir)/common/cleanups.c \
$(srcdir)/common/common-exceptions.c $(srcdir)/symbol.c \
$(srcdir)/common/btrace-common.c \
- $(srcdir)/common/fileio.c $(srcdir)/nat/linux-namespaces.c
+ $(srcdir)/common/fileio.c $(srcdir)/nat/linux-namespaces.c \
+ $(srcdir)/arch/arm.c
DEPFILES = @GDBSERVER_DEPFILES@
@@ -583,6 +584,12 @@ fileio.o: ../common/fileio.c
$(COMPILE) $<
$(POSTCOMPILE)
+# Arch object files rules form ../arch
+
+arm.o: ../arch/arm.c
+ $(COMPILE) $<
+ $(POSTCOMPILE)
+
# Native object files rules from ../nat
x86-dregs.o: ../nat/x86-dregs.c
diff --git a/gdb/gdbserver/configure.srv b/gdb/gdbserver/configure.srv
index f187c9d..e854110 100644
--- a/gdb/gdbserver/configure.srv
+++ b/gdb/gdbserver/configure.srv
@@ -70,6 +70,7 @@ case "${target}" in
srv_regobj="${srv_regobj} arm-with-neon.o"
srv_tgtobj="$srv_linux_obj linux-arm-low.o"
srv_tgtobj="$srv_tgtobj linux-aarch32-low.o"
+ srv_tgtobj="${srv_tgtobj} arm.o"
srv_xmlfiles="arm-with-iwmmxt.xml"
srv_xmlfiles="${srv_xmlfiles} arm-with-vfpv2.xml"
srv_xmlfiles="${srv_xmlfiles} arm-with-vfpv3.xml"
diff --git a/gdb/gdbserver/linux-arm-low.c b/gdb/gdbserver/linux-arm-low.c
index 3a56620..9b0975f 100644
--- a/gdb/gdbserver/linux-arm-low.c
+++ b/gdb/gdbserver/linux-arm-low.c
@@ -30,6 +30,8 @@
#include "nat/gdb_ptrace.h"
#include <signal.h>
+#include "arch/arm.h"
+
/* Defined in auto-generated files. */
void init_registers_arm (void);
extern const struct target_desc *tdesc_arm;
@@ -80,6 +82,14 @@ typedef enum
arm_hwbp_access = 3
} arm_hwbp_type;
+/* Enum describing the different kinds of breakpoints. */
+enum arm_breakpoint_kinds
+{
+ ARM_BP_KIND_THUMB = 2,
+ ARM_BP_KIND_THUMB2 = 3,
+ ARM_BP_KIND_ARM = 4,
+};
+
/* Type describing an ARM Hardware Breakpoint Control register value. */
typedef unsigned int arm_hwbp_control_t;
@@ -237,7 +247,9 @@ arm_set_pc (struct regcache *regcache, CORE_ADDR pc)
static const unsigned long arm_breakpoint = 0xef9f0001;
#define arm_breakpoint_len 4
static const unsigned short thumb_breakpoint = 0xde01;
+#define thumb_breakpoint_len 2
static const unsigned short thumb2_breakpoint[] = { 0xf7f0, 0xa000 };
+#define thumb2_breakpoint_len 4
/* For new EABI binaries. We recognize it regardless of which ABI
is used for gdbserver, so single threaded debugging should work
@@ -913,7 +925,43 @@ arm_regs_info (void)
return ®s_info_arm;
}
-/* Implementation of linux_target_ops method "sw_breakpoint_from_kind". */
+/* Implementation of linux_target_ops method "breakpoint_kind_from_pc".
+
+ Determine the type and size of breakpoint to insert at PCPTR. Uses the
+ program counter value to determine whether a 16-bit or 32-bit breakpoint
+ should be used. It returns the breakpoint's kind, and adjusts the program
+ counter (if necessary) to point to the actual memory location where the
+ breakpoint should be inserted. */
+
+static int
+arm_breakpoint_kind_from_pc (CORE_ADDR *pcptr)
+{
+ if (IS_THUMB_ADDR (*pcptr))
+ {
+ gdb_byte buf[2];
+
+ *pcptr = UNMAKE_THUMB_ADDR (*pcptr);
+
+ /* Check whether we are replacing a thumb2 32-bit instruction. */
+ if ((*the_target->read_memory) (*pcptr, buf, 2) == 0)
+ {
+ unsigned short inst1 = 0;
+
+ (*the_target->read_memory) (*pcptr, (gdb_byte *) &inst1, 2);
+ if (thumb_insn_size (inst1) == 4)
+ {
+ return ARM_BP_KIND_THUMB2;
+ }
+ }
+ return ARM_BP_KIND_THUMB;
+ }
+ else
+ {
+ return ARM_BP_KIND_ARM;
+ }
+}
+
+/* Implementation of the linux_target_ops method "sw_breakpoint_from_kind". */
static const gdb_byte *
arm_sw_breakpoint_from_kind (int kind , int *size)
@@ -924,11 +972,25 @@ arm_sw_breakpoint_from_kind (int kind , int *size)
clone events, we will never insert a breakpoint, so even a Thumb
C library will work; so will mixing EABI/non-EABI gdbserver and
application. */
+ switch (kind)
+ {
+ case ARM_BP_KIND_THUMB:
+ *size = thumb_breakpoint_len;
+ return (gdb_byte *) &thumb_breakpoint;
+ case ARM_BP_KIND_THUMB2:
+ *size = thumb2_breakpoint_len;
+ return (gdb_byte *) &thumb2_breakpoint;
+ case ARM_BP_KIND_ARM:
+ *size = arm_breakpoint_len;
#ifndef __ARM_EABI__
- return (const gdb_byte *) &arm_breakpoint;
+ return (const gdb_byte *) &arm_breakpoint;
#else
- return (const gdb_byte *) &arm_eabi_breakpoint;
+ return (const gdb_byte *) &arm_eabi_breakpoint;
#endif
+ default:
+ return NULL;
+ }
+ return NULL;
}
struct linux_target_ops the_low_target = {
@@ -939,7 +1001,7 @@ struct linux_target_ops the_low_target = {
NULL, /* fetch_register */
arm_get_pc,
arm_set_pc,
- NULL, /* breakpoint_kind_from_pc */
+ arm_breakpoint_kind_from_pc,
arm_sw_breakpoint_from_kind,
arm_reinsert_addr,
0,
--
1.9.1
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 1/5] Add the target_ops needed for software breakpoints in GDBServer.
2015-10-20 16:48 ` [PATCH v3 1/5] Add the target_ops needed for software breakpoints " Antoine Tremblay
@ 2015-10-21 11:22 ` Pedro Alves
0 siblings, 0 replies; 20+ messages in thread
From: Pedro Alves @ 2015-10-21 11:22 UTC (permalink / raw)
To: Antoine Tremblay, gdb-patches
On 10/20/2015 05:48 PM, Antoine Tremblay wrote:
> --- a/gdb/gdbserver/linux-low.h
> +++ b/gdb/gdbserver/linux-low.h
> @@ -141,8 +141,17 @@ struct linux_target_ops
>
> CORE_ADDR (*get_pc) (struct regcache *regcache);
> void (*set_pc) (struct regcache *regcache, CORE_ADDR newpc);
> - const unsigned char *breakpoint;
> - int breakpoint_len;
> +
> + /* Return the breakpoint kind for this target based on PC. The PCPTR is
> + adjusted to the real memory location in case a flag (e.g., the Thumb bit on
> + ARM) was present in the PC. */
Spurious double space before "was present".
> + int (*breakpoint_kind_from_pc) (CORE_ADDR *pcptr);
> +
> + /* Return the software breakpoint from KIND. KIND can have target
> + specific meaning like the z0 kind parameter.
It's more usual to talk in terms of the "insert" packet. That is,
write uppercase Z0.
> --- a/gdb/gdbserver/target.h
> +++ b/gdb/gdbserver/target.h
> @@ -441,6 +441,16 @@ struct target_ops
> readlink(2). */
> ssize_t (*multifs_readlink) (int pid, const char *filename,
> char *buf, size_t bufsiz);
> +
> + /* Return the breakpoint kind for this target based on PC. The PCPTR is
> + adjusted to the real memory location in case a flag (e.g., the Thumb bit on
> + ARM) was present in the PC. */
> + int (*breakpoint_kind_from_pc) (CORE_ADDR *pcptr);
> +
> + /* Return the software breakpoint from KIND. KIND can have target
> + specific meaning like the z0 kind parameter.
> + SIZE is set to the software breakpoint's length in memory. */
> + const gdb_byte *(*sw_breakpoint_from_kind) (int kind, int *size);
Ditto.
I think the linux-low.h comments should just say "See target.h for details."
or some such, avoiding the duplication (and the forgetting to update
linux-low.h when target.h next changes).
Otherwise LGTM.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 2/5] Support breakpoint kinds for software breakpoints in GDBServer.
2015-10-20 16:49 ` [PATCH v3 2/5] Support breakpoint kinds " Antoine Tremblay
@ 2015-10-21 11:49 ` Pedro Alves
2015-10-21 14:19 ` Antoine Tremblay
2015-10-22 15:17 ` Ulrich Weigand
1 sibling, 1 reply; 20+ messages in thread
From: Pedro Alves @ 2015-10-21 11:49 UTC (permalink / raw)
To: Antoine Tremblay, gdb-patches
On 10/20/2015 05:48 PM, Antoine Tremblay wrote:
> As the kind is now a differentiating factor equivalent to size for the
> breakpoint struct and that it's size can be queryed using
(queried)
> sw_breakpoint_from_kind, the size field has been replaced with the kind field.
> All references to size are now replaced by kind or a call to bp_size that wraps
> sw_breakpoing_from_kind and returns the size of the breakpoint in memory.
>
> gdb/gdbserver/ChangeLog:
> * linux-low.c (initialize_low): Ajdust for breakpoint global variables
"Adjust"
> removal.
> * mem-break.c : Remove breakpoint_data/breakpoint_len global variables.
* mem-break.c (breakpoint_data, breakpoint_len): Delete.
> (struct raw_breakpoint) <size>: Remove.
> (struct raw_breakpoint) <kind>: Add.
> (bp_size): New function.
> (bp_opcode): Likewise.
> (find_raw_breakpoint_at): Adjust for kind.
> (insert_memory_breakpoint): Ajust for kind call bp_size,bp_opcode.
> (remove_memory_breakpoint): Ajust for kind call bp_size.
"Adjust".
> (set_raw_breakpoint_at): Adjust for kind.
> (set_breakpoint): Likewise.
> (set_breakpoint_at): Call breakpoint_kind_from_pc.
> (delete_raw_breakpoint): Adjust for kind.
> (delete_breakpoint): Likewise.
> (find_gdb_breakpoint): Likewise.
> (set_gdb_breakpoint_1): Likewise.
> (set_gdb_breakpoint): Likewise.
> (delete_gdb_breakpoint_1): Likewise.
> (delete_gdb_breakpoint): Likewise.
> (uninsert_raw_breakpoint): Likewise.
> (reinsert_raw_breakpoint): Likewise.
> (set_breakpoint_data): Remove.
> (validate_inserted_breakpoint): Adjust for kind call bp_size,bp_opcode.
> (check_mem_read): Adjust for kind call bp_size.
> (check_mem_write): Adjust for kind call bp_size,bp_opcode.
(check_mem_read): Adjust for kind. Call bp_size.
(check_mem_write): Adjust for kind. Call bp_size, bp_opcode.
> (clone_one_breakpoint): Adjust for kind.
> * mem-break.h (set_gdb_breakpoint): Likewise.
> (delete_gdb_breakpoint): Likewise.
> * server.c (process_serial_event): Likewise.
> ---
> gdb/gdbserver/linux-low.c | 10 ----
> gdb/gdbserver/mem-break.c | 150 +++++++++++++++++++++++-----------------------
> gdb/gdbserver/mem-break.h | 8 +--
> gdb/gdbserver/server.c | 8 +--
> 4 files changed, 83 insertions(+), 93 deletions(-)
>
> diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
> index 398c0aa..c20e257 100644
> --- a/gdb/gdbserver/linux-low.c
> +++ b/gdb/gdbserver/linux-low.c
> @@ -7082,20 +7082,10 @@ void
> initialize_low (void)
> {
> struct sigaction sigchld_action;
> - int breakpoint_kind = 0;
> - int breakpoint_size = 0;
> - const gdb_byte *breakpoint = NULL;
>
> memset (&sigchld_action, 0, sizeof (sigchld_action));
> set_target_ops (&linux_target_ops);
>
> - breakpoint_kind = the_target->breakpoint_kind_from_pc (NULL);
> - breakpoint = the_target->sw_breakpoint_from_kind (breakpoint_kind,
> - &breakpoint_size);
> -
> - set_breakpoint_data (breakpoint,
> - breakpoint_size);
> -
> linux_init_signals ();
> linux_ptrace_init_warnings ();
>
> diff --git a/gdb/gdbserver/mem-break.c b/gdb/gdbserver/mem-break.c
> index f077497..85ef7b0 100644
> --- a/gdb/gdbserver/mem-break.c
> +++ b/gdb/gdbserver/mem-break.c
> @@ -21,8 +21,6 @@
> #include "server.h"
> #include "regcache.h"
> #include "ax.h"
> -const unsigned char *breakpoint_data;
> -int breakpoint_len;
>
> #define MAX_BREAKPOINT_LEN 8
>
> @@ -100,8 +98,8 @@ struct raw_breakpoint
> breakpoint for a given PC. */
> CORE_ADDR pc;
>
> - /* The breakpoint's size. */
> - int size;
> + /* The breakpoint's kind. */
> + int kind;
This comment should be expanded. Something like:
/* The software breakpoint's kind. This is target specific.
Most architectures only use one specific instruction for software
breakpoints, while others may use more than one. E.g., on ARM, we
need to set different breakpoint instructions on Thumb, Thumb-2,
and ARM code. */
int kind;
> -/* Locate a GDB breakpoint of type Z_TYPE and size SIZE placed at
> - address ADDR and return a pointer to its structure. If SIZE is -1,
> - the breakpoints' sizes are ignored. */
> +/* Locate a GDB breakpoint of type Z_TYPE and kind KIND placed at
> + address ADDR and return a pointer to its structure. If KIND is -1,
> + the breakpoint's kind are ignored. */
Either:
"breakpoint's kind is ignored".
or:
"breakpoints' kinds are ignored".
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 3/5] Implement breakpoint_kind_from_pc and sw_breakpoint_from_kind for ARM in GDBServer.
2015-10-20 16:55 ` [PATCH v3 3/5] Implement breakpoint_kind_from_pc and sw_breakpoint_from_kind for ARM in GDBServer Antoine Tremblay
@ 2015-10-21 12:04 ` Pedro Alves
0 siblings, 0 replies; 20+ messages in thread
From: Pedro Alves @ 2015-10-21 12:04 UTC (permalink / raw)
To: Antoine Tremblay, gdb-patches
On 10/20/2015 05:48 PM, Antoine Tremblay wrote:
> +static int
> +arm_breakpoint_kind_from_pc (CORE_ADDR *pcptr)
> +{
> + if (IS_THUMB_ADDR (*pcptr))
> + {
> + gdb_byte buf[2];
> +
> + *pcptr = UNMAKE_THUMB_ADDR (*pcptr);
> +
> + /* Check whether we are replacing a thumb2 32-bit instruction. */
> + if ((*the_target->read_memory) (*pcptr, buf, 2) == 0)
> + {
> + unsigned short inst1 = 0;
> +
> + (*the_target->read_memory) (*pcptr, (gdb_byte *) &inst1, 2);
> + if (thumb_insn_size (inst1) == 4)
> + {
> + return ARM_BP_KIND_THUMB2;
> + }
Unnecessary braces.
> + }
> + return ARM_BP_KIND_THUMB;
> + }
> + else
> + {
> + return ARM_BP_KIND_ARM;
> + }
Unnecessary braces.
Otherwise LGTM.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 4/5] Refactor the breakpoint definitions in linux-arm-low.c.
2015-10-20 16:49 ` [PATCH v3 4/5] Refactor the breakpoint definitions in linux-arm-low.c Antoine Tremblay
@ 2015-10-21 13:04 ` Pedro Alves
0 siblings, 0 replies; 20+ messages in thread
From: Pedro Alves @ 2015-10-21 13:04 UTC (permalink / raw)
To: Antoine Tremblay, gdb-patches
On 10/20/2015 05:48 PM, Antoine Tremblay wrote:
> Before arm_breakpoint_from_pc would use an #ifdef to return the right
> arm_breakpoint from the abi or eabi breakpoint type.
>
> arm_breakpoint_at would also check for the arm_breakpoint ||
> arm_eabi_breakpoint.
>
> Thus the selected arm_breakpoint would be what arm_breakpoint_from_pc returned
> and arm_breakpoint was arm_abi_breakpoint.
>
> This patch makes it more clear by naming those for what they are : 2 separate
> entities: arm_abi_breakpoint and arm_eabi_breakpoint and set the current used
> one as arm_breakpoint.
>
> This allows a cleaner arm_breakpoint_from_pc as it just returns arm_breakpoint
> rather than having the #ifdef in that function.
>
> Any other reference to the arm_breakpoint can now also be clear of #ifdefs...
The text above needs to be updated per the new target method names.
>
> No regressions on Ubuntu 14.04 on ARMv7 and x86.
> With gdbserver-{native,extended} / { -marm -mthumb }
>
> gdb/gdbserver/ChangeLog:
> * linux-arm-low.c: Refactor breakpoint definitions.
> (arm_breakpoint_at): Adjust for arm_abi_breakpoint.
> (arm_breakpoint_from_pc): Adjust for arm_breakpoint.
Likewise.
Otherwise LGTM.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 5/5] Support software breakpoints for ARM linux in GDBServer.
2015-10-20 16:48 ` [PATCH v3 5/5] Support software breakpoints " Antoine Tremblay
@ 2015-10-21 13:12 ` Pedro Alves
0 siblings, 0 replies; 20+ messages in thread
From: Pedro Alves @ 2015-10-21 13:12 UTC (permalink / raw)
To: Antoine Tremblay, gdb-patches
On 10/20/2015 05:48 PM, Antoine Tremblay wrote:
> In this v3:
> * Removed NEWS entry
> -
>
> This patch enables software breakpoints via GDB's Z0 packets on ARM.
>
> No regressions, tested on ubuntu 14.04 ARMv7 and x86.
> With gdbserver-{native,extended} / { -marm -mthumb }
>
> gdb/gdbserver/ChangeLog:
> * linux-arm-low.c (arm_supports_z_point_type): Add software
> breakpoint support.
LGTM.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 0/5] Software breakpoints support for ARM linux in GDBServer.
2015-10-20 16:48 [PATCH v3 0/5] Software breakpoints support for ARM linux in GDBServer Antoine Tremblay
` (4 preceding siblings ...)
2015-10-20 16:55 ` [PATCH v3 3/5] Implement breakpoint_kind_from_pc and sw_breakpoint_from_kind for ARM in GDBServer Antoine Tremblay
@ 2015-10-21 13:34 ` Pedro Alves
2015-10-21 18:44 ` Yao Qi
5 siblings, 1 reply; 20+ messages in thread
From: Pedro Alves @ 2015-10-21 13:34 UTC (permalink / raw)
To: Antoine Tremblay, gdb-patches
On 10/20/2015 05:48 PM, Antoine Tremblay wrote:
> In this v3 :
>
> The main change is that the functions breakpoint_from_pc and
> breakpoint_from_kind are now called : breakpoint_kind_from_pc and
> sw_breakpoint_from_kind.
>
> As suggested in : https://sourceware.org/ml/gdb-patches/2015-10/msg00287.html
>
> Thank you Pedro
Thank you for working on this.
> the patch set is really more clear now.
I agree.
> Thus the patchset is restructured for that, patches have been renamed and
> reordered.
I replied to all patches individually, and in general this all LGTM now.
Feel free to push with the nits I pointed out fixed, if Yao is happy with
it as well.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 2/5] Support breakpoint kinds for software breakpoints in GDBServer.
2015-10-21 11:49 ` Pedro Alves
@ 2015-10-21 14:19 ` Antoine Tremblay
2015-10-21 14:23 ` Pedro Alves
0 siblings, 1 reply; 20+ messages in thread
From: Antoine Tremblay @ 2015-10-21 14:19 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
> This comment should be expanded. Something like:
>
> /* The software breakpoint's kind. This is target specific.
> Most architectures only use one specific instruction for software
> breakpoints, while others may use more than one. E.g., on ARM, we
> need to set different breakpoint instructions on Thumb, Thumb-2,
> and ARM code. */
> int kind;
>
I will remove the "software" specification since hardware breakpoints
have kinds too encoded in this struct see for example :
arm_linux_hw_point_initialize.
Like so :
/* The breakpoint's kind. This is target specific. Most
architectures only use one specific instruction for breakpoints, while
others may use more than one. E.g., on ARM, we need to set different
breakpoint instructions on Thumb, Thumb-2, and ARM code. */
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 2/5] Support breakpoint kinds for software breakpoints in GDBServer.
2015-10-21 14:19 ` Antoine Tremblay
@ 2015-10-21 14:23 ` Pedro Alves
2015-10-21 14:41 ` Antoine Tremblay
0 siblings, 1 reply; 20+ messages in thread
From: Pedro Alves @ 2015-10-21 14:23 UTC (permalink / raw)
To: Antoine Tremblay, gdb-patches
On 10/21/2015 01:03 PM, Antoine Tremblay wrote:
>
>
>> This comment should be expanded. Something like:
>>
>> /* The software breakpoint's kind. This is target specific.
>> Most architectures only use one specific instruction for software
>> breakpoints, while others may use more than one. E.g., on ARM, we
>> need to set different breakpoint instructions on Thumb, Thumb-2,
>> and ARM code. */
>> int kind;
>>
>
> I will remove the "software" specification since hardware breakpoints
> have kinds too encoded in this struct see for example :
> arm_linux_hw_point_initialize.
>
> Like so :
>
> /* The breakpoint's kind. This is target specific. Most
> architectures only use one specific instruction for breakpoints, while
> others may use more than one. E.g., on ARM, we need to set different
> breakpoint instructions on Thumb, Thumb-2, and ARM code. */
>
Ah. Hmm, but if we only remove the "software" word and continue only talking
about "instructions", then I think it ends up the same. How about extending
it like this:
/* The breakpoint's kind. This is target specific. Most
architectures only use one specific instruction for breakpoints, while
others may use more than one. E.g., on ARM, we need to use different
breakpoint instructions on Thumb, Thumb-2, and ARM code. Likewise for
hardware breakpoints -- some architectures (including ARM) need to
setup debug registers differently depending on mode. */
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 2/5] Support breakpoint kinds for software breakpoints in GDBServer.
2015-10-21 14:23 ` Pedro Alves
@ 2015-10-21 14:41 ` Antoine Tremblay
0 siblings, 0 replies; 20+ messages in thread
From: Antoine Tremblay @ 2015-10-21 14:41 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
On 10/21/2015 09:04 AM, Pedro Alves wrote:
> On 10/21/2015 01:03 PM, Antoine Tremblay wrote:
>>
>>
>>> This comment should be expanded. Something like:
>>>
>>> /* The software breakpoint's kind. This is target specific.
>>> Most architectures only use one specific instruction for software
>>> breakpoints, while others may use more than one. E.g., on ARM, we
>>> need to set different breakpoint instructions on Thumb, Thumb-2,
>>> and ARM code. */
>>> int kind;
>>>
>>
>> I will remove the "software" specification since hardware breakpoints
>> have kinds too encoded in this struct see for example :
>> arm_linux_hw_point_initialize.
>>
>> Like so :
>>
>> /* The breakpoint's kind. This is target specific. Most
>> architectures only use one specific instruction for breakpoints, while
>> others may use more than one. E.g., on ARM, we need to set different
>> breakpoint instructions on Thumb, Thumb-2, and ARM code. */
>>
>
> Ah. Hmm, but if we only remove the "software" word and continue only talking
> about "instructions", then I think it ends up the same. How about extending
> it like this:
>
> /* The breakpoint's kind. This is target specific. Most
> architectures only use one specific instruction for breakpoints, while
> others may use more than one. E.g., on ARM, we need to use different
> breakpoint instructions on Thumb, Thumb-2, and ARM code. Likewise for
> hardware breakpoints -- some architectures (including ARM) need to
> setup debug registers differently depending on mode. */
>
Sounds good to me.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 0/5] Software breakpoints support for ARM linux in GDBServer.
2015-10-21 13:34 ` [PATCH v3 0/5] Software breakpoints support for ARM linux " Pedro Alves
@ 2015-10-21 18:44 ` Yao Qi
2015-10-21 19:31 ` Antoine Tremblay
0 siblings, 1 reply; 20+ messages in thread
From: Yao Qi @ 2015-10-21 18:44 UTC (permalink / raw)
To: Pedro Alves; +Cc: Antoine Tremblay, gdb-patches
Pedro Alves <palves@redhat.com> writes:
> I replied to all patches individually, and in general this all LGTM now.
> Feel free to push with the nits I pointed out fixed, if Yao is happy with
> it as well.
With these things Pedro pointed out fixed, these patches look good to me too.
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 0/5] Software breakpoints support for ARM linux in GDBServer.
2015-10-21 18:44 ` Yao Qi
@ 2015-10-21 19:31 ` Antoine Tremblay
2015-10-22 9:59 ` Antoine Tremblay
0 siblings, 1 reply; 20+ messages in thread
From: Antoine Tremblay @ 2015-10-21 19:31 UTC (permalink / raw)
To: Yao Qi, Pedro Alves; +Cc: gdb-patches
On 10/21/2015 10:47 AM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
>
>> I replied to all patches individually, and in general this all LGTM now.
>> Feel free to push with the nits I pointed out fixed, if Yao is happy with
>> it as well.
>
> With these things Pedro pointed out fixed, these patches look good to me too.
>
This is now pushed in.
Thanks for the review!
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 0/5] Software breakpoints support for ARM linux in GDBServer.
2015-10-21 19:31 ` Antoine Tremblay
@ 2015-10-22 9:59 ` Antoine Tremblay
0 siblings, 0 replies; 20+ messages in thread
From: Antoine Tremblay @ 2015-10-22 9:59 UTC (permalink / raw)
To: Yao Qi, Pedro Alves; +Cc: gdb-patches
On 10/21/2015 11:37 AM, Antoine Tremblay wrote:
>
>
> On 10/21/2015 10:47 AM, Yao Qi wrote:
>> Pedro Alves <palves@redhat.com> writes:
>>
>>> I replied to all patches individually, and in general this all LGTM now.
>>> Feel free to push with the nits I pointed out fixed, if Yao is happy
>>> with
>>> it as well.
>>
>> With these things Pedro pointed out fixed, these patches look good to
>> me too.
>>
>
> This is now pushed in.
>
> Thanks for the review!
Hi, I just noticed this breaks tracing in some situations on x86 like
trace-break.exp fails.
Working on a fix now.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 2/5] Support breakpoint kinds for software breakpoints in GDBServer.
2015-10-20 16:49 ` [PATCH v3 2/5] Support breakpoint kinds " Antoine Tremblay
2015-10-21 11:49 ` Pedro Alves
@ 2015-10-22 15:17 ` Ulrich Weigand
2015-10-22 15:54 ` Antoine Tremblay
1 sibling, 1 reply; 20+ messages in thread
From: Ulrich Weigand @ 2015-10-22 15:17 UTC (permalink / raw)
To: Antoine Tremblay; +Cc: gdb-patches, Antoine Tremblay, palves
Antoine Tremblay wrote:
> gdb/gdbserver/ChangeLog:
> * mem-break.c : Remove breakpoint_data/breakpoint_len global variables.
> (set_breakpoint_data): Remove.
This breaks building gdbserver for spu-elf:
spu-low.o: In function `initialize_low':
gdb/gdbserver/spu-low.c:684: undefined reference to `set_breakpoint_data'
and most likely other non-Linux gdbserver targets that still refer to
set_breakpoint_data:
gdb/gdbserver/nto-low.c: set_breakpoint_data (the_low_target.breakpoint,
gdb/gdbserver/win32-low.c: set_breakpoint_data (the_low_target.breakpoint,
What needs to be done to fix those?
B.t.w. if set_breakpoint_data is really gone, its prototype should be
removed from mem-break.h as well.
Bye,
Ulrich
--
Dr. Ulrich Weigand
GNU/Linux compilers and toolchain
Ulrich.Weigand@de.ibm.com
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 2/5] Support breakpoint kinds for software breakpoints in GDBServer.
2015-10-22 15:17 ` Ulrich Weigand
@ 2015-10-22 15:54 ` Antoine Tremblay
0 siblings, 0 replies; 20+ messages in thread
From: Antoine Tremblay @ 2015-10-22 15:54 UTC (permalink / raw)
To: Ulrich Weigand; +Cc: gdb-patches, palves
On 10/22/2015 09:49 AM, Ulrich Weigand wrote:
> Antoine Tremblay wrote:
>
>> gdb/gdbserver/ChangeLog:
>> * mem-break.c : Remove breakpoint_data/breakpoint_len global variables.
>> (set_breakpoint_data): Remove.
>
> This breaks building gdbserver for spu-elf:
>
> spu-low.o: In function `initialize_low':
> gdb/gdbserver/spu-low.c:684: undefined reference to `set_breakpoint_data'
>
> and most likely other non-Linux gdbserver targets that still refer to
> set_breakpoint_data:
>
> gdb/gdbserver/nto-low.c: set_breakpoint_data (the_low_target.breakpoint,
> gdb/gdbserver/win32-low.c: set_breakpoint_data (the_low_target.breakpoint,
>
> What needs to be done to fix those?
>
Right, I need to implement the breakpoint_kind_from_pc and
breakpoint_from_kind in non-linux targets.
I'll send a patch soon.
> B.t.w. if set_breakpoint_data is really gone, its prototype should be
> removed from mem-break.h as well.
>
Indeed will patch this too.
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2015-10-22 13:55 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-20 16:48 [PATCH v3 0/5] Software breakpoints support for ARM linux in GDBServer Antoine Tremblay
2015-10-20 16:48 ` [PATCH v3 5/5] Support software breakpoints " Antoine Tremblay
2015-10-21 13:12 ` Pedro Alves
2015-10-20 16:48 ` [PATCH v3 1/5] Add the target_ops needed for software breakpoints " Antoine Tremblay
2015-10-21 11:22 ` Pedro Alves
2015-10-20 16:49 ` [PATCH v3 2/5] Support breakpoint kinds " Antoine Tremblay
2015-10-21 11:49 ` Pedro Alves
2015-10-21 14:19 ` Antoine Tremblay
2015-10-21 14:23 ` Pedro Alves
2015-10-21 14:41 ` Antoine Tremblay
2015-10-22 15:17 ` Ulrich Weigand
2015-10-22 15:54 ` Antoine Tremblay
2015-10-20 16:49 ` [PATCH v3 4/5] Refactor the breakpoint definitions in linux-arm-low.c Antoine Tremblay
2015-10-21 13:04 ` Pedro Alves
2015-10-20 16:55 ` [PATCH v3 3/5] Implement breakpoint_kind_from_pc and sw_breakpoint_from_kind for ARM in GDBServer Antoine Tremblay
2015-10-21 12:04 ` Pedro Alves
2015-10-21 13:34 ` [PATCH v3 0/5] Software breakpoints support for ARM linux " Pedro Alves
2015-10-21 18:44 ` Yao Qi
2015-10-21 19:31 ` Antoine Tremblay
2015-10-22 9:59 ` Antoine Tremblay
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 read-only IMAP folder(s) and NNTP newsgroup(s).