This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[rfc][6/13] Eliminate read_register: read_register (invalidly) used in prologue analyzers
- From: "Ulrich Weigand" <uweigand at de dot ibm dot com>
- To: gdb-patches at sourceware dot org
- Date: Thu, 7 Jun 2007 22:59:14 +0200 (CEST)
- Subject: [rfc][6/13] Eliminate read_register: read_register (invalidly) used in prologue analyzers
Hello,
while eliminating the read_register family of routines, I noticed a couple
of places where read_register is currently used in what appears to be an
invalid fashion as part of prologue analyzer routines.
Those routines are generally called from two places: the unwinder, in which
case you should examine register values from the next frame instead of
referring to global state, and the skip_prologue callback, which doesn't
refer to *any* frame, but the global register state is invalid as well,
since the callback can be invoked at any time (e.g. when re-evaluating
breakpoints).
The three problematic locations I found are:
- v850_analyze_prologue uses read_register to retrieve the CTBP register
(call table base pointer). As this is used *only* from within the
unwinder, the patch adds a CTBP argument to v850_analyze_prologue and
has the unwinder pass in the unwound value of the register.
- Similarly, sh_analyze_prologue reads the FPSCR register to find out
whether a floating-point "push" instruction decrements the stack
pointer by 4 or 8 bytes. Again, I've added a FPSCR parameter and have
the unwinder pass in the unwound register value. However, this
routine is also called from the skip_prologue callback. In this case,
we simply do not know the contents of that flags register.
However, the only thing we get wrong if we don't know that setting
is the stack offset of saved registers, and skip_prologue does not
actually care about those. So just arbitrarily passing in 0 in that
case should work fine.
This all really needs to be separated out more cleanly ...
- Finally, the mips{16,32}_scan_prologue routines are called either
from the unwinder with a next_frame argument, or from skip_prologue
with a NULL next_frame argument. They make use of a helper routine
read_next_frame_reg that either unwinds from next_frame or else
reads from the current_regcache. That strikes me as bogus, as the
current_regcache isn't at all related to the function being
queried via skip_prologue ...
However, again the only thing we'd be getting wrong should be
stack offsets of saved registers in the case of functions using
alloca. So the patch below changes the scan_prologue routines to
skip trying to handle that case if called without next_frame.
Bye,
Ulrich
ChangeLog:
* mips-tdep.c (mips16_scan_prologue): Replace read_next_frame_reg
by frame_unwind_register_signed calls.
(mips32_scan_prologue): Likewise. Skip analysis of alloca stack
frame allocations when called with NULL NEXT_FRAME parameter.
(read_next_frame_reg): Remove.
* sh-tdep.c (sh_analyze_prologue): Add FPSCR parameter. Use it
instead of reading the FPSCR register.
(sh_frame_cache): Pass unwound FPSCR register value to
sh_analyze_prologue.
(sh_skip_prologue): Pass dummy FPSCR value to sh_analyze_prologue.
* v850-tdep.c (v850_analyze_prologue): Add CTBP parameter. Use it
instead of reading the CTBP register.
(v850_frame_cache): Pass unwound CTBP register value to
v850_analyze_prologue.
diff -urNp gdb-orig/gdb/mips-tdep.c gdb-head/gdb/mips-tdep.c
--- gdb-orig/gdb/mips-tdep.c 2007-06-06 18:57:49.422550908 +0200
+++ gdb-head/gdb/mips-tdep.c 2007-06-06 18:58:57.267028491 +0200
@@ -402,8 +402,6 @@ mips2_fp_compat (void)
static CORE_ADDR heuristic_proc_start (CORE_ADDR);
-static CORE_ADDR read_next_frame_reg (struct frame_info *, int);
-
static void reinit_frame_cache_sfunc (char *, int, struct cmd_list_element *);
static struct type *mips_float_register_type (void);
@@ -1436,8 +1434,9 @@ mips16_scan_prologue (CORE_ADDR start_pc
/* Can be called when there's no process, and hence when there's no
NEXT_FRAME. */
if (next_frame != NULL)
- sp = read_next_frame_reg (next_frame, gdbarch_num_regs (current_gdbarch)
- + MIPS_SP_REGNUM);
+ sp = frame_unwind_register_signed (next_frame,
+ gdbarch_num_regs (current_gdbarch)
+ + MIPS_SP_REGNUM);
else
sp = 0;
@@ -1757,8 +1756,9 @@ mips32_scan_prologue (CORE_ADDR start_pc
/* Can be called when there's no process, and hence when there's no
NEXT_FRAME. */
if (next_frame != NULL)
- sp = read_next_frame_reg (next_frame, gdbarch_num_regs (current_gdbarch)
- + MIPS_SP_REGNUM);
+ sp = frame_unwind_register_signed (next_frame,
+ gdbarch_num_regs (current_gdbarch)
+ + MIPS_SP_REGNUM);
else
sp = 0;
@@ -1808,14 +1808,15 @@ restart:
/* Old gcc frame, r30 is virtual frame pointer. */
if ((long) low_word != frame_offset)
frame_addr = sp + low_word;
- else if (frame_reg == MIPS_SP_REGNUM)
+ else if (next_frame && frame_reg == MIPS_SP_REGNUM)
{
unsigned alloca_adjust;
frame_reg = 30;
- frame_addr = read_next_frame_reg (next_frame,
- gdbarch_num_regs
- (current_gdbarch) + 30);
+ frame_addr = frame_unwind_register_signed
+ (next_frame,
+ gdbarch_num_regs (current_gdbarch) + 30);
+
alloca_adjust = (unsigned) (frame_addr - (sp + low_word));
if (alloca_adjust > 0)
{
@@ -1838,14 +1839,15 @@ restart:
else if (inst == 0x03A0F021 || inst == 0x03a0f025 || inst == 0x03a0f02d)
{
/* New gcc frame, virtual frame pointer is at r30 + frame_size. */
- if (frame_reg == MIPS_SP_REGNUM)
+ if (next_frame && frame_reg == MIPS_SP_REGNUM)
{
unsigned alloca_adjust;
frame_reg = 30;
- frame_addr = read_next_frame_reg (next_frame,
- gdbarch_num_regs
- (current_gdbarch) + 30);
+ frame_addr = frame_unwind_register_signed
+ (next_frame,
+ gdbarch_num_regs (current_gdbarch) + 30);
+
alloca_adjust = (unsigned) (frame_addr - sp);
if (alloca_adjust > 0)
{
@@ -2153,22 +2155,6 @@ mips_stub_frame_base_sniffer (struct fra
return NULL;
}
-static CORE_ADDR
-read_next_frame_reg (struct frame_info *fi, int regno)
-{
- /* Always a pseudo. */
- gdb_assert (regno >= gdbarch_num_regs (current_gdbarch));
- if (fi == NULL)
- {
- LONGEST val;
- regcache_cooked_read_signed (current_regcache, regno, &val);
- return val;
- }
- else
- return frame_unwind_register_signed (fi, regno);
-
-}
-
/* mips_addr_bits_remove - remove useless address bits */
static CORE_ADDR
diff -urNp gdb-orig/gdb/sh-tdep.c gdb-head/gdb/sh-tdep.c
--- gdb-orig/gdb/sh-tdep.c 2007-06-06 18:58:46.803937873 +0200
+++ gdb-head/gdb/sh-tdep.c 2007-06-06 18:58:57.322020578 +0200
@@ -508,7 +508,7 @@ gdb_print_insn_sh (bfd_vma memaddr, disa
static CORE_ADDR
sh_analyze_prologue (CORE_ADDR pc, CORE_ADDR current_pc,
- struct sh_frame_cache *cache)
+ struct sh_frame_cache *cache, ULONGEST fpscr)
{
ULONGEST inst;
CORE_ADDR opc;
@@ -615,7 +615,7 @@ sh_analyze_prologue (CORE_ADDR pc, CORE_
}
else if (IS_FPUSH (inst))
{
- if (read_register (FPSCR_REGNUM) & FPSCR_SZ)
+ if (fpscr & FPSCR_SZ)
{
cache->sp_offset += 8;
}
@@ -728,7 +728,7 @@ sh_skip_prologue (CORE_ADDR start_pc)
return max (pc, start_pc);
cache.sp_offset = -4;
- pc = sh_analyze_prologue (start_pc, (CORE_ADDR) -1, &cache);
+ pc = sh_analyze_prologue (start_pc, (CORE_ADDR) -1, &cache, 0);
if (!cache.uses_fp)
return start_pc;
@@ -2360,7 +2360,11 @@ sh_frame_cache (struct frame_info *next_
cache->pc = frame_func_unwind (next_frame, NORMAL_FRAME);
current_pc = frame_pc_unwind (next_frame);
if (cache->pc != 0)
- sh_analyze_prologue (cache->pc, current_pc, cache);
+ {
+ ULONGEST fpscr;
+ fpscr = frame_unwind_register_unsigned (next_frame, FPSCR_REGNUM);
+ sh_analyze_prologue (cache->pc, current_pc, cache, fpscr);
+ }
if (!cache->uses_fp)
{
diff -urNp gdb-orig/gdb/v850-tdep.c gdb-head/gdb/v850-tdep.c
--- gdb-orig/gdb/v850-tdep.c 2007-06-04 22:23:28.000000000 +0200
+++ gdb-head/gdb/v850-tdep.c 2007-06-06 18:58:57.364014535 +0200
@@ -456,7 +456,7 @@ v850_is_save_register (int reg)
static CORE_ADDR
v850_analyze_prologue (CORE_ADDR func_addr, CORE_ADDR pc,
- struct v850_frame_cache *pi)
+ struct v850_frame_cache *pi, ULONGEST ctbp)
{
CORE_ADDR prologue_end, current_pc;
struct pifsr pifsrs[E_NUM_REGS + 1];
@@ -517,7 +517,6 @@ v850_analyze_prologue (CORE_ADDR func_ad
}
else if ((insn & 0xffc0) == 0x0200 && !regsave_func_p)
{ /* callt <imm6> */
- long ctbp = read_register (E_CTBP_REGNUM);
long adr = ctbp + ((insn & 0x3f) << 1);
save_pc = current_pc;
@@ -859,7 +858,11 @@ v850_frame_cache (struct frame_info *nex
cache->pc = frame_func_unwind (next_frame, NORMAL_FRAME);
current_pc = frame_pc_unwind (next_frame);
if (cache->pc != 0)
- v850_analyze_prologue (cache->pc, current_pc, cache);
+ {
+ ULONGEST ctbp;
+ ctbp = frame_unwind_register_unsigned (next_frame, E_CTBP_REGNUM);
+ v850_analyze_prologue (cache->pc, current_pc, cache, ctbp);
+ }
if (!cache->uses_fp)
{
--
Dr. Ulrich Weigand
GNU Toolchain for Linux on System z and Cell BE
Ulrich.Weigand@de.ibm.com