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

Emi SUZUKI
2007-02-27T13:00:49Z

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

Luis,

Thank you for testing and sorry for being late to be back on this.

From: Luis Machado <luisgpm at linux.vnet.ibm.com>
Subject: Re: [patch] "single step" atomic instruction sequences as a whole.
Date: Sat, 17 Feb 2007 00:23:19 -0200

> So far so good. But if we continue executing the program we will
> eventually reach a set of instructions like this one:
>
> 0x40000119ae8 <._IO_puts+312>: lwarx r9,0,r3
> 0x40000119aec <._IO_puts+316>: stwcx. r0,0,r3
> 0x40000119af0 <._IO_puts+320>: bne+ 0x40000119ae8 <._IO_puts+312>
>
> At this point, GDB keeps stepping through the lwarx and stwcx
> instructions endlessly. If i hit continue right after the first
> breakpoint (0x40000119a08), the program ends normally
>
> I tested the same instruction block with Paul's patch and this did not
> happen, stating that it skipped the atomic instruction set as usual, and
> ending the program.

I have investigeted it and found that the code in my patch cannot
detect the region to skip from the sequence of instructions mentioned
above.
But, in fact, I have diverted that part of codes from Paul's one :-(
I have been misguided by some useless codes in that patch... Yes, I
should have made myself understand more deeply about its behavior.

Anyway, I have looked anew at that part of codes and improved it.
Please try the attached version of patchset, if you don't mind.
It's for CVS head.

Best regards,
--
Emi SUZUKI

[-- Attachment #2: rs6000-atomic-sequence_070227.diff --]
[-- Type: Text/Plain, Size: 7612 bytes --]

diff -ruN -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-27 21:52:46.000000000 +0900
@@ -90,3 +90,9 @@

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 -ruN -x CVS src/gdb/infrun.c gdb/gdb/infrun.c
--- src/gdb/infrun.c 2007-02-27 21:34:23.000000000 +0900
+++ gdb/gdb/infrun.c 2007-02-27 21:28:54.000000000 +0900
@@ -556,15 +556,19 @@
if (breakpoint_here_p (read_pc ()) == permanent_breakpoint_here)
SKIP_PERMANENT_BREAKPOINT ();

- if (SOFTWARE_SINGLE_STEP_P () && step)
+ if (step)
{
- /* Do it the hard way, w/temp breakpoints */
- SOFTWARE_SINGLE_STEP (sig, 1 /*insert-breakpoints */ );
- /* ...and don't ask hardware to do it. */
- step = 0;
- /* and do not pull these breakpoints until after a `wait' in
- `wait_for_inferior' */
- singlestep_breakpoints_inserted_p = 1;
+ if (SOFTWARE_SINGLE_STEP_P ())
+ {
+ /* Do it the hard way, w/temp breakpoints */
+ SOFTWARE_SINGLE_STEP (sig, 1 /*insert-breakpoints */ );
+ /* ...and don't ask hardware to do it. */
+ step = 0;
+ /* 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 ();
}
@@ -1566,8 +1570,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));

@@ -1580,9 +1582,13 @@
{
if (debug_infrun)
fprintf_unfiltered (gdb_stdlog, "infrun: stepping_past_singlestep_breakpoint\n");
- /* Pull the single step breakpoints out of the target. */
- SOFTWARE_SINGLE_STEP (0, 0);
- singlestep_breakpoints_inserted_p = 0;
+
+ 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;

@@ -1616,7 +1622,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. */
@@ -1687,7 +1693,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);
@@ -1736,7 +1742,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;
@@ -1758,7 +1764,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 -ruN -x CVS src/gdb/rs6000-tdep.c gdb/gdb/rs6000-tdep.c
--- src/gdb/rs6000-tdep.c 2007-02-21 12:01:58.000000000 +0900
+++ gdb/gdb/rs6000-tdep.c 2007-02-27 21:00:20.000000000 +0900
@@ -701,6 +701,64 @@
return little_breakpoint;
}

+#define LWARX_MASK 0xfc0007fe
+#define LWARX_INSTRUCTION 0x7C000028
+#define LDARX_INSTRUCTION 0x7C000108
+#define STWCX_MASK 0xfc0007ff
+#define STWCX_INSTRUCTION 0x7c00012d
+#define STDCX_INSTRUCTION 0x7c0001ad
+#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 loc = pc;
+ int insn = read_memory_integer (loc, PPC_INSN_SIZE);
+ int i;
+
+ /* Assume all atomic sequences start with an lwarx instruction. */
+ if ((insn & LWARX_MASK) != LWARX_INSTRUCTION
+ && (insn & LWARX_MASK) != LDARX_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);
+
+ /* At most one conditional branch instuction appears between
+ the lwarx/ldarx and stwcx/stdcx instructions. But its target
+ address should be where the second conditional branch goes
+ when the branch is not taken. */
+ if ((insn & STWCX_MASK) == STWCX_INSTRUCTION
+ || (insn & STWCX_MASK) == STDCX_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."));
+
+ return loc;
+}
+
+/* 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,15 +778,29 @@
{
loc = read_pc ();

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

- breaks[0] = loc + breakp_sz;
- opcode = insn >> 26;
- breaks[1] = branch_dest (opcode, insn, loc, breaks[0]);
-
- /* Don't put two breakpoints on the same address. */
- if (breaks[1] == breaks[0])
- breaks[1] = -1;
+ 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]);
+
+ /* Don't put two breakpoints on the same address. */
+ if (breaks[1] == breaks[0])
+ breaks[1] = -1;
+ }

for (ii = 0; ii < 2; ++ii)
{
@@ -3446,6 +3518,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
@@ -3530,3 +3603,4 @@
gdbarch_register (bfd_arch_rs6000, rs6000_gdbarch_init, rs6000_dump_tdep);
gdbarch_register (bfd_arch_powerpc, rs6000_gdbarch_init, rs6000_dump_tdep);
}
+