[PATCH v3 2/5] Support breakpoint kinds for software breakpoints in GDBServer.

Antoine Tremblay
2015-10-20T16:49:03Z

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