Re: [patch] "single step" atomic instruction sequences as a whole.

Emi SUZUKI
2007-02-06T12:11:49Z

[-- Attachment #1: Type: Text/Plain, Size: 1709 bytes --]

Hello Luis and members,

I am working on Linux running on PlayStation 3, and on Fedora Core 5
for PowerPC running on our own hardware equipped with 'Cell' CPU.
I have also been annoyed by this problem for months, because the main
core of Cell is PowerPC.

As far as I know, the patch by Paul has a problem mentioned by the
messages below:
http://sourceware.org/ml/gdb-patches/2006-09/msg00060.html
http://sourceware.org/ml/gdb-patches/2006-09/msg00097.html
http://sourceware.org/ml/gdb-patches/2006-11/msg00049.html
http://sourceware.org/ml/gdb-patches/2006-11/msg00074.html

And the idea is a bit confusing for me...

I just thought that SOFTWARE_SINGLE_STEP_P should indicate if the
target will do software single-stepping at the next time it proceed.
And all the architectures which should skip atomic sequences of
instructions by themselves need to implement are
SOFTWARE_SINGLE_STEP_P to check if the skipping is needed, and
SOFTWARE_SINGLE_STEP to actually skip the sequence.
# I remember someone said that ARM and MIPS have a similar problem.

As a effect of the change above, the return value of SOFTWARE_SINGLE_STEP_P
cannot be used as the condition of thread hopping check or removing
a software single-stepping breakpoint on handle_inferior_event: it
may not be welcomed.

Anyway, I will attach a patch applied on our environment. It's for
CVS head.
Since it's my first time of posting here, any comments are very
appreciated.

P.S.
I have not completed the legal process of copyright assignment, and
unfortunately the boss said to me that it will take months...
Please point me if the patch can be applied and surely needs an
assignment, I will make them hurry.
--
Emi SUZUKI

[-- Attachment #2: rs6000-skip-atomic-sequnce.diff --]
[-- Type: Text/Plain, Size: 6894 bytes --]

diff -uBbEw -r -x CVS src/gdb/config/rs6000/tm-rs6000.h gdb/gdb/config/rs6000/tm-rs6000.h
--- src/gdb/config/rs6000/tm-rs6000.h 2007-01-10 02:59:05.000000000 +0900
+++ gdb/gdb/config/rs6000/tm-rs6000.h 2007-02-06 15:39:31.000000000 +0900
@@ -90,3 +90,8 @@

extern void (*rs6000_set_host_arch_hook) (int);

+extern int rs6000_software_single_step_p (void);
+#ifdef SOFTWARE_SINGLE_STEP_P
+#undef SOFTWARE_SINGLE_STEP_P
+#endif
+#define SOFTWARE_SINGLE_STEP_P() rs6000_software_single_step_p()
diff -uBbEw -r -x CVS src/gdb/infrun.c gdb/gdb/infrun.c
--- src/gdb/infrun.c 2007-01-11 05:10:23.000000000 +0900
+++ gdb/gdb/infrun.c 2007-02-06 15:41:00.000000000 +0900
@@ -556,7 +556,9 @@
if (breakpoint_here_p (read_pc ()) == permanent_breakpoint_here)
SKIP_PERMANENT_BREAKPOINT ();

- if (SOFTWARE_SINGLE_STEP_P () && step)
+ if (step)
+ {
+ if (SOFTWARE_SINGLE_STEP_P ())
{
/* Do it the hard way, w/temp breakpoints */
SOFTWARE_SINGLE_STEP (sig, 1 /*insert-breakpoints */ );
@@ -565,6 +567,8 @@
/* and do not pull these breakpoints until after a `wait' in
`wait_for_inferior' */
singlestep_breakpoints_inserted_p = 1;
+ }
+
singlestep_ptid = inferior_ptid;
singlestep_pc = read_pc ();
}
@@ -1570,8 +1574,6 @@

if (stepping_past_singlestep_breakpoint)
{
- gdb_assert (SOFTWARE_SINGLE_STEP_P ()
- && singlestep_breakpoints_inserted_p);
gdb_assert (ptid_equal (singlestep_ptid, ecs->ptid));
gdb_assert (!ptid_equal (singlestep_ptid, saved_singlestep_ptid));

@@ -1584,9 +1586,13 @@
{
if (debug_infrun)
fprintf_unfiltered (gdb_stdlog, "infrun: stepping_past_singlestep_breakpoint\n");
+
+ if (singlestep_breakpoints_inserted_p)
+ {
/* Pull the single step breakpoints out of the target. */
SOFTWARE_SINGLE_STEP (0, 0);
singlestep_breakpoints_inserted_p = 0;
+ }

ecs->random_signal = 0;

@@ -1620,7 +1626,7 @@
if (!breakpoint_thread_match (stop_pc, ecs->ptid))
thread_hop_needed = 1;
}
- else if (SOFTWARE_SINGLE_STEP_P () && singlestep_breakpoints_inserted_p)
+ else if (singlestep_breakpoints_inserted_p)
{
/* We have not context switched yet, so this should be true
no matter which thread hit the singlestep breakpoint. */
@@ -1691,7 +1697,7 @@
/* Saw a breakpoint, but it was hit by the wrong thread.
Just continue. */

- if (SOFTWARE_SINGLE_STEP_P () && singlestep_breakpoints_inserted_p)
+ /* if (singlestep_breakpoints_inserted_p) */
{
/* Pull the single step breakpoints out of the target. */
SOFTWARE_SINGLE_STEP (0, 0);
@@ -1740,7 +1746,7 @@
return;
}
}
- else if (SOFTWARE_SINGLE_STEP_P () && singlestep_breakpoints_inserted_p)
+ else if (singlestep_breakpoints_inserted_p)
{
sw_single_step_trap_p = 1;
ecs->random_signal = 0;
@@ -1762,7 +1768,7 @@
deprecated_context_hook (pid_to_thread_id (ecs->ptid));
}

- if (SOFTWARE_SINGLE_STEP_P () && singlestep_breakpoints_inserted_p)
+ if (singlestep_breakpoints_inserted_p)
{
/* Pull the single step breakpoints out of the target. */
SOFTWARE_SINGLE_STEP (0, 0);
diff -uBbEw -r -x CVS src/gdb/rs6000-tdep.c gdb/gdb/rs6000-tdep.c
--- src/gdb/rs6000-tdep.c 2007-01-10 02:58:57.000000000 +0900
+++ gdb/gdb/rs6000-tdep.c 2007-02-06 15:40:03.000000000 +0900
@@ -701,6 +701,79 @@
return little_breakpoint;
}

+#define LWARX_MASK 0xfc0007fe
+#define LWARX_INSTRUCTION 0x7C000028
+#define STWCX_MASK 0xfc0007ff
+#define STWCX_INSTRUCTION 0x7c00012d
+#define BC_MASK 0xfc000000
+#define BC_INSTRUCTION 0x40000000
+#define IMMEDIATE_PART(insn) (((insn & ~3) << 16) >> 16)
+#define ABSOLUTE_P(insn) ((int) ((insn >> 1) & 1))
+
+CORE_ADDR
+rs6000_deal_with_atomic_sequence (CORE_ADDR pc)
+{
+ CORE_ADDR breaks[2] = {-1, -1};
+ CORE_ADDR loc = pc;
+ int insn = read_memory_integer (loc, PPC_INSN_SIZE);
+ int last_break = 0;
+ int i;
+
+ /* Assume all atomic sequences start with an lwarx instruction. */
+ if ((insn & LWARX_MASK) != LWARX_INSTRUCTION)
+ return -1;
+
+ /* Assume that no atomic sequence is longer than 6 instructions. */
+ for (i= 1; i < 5; ++i)
+ {
+ loc += PPC_INSN_SIZE;
+ insn = read_memory_integer (loc, PPC_INSN_SIZE);
+
+ /* Assume at most one conditional branch instruction between
+ the lwarx and stwcx instructions.*/
+ if ((insn & BC_MASK) == BC_INSTRUCTION)
+ {
+ last_break = 1;
+ breaks[1] = IMMEDIATE_PART (insn);
+ if ( ! ABSOLUTE_P(insn))
+ breaks[1] += loc;
+ continue;
+ }
+
+ if ((insn & STWCX_MASK) == STWCX_INSTRUCTION)
+ break;
+ }
+
+ /* Assume that the atomic sequence ends with a stwcx instruction
+ followed by a conditional branch instruction. */
+ if ((insn & STWCX_MASK) != STWCX_INSTRUCTION)
+ error (_("Tried to step over an atomic sequence of instructions but could not find the end of the sequence."));
+
+ loc += PPC_INSN_SIZE;
+ insn = read_memory_integer (loc, PPC_INSN_SIZE);
+
+ if ((insn & BC_MASK) != BC_INSTRUCTION)
+ error (_("Tried to step over an atomic sequence of instructions but it did not end as expected."));
+
+ breaks[0] = loc;
+
+ /* This should never happen, but make sure we don't but
+ two breakpoints on the same address. */
+ if (last_break && breaks[1] == breaks[0])
+ last_break = 0;
+
+ if (last_break == 0)
+ return -1;
+
+ return breaks[0];
+}
+
+/* SOFTWARE_SINGLE_STEP_P */
+int
+rs6000_software_single_step_p (void)
+{
+ return (rs6000_deal_with_atomic_sequence (read_pc ()) != -1);
+}

/* AIX does not support PT_STEP. Simulate it. */

@@ -720,8 +793,21 @@
{
loc = read_pc ();

- insn = read_memory_integer (loc, 4);
+ /* check if running on atomic sequence */
+ breaks[0] = rs6000_deal_with_atomic_sequence (loc);

+ if (breaks[0] != -1)
+ {
+ printf_unfiltered (_("Stepping over an atomic sequence of instructions. \n\
+Beginning at %s, break at %s next time.\n"),
+ core_addr_to_string (loc),
+ core_addr_to_string (breaks[0]));
+ gdb_flush (gdb_stdout);
+ breaks[1] = -1;
+ }
+ else
+ {
+ insn = read_memory_integer (loc, PPC_INSN_SIZE);
breaks[0] = loc + breakp_sz;
opcode = insn >> 26;
breaks[1] = branch_dest (opcode, insn, loc, breaks[0]);
@@ -729,6 +815,7 @@
/* Don't put two breakpoints on the same address. */
if (breaks[1] == breaks[0])
breaks[1] = -1;
+ }

for (ii = 0; ii < 2; ++ii)
{
@@ -3413,6 +3500,7 @@

set_gdbarch_inner_than (gdbarch, core_addr_lessthan);
set_gdbarch_breakpoint_from_pc (gdbarch, rs6000_breakpoint_from_pc);
+ set_gdbarch_software_single_step (gdbarch, rs6000_software_single_step);

/* Handle the 64-bit SVR4 minimal-symbol convention of using "FN"
for the descriptor and ".FN" for the entry-point -- a user