This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] PPC - Stepping off breakpoints in non-stop mode
> Overall, this looks good to me, I just have a few questions before we
> commit. Thanks much to Pedro for his informal review, btw.
Thanks for looking into this guys.
> Can we ditch the PPC_MAX_INSN_LEN macro and only use PPC_INSN_SIZE?
> Right now, you define the latter but never use it. Right now, you only
> use PPC_MAX_INSN_LEN, which I like less than PPC_INSN_SIZE, because
> AFAIK all instructions on powerpc are the same size.
Yes, this makes sense. I've changed the code to use only PPC_INSN_SIZE.
> Why did you move these macros to the .h file? Right now, they are only
> used inside rs6000-tdep.c, so we could keep them there.
> Same for these macros, we could certainly keep them inside rs6000-tdep.c.
> I'm not opposed to having them in the .h file, if you prefer it that
> way. But I generally prefer to keep definitions inside the .c file if
> they are not used elsewhere. That way, I instantly know that this macro
> is only used within that unit.
Defines and macros looked better for me in header files since they're
almost always constant values, but i didn't think of it the way you
mentioned. It's a good point. They're back to the .c file.
> I don't think you need to use a gdb_byte in this case. Wouldn't it
> be simpler to use an int? Hopefully, that will allow you to avoid
> this cast. Otherwise, it looks like this constant is only used once
> in your code:
>
> if (link_register_bit)
>
> So another alternative is is to embed it inside the "if" statement
> with a comment. Something like this:
Fixed as well.
How does it look now?
Thanks,
Luis
2008-06-25 Luis Machado <luisgpm@br.ibm.com>
* rs6000-tdep.c (ppc_displaced_step_fixup): New function.
(deal_with_atomic_sequence): Update BC masks.
(rs6000_gdbarch_init): Init displaced stepping infra-structure.
Define BRANCH_MASK, B_INSN, BC_INSN, BXL_INSN, BP_MASK and BP_INSN.
Index: gdb/rs6000-tdep.c
===================================================================
--- gdb.orig/rs6000-tdep.c 2008-06-23 05:13:22.000000000 -0700
+++ gdb/rs6000-tdep.c 2008-06-25 06:20:02.000000000 -0700
@@ -841,6 +841,101 @@
return little_breakpoint;
}
+/* Instruction masks for displaced stepping. */
+#define BRANCH_MASK 0xfc000000
+#define BP_MASK 0xFC0007FE
+#define B_INSN 0x48000000
+#define BC_INSN 0x40000000
+#define BXL_INSN 0x4c000000
+#define BP_INSN 0x7C000008
+
+/* Fix up the state of registers and memory after having single-stepped
+ a displaced instruction. */
+void
+ppc_displaced_step_fixup (struct gdbarch *gdbarch,
+ struct displaced_step_closure *closure,
+ CORE_ADDR from, CORE_ADDR to,
+ struct regcache *regs)
+{
+ /* Since we use simple_displaced_step_copy_insn, our closure is a
+ copy of the instruction. */
+ ULONGEST insn = extract_unsigned_integer ((gdb_byte *) closure,
+ PPC_INSN_SIZE);
+ ULONGEST opcode = 0;
+ /* Offset for non-PC relative instructions. */
+ LONGEST offset = PPC_INSN_SIZE;
+
+ opcode = insn & BRANCH_MASK;
+
+ if (debug_displaced)
+ fprintf_unfiltered (gdb_stdlog,
+ "displaced: (ppc) fixup (0x%s, 0x%s)\n",
+ paddr_nz (from), paddr_nz (to));
+
+
+ /* Handle PC-relative branch instructions. */
+ if (opcode == B_INSN || opcode == BC_INSN || opcode == BXL_INSN)
+ {
+ CORE_ADDR current_pc;
+
+ /* Read the current PC value after the instruction has been executed
+ in a displaced location. Calculate the offset to be applied to the
+ original PC value before the displaced stepping. */
+ regcache_cooked_read_unsigned (regs, gdbarch_pc_regnum (gdbarch),
+ ¤t_pc);
+ offset = current_pc - to;
+
+ if (opcode != BXL_INSN)
+ {
+ if (!(insn & 0x2))
+ {
+ /* AA bit indicating whether this is an absolute addressing or
+ PC-relative. */
+ if (debug_displaced)
+ fprintf_unfiltered (gdb_stdlog,
+ "displaced: (ppc) branch instruction: 0x%s\n"
+ "displaced: (ppc) adjusted PC from 0x%s to 0x%s\n",
+ paddr_nz (insn), paddr_nz (current_pc),
+ paddr_nz (from + offset));
+
+ regcache_cooked_write_unsigned (regs, gdbarch_pc_regnum (gdbarch),
+ from + offset);
+ }
+ }
+ else
+ {
+ /* If we're here, it means we have a branch to LR or CTR. If the
+ branch was taken, the offset is probably greater than 4 (the next
+ instruction), so it's safe to assume that an offset of 4 means we
+ did not take the branch. */
+ if (offset == PPC_INSN_SIZE)
+ regcache_cooked_write_unsigned (regs, gdbarch_pc_regnum (gdbarch),
+ from + PPC_INSN_SIZE);
+ }
+
+ if (insn & 0x1)
+ {
+ /* LK bit Indicates whether we should set the link register to point
+ to the next instruction or not. */
+ regcache_cooked_write_unsigned (regs,
+ gdbarch_tdep (gdbarch)->ppc_lr_regnum,
+ from + PPC_INSN_SIZE);
+ if (debug_displaced)
+ fprintf_unfiltered (gdb_stdlog,
+ "displaced: (ppc) adjusted LR to 0x%s\n",
+ paddr_nz (from + PPC_INSN_SIZE));
+
+ }
+ }
+ /* Check for breakpoints in the inferior. If we've found one, place the PC
+ right at the breakpoint instruction. */
+ else if ((insn & BP_MASK) == BP_INSN)
+ regcache_cooked_write_unsigned (regs, gdbarch_pc_regnum (gdbarch), from);
+ else
+ /* Handle any other instructions that do not fit in the categories above. */
+ regcache_cooked_write_unsigned (regs, gdbarch_pc_regnum (gdbarch),
+ from + offset);
+}
/* Instruction masks used during single-stepping of atomic sequences. */
#define LWARX_MASK 0xfc0007fe
@@ -849,8 +944,6 @@
#define STWCX_MASK 0xfc0007ff
#define STWCX_INSTRUCTION 0x7c00012d
#define STDCX_INSTRUCTION 0x7c0001ad
-#define BC_MASK 0xfc000000
-#define BC_INSTRUCTION 0x40000000
/* Checks for an atomic sequence of instructions beginning with a LWARX/LDARX
instruction and ending with a STWCX/STDCX instruction. If such a sequence
@@ -887,7 +980,7 @@
/* 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 & BC_MASK) == BC_INSTRUCTION)
+ if ((insn & BRANCH_MASK) == BC_INSN)
{
int immediate = ((insn & ~3) << 16) >> 16;
int absolute = ((insn >> 1) & 1);
@@ -3214,6 +3307,17 @@
/* Put the _Decimal128 pseudo-registers after the SPE registers. */
tdep->ppc_dl0_regnum += 32;
+ /* Setup displaced stepping. */
+ set_gdbarch_displaced_step_copy_insn (gdbarch,
+ simple_displaced_step_copy_insn);
+ set_gdbarch_displaced_step_fixup (gdbarch, ppc_displaced_step_fixup);
+ set_gdbarch_displaced_step_free_closure (gdbarch,
+ simple_displaced_step_free_closure);
+ set_gdbarch_displaced_step_location (gdbarch,
+ displaced_step_at_entry_point);
+
+ set_gdbarch_max_insn_length (gdbarch, PPC_INSN_SIZE);
+
return gdbarch;
}