This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 2/3] Support up to 3 conditional branches in an atomic sequence
- From: Joel Brobecker <brobecker at adacore dot com>
- To: Anton Blanchard <anton at samba dot org>
- Cc: gdb-patches at sourceware dot org
- Date: Fri, 28 Sep 2012 12:44:31 +0200
- Subject: Re: [PATCH 2/3] Support up to 3 conditional branches in an atomic sequence
- References: <20120606135557.7da37cbe@kryten> <20120606135655.57bd5b54@kryten> <20120613160208.GH18729@adacore.com> <20120716163355.74acba34@kryten>
> Here is a new version.
[...]
> 2012-06-05 Anton Blanchard <anton@samba.org>
>
> * gdb/breakpoint.h: Define MAX_SINGLE_STEP_BREAKPOINTS
> * rs6000-tdep.c (ppc_deal_with_atomic_sequence): Allow for more
> than two breakpoints.
> * gdb/breakpoint.c (insert_single_step_breakpoint): Likewise
> (insert_single_step_breakpoint): Likewise
> (single_step_breakpoints_inserted): Likewise
> (cancel_single_step_breakpoints): Likewise
> (detach_single_step_breakpoints): Likewise
> (single_step_breakpoint_inserted_here_p): Likewise
Thanks for sending the new version, and sorry for dropping the ball on
this one.
This version of the patch is OK.
> Index: b/gdb/rs6000-tdep.c
> ===================================================================
> --- a/gdb/rs6000-tdep.c
> +++ b/gdb/rs6000-tdep.c
> @@ -1087,7 +1087,7 @@ ppc_deal_with_atomic_sequence (struct fr
> struct address_space *aspace = get_frame_address_space (frame);
> enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> CORE_ADDR pc = get_frame_pc (frame);
> - CORE_ADDR breaks[2] = {-1, -1};
> + CORE_ADDR breaks[MAX_SINGLE_STEP_BREAKPOINTS];
> CORE_ADDR loc = pc;
> CORE_ADDR closing_insn; /* Instruction that closes the atomic sequence. */
> int insn = read_memory_integer (loc, PPC_INSN_SIZE, byte_order);
> @@ -1096,7 +1096,6 @@ ppc_deal_with_atomic_sequence (struct fr
> int last_breakpoint = 0; /* Defaults to 0 (no breakpoints placed). */
> const int atomic_sequence_length = 16; /* Instruction sequence length. */
> int opcode; /* Branch instruction's OPcode. */
> - int bc_insn_count = 0; /* Conditional branch instruction count. */
>
> /* Assume all atomic sequences start with a lwarx/ldarx instruction. */
> if ((insn & LWARX_MASK) != LWARX_INSTRUCTION
> @@ -1110,24 +1109,20 @@ ppc_deal_with_atomic_sequence (struct fr
> loc += PPC_INSN_SIZE;
> insn = read_memory_integer (loc, PPC_INSN_SIZE, byte_order);
>
> - /* Assume that there is at most one conditional branch in the atomic
> - sequence. If a conditional branch is found, put a breakpoint in
> - its destination address. */
> if ((insn & BRANCH_MASK) == BC_INSN)
> {
> int immediate = ((insn & 0xfffc) ^ 0x8000) - 0x8000;
> int absolute = insn & 2;
>
> - if (bc_insn_count >= 1)
> - return 0; /* More than one conditional branch found, fallback
> + if (last_breakpoint >= MAX_SINGLE_STEP_BREAKPOINTS - 1)
> + return 0; /* too many conditional branches found, fallback
> to the standard single-step code. */
>
> if (absolute)
> - breaks[1] = immediate;
> + breaks[last_breakpoint] = immediate;
> else
> - breaks[1] = loc + immediate;
> + breaks[last_breakpoint] = loc + immediate;
>
> - bc_insn_count++;
> last_breakpoint++;
> }
>
> @@ -1146,18 +1141,29 @@ ppc_deal_with_atomic_sequence (struct fr
> insn = read_memory_integer (loc, PPC_INSN_SIZE, byte_order);
>
> /* Insert a breakpoint right after the end of the atomic sequence. */
> - breaks[0] = loc;
> + breaks[last_breakpoint] = loc;
>
> - /* Check for duplicated breakpoints. Check also for a breakpoint
> - placed (branch instruction's destination) anywhere in sequence. */
> - if (last_breakpoint
> - && (breaks[1] == breaks[0]
> - || (breaks[1] >= pc && breaks[1] <= closing_insn)))
> - last_breakpoint = 0;
> -
> - /* Effectively inserts the breakpoints. */
> for (index = 0; index <= last_breakpoint; index++)
> - insert_single_step_breakpoint (gdbarch, aspace, breaks[index]);
> + {
> + int index2;
> + int insert_bp = 1;
> +
> + /* Check for a breakpoint placed (branch instruction's destination)
> + anywhere in sequence. */
> + if (breaks[index] >= pc && breaks[index] <= closing_insn)
> + continue;
> +
> + /* Check for duplicated breakpoints. */
> + for (index2 = 0; index2 < index; index2++)
> + {
> + if (breaks[index] == breaks[index2])
> + insert_bp = 0;
> + }
> +
> + /* insert the breakpoint. */
> + if (insert_bp)
> + insert_single_step_breakpoint (gdbarch, aspace, breaks[index]);
> + }
>
> return 1;
> }
> Index: b/gdb/breakpoint.c
> ===================================================================
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -14741,11 +14741,10 @@ deprecated_remove_raw_breakpoint (struct
> return ret;
> }
>
> -/* One (or perhaps two) breakpoints used for software single
> - stepping. */
> +/* Breakpoints used for software single stepping. */
>
> -static void *single_step_breakpoints[2];
> -static struct gdbarch *single_step_gdbarch[2];
> +static void *single_step_breakpoints[MAX_SINGLE_STEP_BREAKPOINTS];
> +static struct gdbarch *single_step_gdbarch[MAX_SINGLE_STEP_BREAKPOINTS];
>
> /* Create and insert a breakpoint for software single step. */
>
> @@ -14754,19 +14753,17 @@ insert_single_step_breakpoint (struct gd
> struct address_space *aspace,
> CORE_ADDR next_pc)
> {
> + int i;
> void **bpt_p;
>
> - if (single_step_breakpoints[0] == NULL)
> - {
> - bpt_p = &single_step_breakpoints[0];
> - single_step_gdbarch[0] = gdbarch;
> - }
> - else
> - {
> - gdb_assert (single_step_breakpoints[1] == NULL);
> - bpt_p = &single_step_breakpoints[1];
> - single_step_gdbarch[1] = gdbarch;
> - }
> + for (i = 0; i < MAX_SINGLE_STEP_BREAKPOINTS; i++)
> + if (single_step_breakpoints[i] == NULL)
> + break;
> +
> + gdb_assert (i < MAX_SINGLE_STEP_BREAKPOINTS);
> +
> + bpt_p = &single_step_breakpoints[i];
> + single_step_gdbarch[i] = gdbarch;
>
> /* NOTE drow/2006-04-11: A future improvement to this function would
> be to only create the breakpoints once, and actually put them on
> @@ -14787,8 +14784,13 @@ insert_single_step_breakpoint (struct gd
> int
> single_step_breakpoints_inserted (void)
> {
> - return (single_step_breakpoints[0] != NULL
> - || single_step_breakpoints[1] != NULL);
> + int i;
> +
> + for (i = 0; i < MAX_SINGLE_STEP_BREAKPOINTS; i++)
> + if (single_step_breakpoints[i] != NULL)
> + return 1;
> +
> + return 0;
> }
>
> /* Remove and delete any breakpoints used for software single step. */
> @@ -14796,22 +14798,21 @@ single_step_breakpoints_inserted (void)
> void
> remove_single_step_breakpoints (void)
> {
> + int i;
> +
> gdb_assert (single_step_breakpoints[0] != NULL);
>
> /* See insert_single_step_breakpoint for more about this deprecated
> call. */
> - deprecated_remove_raw_breakpoint (single_step_gdbarch[0],
> - single_step_breakpoints[0]);
> - single_step_gdbarch[0] = NULL;
> - single_step_breakpoints[0] = NULL;
>
> - if (single_step_breakpoints[1] != NULL)
> - {
> - deprecated_remove_raw_breakpoint (single_step_gdbarch[1],
> - single_step_breakpoints[1]);
> - single_step_gdbarch[1] = NULL;
> - single_step_breakpoints[1] = NULL;
> - }
> + for (i = 0; i < MAX_SINGLE_STEP_BREAKPOINTS; i++)
> + if (single_step_breakpoints[i] != NULL)
> + {
> + deprecated_remove_raw_breakpoint (single_step_gdbarch[i],
> + single_step_breakpoints[i]);
> + single_step_gdbarch[i] = NULL;
> + single_step_breakpoints[i] = NULL;
> + }
> }
>
> /* Delete software single step breakpoints without removing them from
> @@ -14824,7 +14825,7 @@ cancel_single_step_breakpoints (void)
> {
> int i;
>
> - for (i = 0; i < 2; i++)
> + for (i = 0; i < MAX_SINGLE_STEP_BREAKPOINTS; i++)
> if (single_step_breakpoints[i])
> {
> xfree (single_step_breakpoints[i]);
> @@ -14841,7 +14842,7 @@ detach_single_step_breakpoints (void)
> {
> int i;
>
> - for (i = 0; i < 2; i++)
> + for (i = 0; i < MAX_SINGLE_STEP_BREAKPOINTS; i++)
> if (single_step_breakpoints[i])
> target_remove_breakpoint (single_step_gdbarch[i],
> single_step_breakpoints[i]);
> @@ -14856,7 +14857,7 @@ single_step_breakpoint_inserted_here_p (
> {
> int i;
>
> - for (i = 0; i < 2; i++)
> + for (i = 0; i < MAX_SINGLE_STEP_BREAKPOINTS; i++)
> {
> struct bp_target_info *bp_tgt = single_step_breakpoints[i];
> if (bp_tgt
> Index: b/gdb/breakpoint.h
> ===================================================================
> --- a/gdb/breakpoint.h
> +++ b/gdb/breakpoint.h
> @@ -1408,8 +1408,10 @@ extern int is_catchpoint (struct breakpo
> deletes all breakpoints. */
> extern void delete_command (char *arg, int from_tty);
>
> -/* Manage a software single step breakpoint (or two). Insert may be
> - called twice before remove is called. */
> +/* Manage software single step breakpoints. Insert may be
> + called multiple times before remove is called. */
> +#define MAX_SINGLE_STEP_BREAKPOINTS 4
> +
> extern void insert_single_step_breakpoint (struct gdbarch *,
> struct address_space *,
> CORE_ADDR);
--
Joel