* RFA: ia64 tdep patch
@ 2003-10-17 19:58 J. Johnston
2003-10-20 20:13 ` Kevin Buettner
0 siblings, 1 reply; 17+ messages in thread
From: J. Johnston @ 2003-10-17 19:58 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 1729 bytes --]
The attached ia64 patch fixes a few problems, most notably backtracing through
signal handlers.
Ok to commit?
2003-10-17 Jeff Johnston <jjohnstn@redhat.com>
* ia64-tdep.c: Change all references of DEPRECATED_REGISTER_RAW_SIZE
to use register_size() instead.
(ia64_frame_cache): Add new prev_cfm field.
(ia64_alloc_frame_cache): Initialize new prev_cfm field to 0.
(floatformat_valid): New static routine.
(floatformat_ia64_ext): Add name field and set up is_valid routine
to floatformat_valid().
(examine_prologue): For the previous cfm, use frame_unwind_register()
if the cfm is not stored in a register-stack register. Save the
previous cfm value in the prev_cfm field. Add debug output.
(ia64_frame_this_id): Use frame_id_build_special() to also register
the bsp. Add debug output.
(ia64_sigtramp_frame_this_id): Ditto.
(ia64_frame_prev_register): Look at cache saved_regs for a few more
registers and also add some checks for framelessness before accepting
current register values for fields such as return address. For cfm,
use the cached prev_cfm field if available. Add debug output.
(ia64_sigtramp_frame_init_saved_regs): Bump up base by 16 to get
sp needed for calling lower level
ia64_linux_sigcontext_register_address(). Also save the
bsp and sp address as part of initialization.
(ia64_sigtramp_frame_cache): Don't bother using cached stack size as it
won't be set. Cache the bsp and cfm values.
(ia64_sigtramp_frame_prev_register): Flesh this routine out instead of
using ia64_frame_prev_register(). The saved values for bsp and sp can
be taken from the cache. Add debug output.
(ia64_push_dummy_call): Use frame_id_build_special() to also register
the bsp.
[-- Attachment #2: ia64-tdep.patch --]
[-- Type: text/plain, Size: 16269 bytes --]
Index: ia64-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/ia64-tdep.c,v
retrieving revision 1.98
diff -u -p -r1.98 ia64-tdep.c
--- ia64-tdep.c 2 Oct 2003 20:28:29 -0000 1.98
+++ ia64-tdep.c 17 Oct 2003 19:21:23 -0000
@@ -232,6 +232,7 @@ struct ia64_frame_cache
CORE_ADDR saved_sp; /* stack pointer for frame */
CORE_ADDR bsp; /* points at r32 for the current frame */
CORE_ADDR cfm; /* cfm value for current frame */
+ CORE_ADDR prev_cfm; /* cfm value for previous frame */
int frameless;
int sof; /* Size of frame (decoded from cfm value) */
int sol; /* Size of locals (decoded from cfm value) */
@@ -316,10 +317,18 @@ ia64_dwarf_reg_to_regnum (int reg)
return reg;
}
+static int
+floatformat_valid (fmt, from)
+ const struct floatformat *fmt;
+ const char *from;
+{
+ return 1;
+}
+
const struct floatformat floatformat_ia64_ext =
{
floatformat_little, 82, 0, 1, 17, 65535, 0x1ffff, 18, 64,
- floatformat_intbit_yes
+ floatformat_intbit_yes, "floatformat_ia64_ext", floatformat_valid
};
@@ -1030,6 +1039,7 @@ ia64_alloc_frame_cache (void)
cache->base = 0;
cache->pc = 0;
cache->cfm = 0;
+ cache->prev_cfm = 0;
cache->sof = 0;
cache->sol = 0;
cache->sor = 0;
@@ -1450,9 +1460,20 @@ examine_prologue (CORE_ADDR pc, CORE_ADD
/* For the previous argument registers we require the previous bof.
If we can't find the previous cfm, then we can do nothing. */
+ cfm = 0;
if (cache->saved_regs[IA64_CFM_REGNUM] != 0)
{
cfm = read_memory_integer (cache->saved_regs[IA64_CFM_REGNUM], 8);
+ }
+ else if (cfm_reg != 0)
+ {
+ frame_unwind_register (next_frame, cfm_reg, buf);
+ cfm = extract_unsigned_integer (buf, 8);
+ }
+ cache->prev_cfm = cfm;
+
+ if (cfm != 0)
+ {
sor = ((cfm >> 14) & 0xf) * 8;
sof = (cfm & 0x7f);
sol = (cfm >> 7) & 0x7f;
@@ -1564,7 +1585,11 @@ ia64_frame_this_id (struct frame_info *n
if (cache->base == 0)
return;
- (*this_id) = frame_id_build (cache->base, cache->pc);
+ (*this_id) = frame_id_build_special (cache->base, cache->pc, cache->bsp);
+ if (gdbarch_debug >= 1)
+ fprintf_unfiltered (gdb_stdlog,
+ "regular frame id: code %lx, stack %lx, special %lx, next_frame %p\n",
+ this_id->code_addr, this_id->stack_addr, cache->bsp, next_frame);
}
static void
@@ -1613,33 +1638,35 @@ ia64_frame_prev_register (struct frame_i
/* We want to calculate the previous bsp as the end of the previous register stack frame.
This corresponds to what the hardware bsp register will be if we pop the frame
back which is why we might have been called. We know the beginning of the current
- frame is cache->bsp - cache->sof. This value in the previous frame points to
+ frame is cache->bsp - cache->sof. This value in the previous frame points to
the start of the output registers. We can calculate the end of that frame by adding
the size of output (sof (size of frame) - sol (size of locals)). */
ia64_frame_prev_register (next_frame, this_cache, IA64_CFM_REGNUM,
&cfm_optim, &cfm_lval, &cfm_addr, &cfm_realnum, cfm_valuep);
prev_cfm = extract_unsigned_integer (cfm_valuep, 8);
-
+
bsp = rse_address_add (cache->bsp, -(cache->sof));
prev_bsp = rse_address_add (bsp, (prev_cfm & 0x7f) - ((prev_cfm >> 7) & 0x7f));
-
+
store_unsigned_integer (valuep, DEPRECATED_REGISTER_RAW_SIZE (regnum),
prev_bsp);
}
else if (regnum == IA64_CFM_REGNUM)
{
- CORE_ADDR addr = 0;
-
- if (cache->frameless)
+ CORE_ADDR addr = cache->saved_regs[IA64_CFM_REGNUM];
+
+ if (addr != 0)
{
- CORE_ADDR cfm = 0;
- frame_unwind_register (next_frame, IA64_PFS_REGNUM, valuep);
+ *lvalp = lval_memory;
+ *addrp = addr;
+ read_memory (addr, valuep, DEPRECATED_REGISTER_RAW_SIZE (regnum));
}
- else
+ else if (cache->prev_cfm)
+ store_unsigned_integer (valuep, DEPRECATED_REGISTER_RAW_SIZE (regnum), cache->prev_cfm);
+ else if (cache->frameless)
{
- addr = cache->saved_regs[IA64_CFM_REGNUM];
- if (addr != 0)
- read_memory (addr, valuep, DEPRECATED_REGISTER_RAW_SIZE (regnum));
+ CORE_ADDR cfm = 0;
+ frame_unwind_register (next_frame, IA64_PFS_REGNUM, valuep);
}
}
else if (regnum == IA64_VFP_REGNUM)
@@ -1727,20 +1754,19 @@ ia64_frame_prev_register (struct frame_i
else if (regnum == IA64_IP_REGNUM)
{
CORE_ADDR pc = 0;
+ CORE_ADDR addr = cache->saved_regs[IA64_VRAP_REGNUM];
- if (cache->frameless)
+ if (addr != 0)
{
- frame_unwind_register (next_frame, IA64_BR0_REGNUM, buf);
+ *lvalp = lval_memory;
+ *addrp = addr;
+ read_memory (addr, buf, DEPRECATED_REGISTER_RAW_SIZE (IA64_IP_REGNUM));
pc = extract_unsigned_integer (buf, 8);
}
- else
+ else if (cache->frameless)
{
- CORE_ADDR addr = cache->saved_regs[IA64_VRAP_REGNUM];
- if (addr != 0)
- {
- read_memory (addr, buf, DEPRECATED_REGISTER_RAW_SIZE (IA64_IP_REGNUM));
- pc = extract_unsigned_integer (buf, 8);
- }
+ frame_unwind_register (next_frame, IA64_BR0_REGNUM, buf);
+ pc = extract_unsigned_integer (buf, 8);
}
pc &= ~0xf;
store_unsigned_integer (valuep, 8, pc);
@@ -1750,30 +1776,42 @@ ia64_frame_prev_register (struct frame_i
ULONGEST slot_num = 0;
CORE_ADDR pc= 0;
CORE_ADDR psr = 0;
+ CORE_ADDR addr = cache->saved_regs[IA64_VRAP_REGNUM];
frame_unwind_register (next_frame, IA64_PSR_REGNUM, buf);
psr = extract_unsigned_integer (buf, 8);
- if (cache->frameless)
+ if (addr != 0)
{
- CORE_ADDR pc;
- frame_unwind_register (next_frame, IA64_BR0_REGNUM, buf);
+ *lvalp = lval_memory;
+ *addrp = addr;
+ read_memory (addr, buf, DEPRECATED_REGISTER_RAW_SIZE (IA64_IP_REGNUM));
pc = extract_unsigned_integer (buf, 8);
}
- else
+ else if (cache->frameless)
{
- CORE_ADDR addr = cache->saved_regs[IA64_VRAP_REGNUM];
- if (addr != 0)
- {
- read_memory (addr, buf, DEPRECATED_REGISTER_RAW_SIZE (IA64_IP_REGNUM));
- pc = extract_unsigned_integer (buf, 8);
- }
+ CORE_ADDR pc;
+ frame_unwind_register (next_frame, IA64_BR0_REGNUM, buf);
+ pc = extract_unsigned_integer (buf, 8);
}
psr &= ~(3LL << 41);
slot_num = pc & 0x3LL;
psr |= (CORE_ADDR)slot_num << 41;
store_unsigned_integer (valuep, 8, psr);
}
+ else if (regnum == IA64_BR0_REGNUM)
+ {
+ CORE_ADDR br0 = 0;
+ CORE_ADDR addr = cache->saved_regs[IA64_BR0_REGNUM];
+ if (addr != 0)
+ {
+ *lvalp = lval_memory;
+ *addrp = addr;
+ read_memory (addr, buf, DEPRECATED_REGISTER_RAW_SIZE (IA64_BR0_REGNUM));
+ br0 = extract_unsigned_integer (buf, 8);
+ }
+ store_unsigned_integer (valuep, 8, br0);
+ }
else if ((regnum >= IA64_GR32_REGNUM && regnum <= IA64_GR127_REGNUM) ||
(regnum >= V32_REGNUM && regnum <= V127_REGNUM))
{
@@ -1836,9 +1874,17 @@ ia64_frame_prev_register (struct frame_i
read_memory (addr, valuep, DEPRECATED_REGISTER_RAW_SIZE (regnum));
}
/* Otherwise, punt and get the current value of the register. */
- else
- frame_unwind_register (next_frame, regnum, valuep);
+ else
+ {
+ frame_unwind_register (next_frame, regnum, valuep);
+ }
}
+
+ if (gdbarch_debug >= 1)
+ fprintf_unfiltered (gdb_stdlog,
+ "regular prev register <%d> <%s> is %lx\n", regnum,
+ (((unsigned) regnum <= IA64_NAT127_REGNUM)
+ ? ia64_register_names[regnum] : "r??"), extract_unsigned_integer (valuep, 8));
}
static const struct frame_unwind ia64_frame_unwind =
@@ -1862,39 +1908,37 @@ ia64_sigtramp_frame_init_saved_regs (str
if (SIGCONTEXT_REGISTER_ADDRESS)
{
int regno;
+ CORE_ADDR sp = cache->base + 16;
cache->saved_regs[IA64_VRAP_REGNUM] =
- SIGCONTEXT_REGISTER_ADDRESS (cache->base, IA64_IP_REGNUM);
+ SIGCONTEXT_REGISTER_ADDRESS (sp, IA64_IP_REGNUM);
cache->saved_regs[IA64_CFM_REGNUM] =
- SIGCONTEXT_REGISTER_ADDRESS (cache->base, IA64_CFM_REGNUM);
+ SIGCONTEXT_REGISTER_ADDRESS (sp, IA64_CFM_REGNUM);
cache->saved_regs[IA64_PSR_REGNUM] =
- SIGCONTEXT_REGISTER_ADDRESS (cache->base, IA64_PSR_REGNUM);
-#if 0
+ SIGCONTEXT_REGISTER_ADDRESS (sp, IA64_PSR_REGNUM);
cache->saved_regs[IA64_BSP_REGNUM] =
- SIGCONTEXT_REGISTER_ADDRESS (frame->frame, IA64_BSP_REGNUM);
-#endif
+ SIGCONTEXT_REGISTER_ADDRESS (sp, IA64_BSP_REGNUM);
cache->saved_regs[IA64_RNAT_REGNUM] =
- SIGCONTEXT_REGISTER_ADDRESS (cache->base, IA64_RNAT_REGNUM);
+ SIGCONTEXT_REGISTER_ADDRESS (sp, IA64_RNAT_REGNUM);
cache->saved_regs[IA64_CCV_REGNUM] =
- SIGCONTEXT_REGISTER_ADDRESS (cache->base, IA64_CCV_REGNUM);
+ SIGCONTEXT_REGISTER_ADDRESS (sp, IA64_CCV_REGNUM);
cache->saved_regs[IA64_UNAT_REGNUM] =
- SIGCONTEXT_REGISTER_ADDRESS (cache->base, IA64_UNAT_REGNUM);
+ SIGCONTEXT_REGISTER_ADDRESS (sp, IA64_UNAT_REGNUM);
cache->saved_regs[IA64_FPSR_REGNUM] =
- SIGCONTEXT_REGISTER_ADDRESS (cache->base, IA64_FPSR_REGNUM);
+ SIGCONTEXT_REGISTER_ADDRESS (sp, IA64_FPSR_REGNUM);
cache->saved_regs[IA64_PFS_REGNUM] =
- SIGCONTEXT_REGISTER_ADDRESS (cache->base, IA64_PFS_REGNUM);
+ SIGCONTEXT_REGISTER_ADDRESS (sp, IA64_PFS_REGNUM);
cache->saved_regs[IA64_LC_REGNUM] =
- SIGCONTEXT_REGISTER_ADDRESS (cache->base, IA64_LC_REGNUM);
+ SIGCONTEXT_REGISTER_ADDRESS (sp, IA64_LC_REGNUM);
for (regno = IA64_GR1_REGNUM; regno <= IA64_GR31_REGNUM; regno++)
- if (regno != sp_regnum)
- cache->saved_regs[regno] =
- SIGCONTEXT_REGISTER_ADDRESS (cache->base, regno);
+ cache->saved_regs[regno] =
+ SIGCONTEXT_REGISTER_ADDRESS (sp, regno);
for (regno = IA64_BR0_REGNUM; regno <= IA64_BR7_REGNUM; regno++)
cache->saved_regs[regno] =
- SIGCONTEXT_REGISTER_ADDRESS (cache->base, regno);
+ SIGCONTEXT_REGISTER_ADDRESS (sp, regno);
for (regno = IA64_FR2_REGNUM; regno <= IA64_FR31_REGNUM; regno++)
cache->saved_regs[regno] =
- SIGCONTEXT_REGISTER_ADDRESS (cache->base, regno);
+ SIGCONTEXT_REGISTER_ADDRESS (sp, regno);
}
}
@@ -1912,7 +1956,14 @@ ia64_sigtramp_frame_cache (struct frame_
cache = ia64_alloc_frame_cache ();
frame_unwind_register (next_frame, sp_regnum, buf);
- cache->base = extract_unsigned_integer (buf, 8) + cache->mem_stack_frame_size;
+ cache->base = extract_unsigned_integer (buf, 8);
+
+ frame_unwind_register (next_frame, IA64_BSP_REGNUM, buf);
+ cache->bsp = extract_unsigned_integer (buf, 8);
+
+ frame_unwind_register (next_frame, IA64_CFM_REGNUM, buf);
+ cache->cfm = extract_unsigned_integer (buf, 8);
+ cache->sof = cache->cfm & 0x7f;
ia64_sigtramp_frame_init_saved_regs (cache);
@@ -1921,27 +1972,127 @@ ia64_sigtramp_frame_cache (struct frame_
}
static void
-ia64_sigtramp_frame_this_id (struct frame_info *next_frame,
- void **this_cache, struct frame_id *this_id)
+ia64_sigtramp_frame_prev_register (struct frame_info *next_frame, void **this_cache,
+ int regnum, int *optimizedp,
+ enum lval_type *lvalp, CORE_ADDR *addrp,
+ int *realnump, void *valuep)
{
+ char dummy_valp[MAX_REGISTER_SIZE];
+ char buf[MAX_REGISTER_SIZE];
+
struct ia64_frame_cache *cache =
ia64_sigtramp_frame_cache (next_frame, this_cache);
- (*this_id) = frame_id_build (cache->base, frame_pc_unwind (next_frame));
+ gdb_assert (regnum >= 0);
+
+ if (!target_has_registers)
+ error ("No registers.");
+
+ *optimizedp = 0;
+ *addrp = 0;
+ *lvalp = not_lval;
+ *realnump = -1;
+
+ /* Rather than check each time if valuep is non-null, supply a dummy buffer
+ when valuep is not supplied. */
+ if (!valuep)
+ valuep = dummy_valp;
+
+ memset (valuep, 0, register_size (current_gdbarch, regnum));
+
+ if (regnum == IA64_IP_REGNUM)
+ {
+ CORE_ADDR pc = 0;
+ CORE_ADDR addr = cache->saved_regs[IA64_VRAP_REGNUM];
+
+ if (addr != 0)
+ {
+ *lvalp = lval_memory;
+ *addrp = addr;
+ read_memory (addr, buf, register_size (current_gdbarch, IA64_IP_REGNUM));
+ pc = extract_unsigned_integer (buf, 8);
+ }
+ pc &= ~0xf;
+ store_unsigned_integer (valuep, 8, pc);
+ }
+ else if (regnum == IA64_PSR_REGNUM)
+ {
+ ULONGEST slot_num = 0;
+ CORE_ADDR pc= 0;
+ CORE_ADDR psr = 0;
+ CORE_ADDR addr = cache->saved_regs[IA64_VRAP_REGNUM];
+
+ if (addr != 0)
+ {
+ *lvalp = lval_memory;
+ *addrp = addr;
+ read_memory (addr, buf, register_size (current_gdbarch, IA64_IP_REGNUM));
+ pc = extract_unsigned_integer (buf, 8);
+ }
+ psr &= ~(3LL << 41);
+ slot_num = pc & 0x3LL;
+ psr |= (CORE_ADDR)slot_num << 41;
+ store_unsigned_integer (valuep, 8, psr);
+ }
+ else if ((regnum >= IA64_GR32_REGNUM && regnum <= IA64_GR127_REGNUM) ||
+ (regnum >= V32_REGNUM && regnum <= V127_REGNUM))
+ {
+ CORE_ADDR addr = 0;
+ if (regnum >= V32_REGNUM)
+ regnum = IA64_GR32_REGNUM + (regnum - V32_REGNUM);
+ addr = cache->saved_regs[regnum];
+ if (addr != 0)
+ {
+ *lvalp = lval_memory;
+ *addrp = addr;
+ read_memory (addr, valuep, register_size (current_gdbarch, regnum));
+ }
+ }
+ else
+ {
+ CORE_ADDR addr = 0;
+ if (IA64_FR32_REGNUM <= regnum && regnum <= IA64_FR127_REGNUM)
+ {
+ /* Fetch floating point register rename base from current
+ frame marker for this frame. */
+ int rrb_fr = (cache->cfm >> 25) & 0x7f;
+
+ /* Adjust the floating point register number to account for
+ register rotation. */
+ regnum = IA64_FR32_REGNUM
+ + ((regnum - IA64_FR32_REGNUM) + rrb_fr) % 96;
+ }
+
+ /* If we have stored a memory address, access the register. */
+ addr = cache->saved_regs[regnum];
+ if (addr != 0)
+ {
+ *lvalp = lval_memory;
+ *addrp = addr;
+ read_memory (addr, valuep, register_size (current_gdbarch, regnum));
+ }
+ }
+
+ if (gdbarch_debug >= 1)
+ fprintf_unfiltered (gdb_stdlog,
+ "sigtramp prev register <%s> is %lx\n",
+ (((unsigned) regnum <= IA64_NAT127_REGNUM)
+ ? ia64_register_names[regnum] : "r??"), extract_unsigned_integer (valuep, 8));
}
+
static void
-ia64_sigtramp_frame_prev_register (struct frame_info *next_frame,
- void **this_cache,
- int regnum, int *optimizedp,
- enum lval_type *lvalp, CORE_ADDR *addrp,
- int *realnump, void *valuep)
+ia64_sigtramp_frame_this_id (struct frame_info *next_frame,
+ void **this_cache, struct frame_id *this_id)
{
- /* Make sure we've initialized the cache. */
- ia64_sigtramp_frame_cache (next_frame, this_cache);
+ struct ia64_frame_cache *cache =
+ ia64_sigtramp_frame_cache (next_frame, this_cache);
- ia64_frame_prev_register (next_frame, this_cache, regnum,
- optimizedp, lvalp, addrp, realnump, valuep);
+ (*this_id) = frame_id_build_special (cache->base, frame_pc_unwind (next_frame), cache->bsp);
+ if (gdbarch_debug >= 1)
+ fprintf_unfiltered (gdb_stdlog,
+ "sigtramp frame id: code %lx, stack %lx, special %lx, next_frame %p\n",
+ this_id->code_addr, this_id->stack_addr, cache->bsp, next_frame);
}
static const struct frame_unwind ia64_sigtramp_frame_unwind =
@@ -2324,7 +2475,7 @@ ia64_push_dummy_call (struct gdbarch *gd
int len, argoffset;
int nslots, rseslots, memslots, slotnum, nfuncargs;
int floatreg;
- CORE_ADDR bsp, cfm, pfs, new_bsp, funcdescaddr, pc, global_pointer;
+ CORE_ADDR bsp, cfm, pfs, new_bsp, funcdescaddr, pc, global_pointer, base;
nslots = 0;
nfuncargs = 0;
@@ -2473,12 +2624,20 @@ static struct frame_id
ia64_unwind_dummy_id (struct gdbarch *gdbarch, struct frame_info *next_frame)
{
char buf[8];
- CORE_ADDR sp;
+ CORE_ADDR sp, bsp;
frame_unwind_register (next_frame, sp_regnum, buf);
sp = extract_unsigned_integer (buf, 8);
- return frame_id_build (sp, frame_pc_unwind (next_frame));
+ frame_unwind_register (next_frame, IA64_BSP_REGNUM, buf);
+ bsp = extract_unsigned_integer (buf, 8);
+
+ if (gdbarch_debug >= 1)
+ fprintf_unfiltered (gdb_stdlog,
+ "dummy frame id: code %lx, stack %lx, special %lx\n",
+ frame_pc_unwind (next_frame), sp, bsp);
+
+ return frame_id_build_special (sp, frame_pc_unwind (next_frame), bsp);
}
static CORE_ADDR
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: RFA: ia64 tdep patch
2003-10-17 19:58 RFA: ia64 tdep patch J. Johnston
@ 2003-10-20 20:13 ` Kevin Buettner
2003-10-20 21:55 ` J. Johnston
0 siblings, 1 reply; 17+ messages in thread
From: Kevin Buettner @ 2003-10-20 20:13 UTC (permalink / raw)
To: J. Johnston, gdb-patches
On Oct 17, 3:58pm, J. Johnston wrote:
> The attached ia64 patch fixes a few problems, most notably
> backtracing through signal handlers.
I think these changes are mostly okay, but...
In your ChangeLog entry, you say:
> * ia64-tdep.c: Change all references of DEPRECATED_REGISTER_RAW_SIZE
> to use register_size() instead.
Yet, later on, in the patch, I see that you're reintroducing a use of
DEPRECATED_REGISTER_RAW_SIZE:
+ else if (regnum == IA64_BR0_REGNUM)
+ {
+ CORE_ADDR br0 = 0;
+ CORE_ADDR addr = cache->saved_regs[IA64_BR0_REGNUM];
+ if (addr != 0)
+ {
+ *lvalp = lval_memory;
+ *addrp = addr;
+ read_memory (addr, buf, DEPRECATED_REGISTER_RAW_SIZE (IA64_BR0_REGNUM));
+ br0 = extract_unsigned_integer (buf, 8);
+ }
+ store_unsigned_integer (valuep, 8, br0);
+ }
Also, regarding:
/* We want to calculate the previous bsp as the end of the previous register stack frame.
This corresponds to what the hardware bsp register will be if we pop the frame
back which is why we might have been called. We know the beginning of the current
- frame is cache->bsp - cache->sof. This value in the previous frame points to
+ frame is cache->bsp - cache->sof. This value in the previous frame points to
the start of the output registers. We can calculate the end of that frame by adding
the size of output (sof (size of frame) - sol (size of locals)). */
ia64_frame_prev_register (next_frame, this_cache, IA64_CFM_REGNUM,
&cfm_optim, &cfm_lval, &cfm_addr, &cfm_realnum, cfm_valuep);
prev_cfm = extract_unsigned_integer (cfm_valuep, 8);
-
+
bsp = rse_address_add (cache->bsp, -(cache->sof));
prev_bsp = rse_address_add (bsp, (prev_cfm & 0x7f) - ((prev_cfm >> 7) & 0x7f));
-
+
The first white space change is okay, but the latter two are not. I'd
really prefer to see whitespace changes occur via a separate patch anyway.
So, if you don't mind, could you please submit separate patches for:
1) whitespace changes
2) DEPRECATED_... elimination
3) the CFM / backtracing changes.
Patches (1) and (2) are preapproved -- just make sure that you don't
introduce extra white space on an otherwise blank line. For (1) and
(2), please post the patches that you end up committing.
Once (1) and (2) are split out, I'd like another chance to review what's
left (patch 3).
With regard to (3), one of the questions that I already have concerns
the following line in ia64_sigtramp_frame_init_saved_regs():
+ CORE_ADDR sp = cache->base + 16;
Could you explain what this is about? (Preferably with a comment in the
code?)
Thanks,
Kevin
P.S. If you'd prefer to reverse things and submit patch 3 first, that'd
be okay too.
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: RFA: ia64 tdep patch
2003-10-20 20:13 ` Kevin Buettner
@ 2003-10-20 21:55 ` J. Johnston
2003-10-21 22:22 ` Kevin Buettner
0 siblings, 1 reply; 17+ messages in thread
From: J. Johnston @ 2003-10-20 21:55 UTC (permalink / raw)
To: Kevin Buettner; +Cc: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 4720 bytes --]
Kevin Buettner wrote:
> On Oct 17, 3:58pm, J. Johnston wrote:
>
>
>>The attached ia64 patch fixes a few problems, most notably
>>backtracing through signal handlers.
>
>
> I think these changes are mostly okay, but...
>
> In your ChangeLog entry, you say:
>
>
>> * ia64-tdep.c: Change all references of DEPRECATED_REGISTER_RAW_SIZE
>> to use register_size() instead.
>
>
> Yet, later on, in the patch, I see that you're reintroducing a use of
> DEPRECATED_REGISTER_RAW_SIZE:
>
> + else if (regnum == IA64_BR0_REGNUM)
> + {
> + CORE_ADDR br0 = 0;
> + CORE_ADDR addr = cache->saved_regs[IA64_BR0_REGNUM];
> + if (addr != 0)
> + {
> + *lvalp = lval_memory;
> + *addrp = addr;
> + read_memory (addr, buf, DEPRECATED_REGISTER_RAW_SIZE (IA64_BR0_REGNUM));
> + br0 = extract_unsigned_integer (buf, 8);
> + }
> + store_unsigned_integer (valuep, 8, br0);
> + }
>
> Also, regarding:
>
> /* We want to calculate the previous bsp as the end of the previous register stack frame.
> This corresponds to what the hardware bsp register will be if we pop the frame
> back which is why we might have been called. We know the beginning of the current
> - frame is cache->bsp - cache->sof. This value in the previous frame points to
> + frame is cache->bsp - cache->sof. This value in the previous frame points to
> the start of the output registers. We can calculate the end of that frame by adding
> the size of output (sof (size of frame) - sol (size of locals)). */
> ia64_frame_prev_register (next_frame, this_cache, IA64_CFM_REGNUM,
> &cfm_optim, &cfm_lval, &cfm_addr, &cfm_realnum, cfm_valuep);
> prev_cfm = extract_unsigned_integer (cfm_valuep, 8);
> -
> +
> bsp = rse_address_add (cache->bsp, -(cache->sof));
> prev_bsp = rse_address_add (bsp, (prev_cfm & 0x7f) - ((prev_cfm >> 7) & 0x7f));
> -
> +
>
> The first white space change is okay, but the latter two are not. I'd
> really prefer to see whitespace changes occur via a separate patch anyway.
>
> So, if you don't mind, could you please submit separate patches for:
>
> 1) whitespace changes
> 2) DEPRECATED_... elimination
> 3) the CFM / backtracing changes.
>
> Patches (1) and (2) are preapproved -- just make sure that you don't
> introduce extra white space on an otherwise blank line. For (1) and
> (2), please post the patches that you end up committing.
>
> Once (1) and (2) are split out, I'd like another chance to review what's
> left (patch 3).
>
Ok, done. A patch for 1) and 2) has been checked in. See the new attached patch.
2003-10-20 Jeff Johnston <jjohnstn@redhat.com>
* ia64-tdep.c: (ia64_frame_cache): Add new prev_cfm field.
(ia64_alloc_frame_cache): Initialize new prev_cfm field to 0.
(floatformat_valid): New static routine.
(floatformat_ia64_ext): Add name field and set up is_valid routine
to floatformat_valid().
(examine_prologue): For the previous cfm, use frame_unwind_register()
if the cfm is not stored in a register-stack register. Save the
previous cfm value in the prev_cfm field. Add debug output.
(ia64_frame_this_id): Use frame_id_build_special() to also register
the bsp. Add debug output.
(ia64_sigtramp_frame_this_id): Ditto.
(ia64_frame_prev_register): Look at cache saved_regs for a few more
registers and also add some checks for framelessness before accepting
current register values for fields such as return address. For cfm,
use the cached prev_cfm field if available. Add debug output.
(ia64_sigtramp_frame_init_saved_regs): Bump up base by 16 to get
sp needed for calling lower level
ia64_linux_sigcontext_register_address(). Also save the
bsp and sp address as part of initialization.
(ia64_sigtramp_frame_cache): Hard-code stack size as it can't be
calculated. Cache the bsp and cfm values.
(ia64_sigtramp_frame_prev_register): Flesh this routine out instead of
using ia64_frame_prev_register(). The saved values for bsp and sp can
be taken from the cache. Add debug output.
(ia64_push_dummy_call): Use frame_id_build_special() to also register
the bsp.
> With regard to (3), one of the questions that I already have concerns
> the following line in ia64_sigtramp_frame_init_saved_regs():
>
> + CORE_ADDR sp = cache->base + 16;
>
> Could you explain what this is about? (Preferably with a comment in the
> code?)
>
I've simplified it. It is the mem stack size that should have been added to the
base. I have added a comment that it cannot be calculated via prologue examination.
> Thanks,
>
> Kevin
>
> P.S. If you'd prefer to reverse things and submit patch 3 first, that'd
> be okay too.
>
[-- Attachment #2: ia64-tdep.patch --]
[-- Type: text/plain, Size: 12944 bytes --]
Index: ia64-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/ia64-tdep.c,v
retrieving revision 1.99
diff -u -p -r1.99 ia64-tdep.c
--- ia64-tdep.c 20 Oct 2003 20:38:07 -0000 1.99
+++ ia64-tdep.c 20 Oct 2003 21:40:48 -0000
@@ -232,6 +232,7 @@ struct ia64_frame_cache
CORE_ADDR saved_sp; /* stack pointer for frame */
CORE_ADDR bsp; /* points at r32 for the current frame */
CORE_ADDR cfm; /* cfm value for current frame */
+ CORE_ADDR prev_cfm; /* cfm value for previous frame */
int frameless;
int sof; /* Size of frame (decoded from cfm value) */
int sol; /* Size of locals (decoded from cfm value) */
@@ -316,10 +317,18 @@ ia64_dwarf_reg_to_regnum (int reg)
return reg;
}
+static int
+floatformat_valid (fmt, from)
+ const struct floatformat *fmt;
+ const char *from;
+{
+ return 1;
+}
+
const struct floatformat floatformat_ia64_ext =
{
floatformat_little, 82, 0, 1, 17, 65535, 0x1ffff, 18, 64,
- floatformat_intbit_yes
+ floatformat_intbit_yes, "floatformat_ia64_ext", floatformat_valid
};
@@ -1030,6 +1039,7 @@ ia64_alloc_frame_cache (void)
cache->base = 0;
cache->pc = 0;
cache->cfm = 0;
+ cache->prev_cfm = 0;
cache->sof = 0;
cache->sol = 0;
cache->sor = 0;
@@ -1450,9 +1460,20 @@ examine_prologue (CORE_ADDR pc, CORE_ADD
/* For the previous argument registers we require the previous bof.
If we can't find the previous cfm, then we can do nothing. */
+ cfm = 0;
if (cache->saved_regs[IA64_CFM_REGNUM] != 0)
{
cfm = read_memory_integer (cache->saved_regs[IA64_CFM_REGNUM], 8);
+ }
+ else if (cfm_reg != 0)
+ {
+ frame_unwind_register (next_frame, cfm_reg, buf);
+ cfm = extract_unsigned_integer (buf, 8);
+ }
+ cache->prev_cfm = cfm;
+
+ if (cfm != 0)
+ {
sor = ((cfm >> 14) & 0xf) * 8;
sof = (cfm & 0x7f);
sol = (cfm >> 7) & 0x7f;
@@ -1564,7 +1585,11 @@ ia64_frame_this_id (struct frame_info *n
if (cache->base == 0)
return;
- (*this_id) = frame_id_build (cache->base, cache->pc);
+ (*this_id) = frame_id_build_special (cache->base, cache->pc, cache->bsp);
+ if (gdbarch_debug >= 1)
+ fprintf_unfiltered (gdb_stdlog,
+ "regular frame id: code %lx, stack %lx, special %lx, next_frame %p\n",
+ this_id->code_addr, this_id->stack_addr, cache->bsp, next_frame);
}
static void
@@ -1628,18 +1653,20 @@ ia64_frame_prev_register (struct frame_i
}
else if (regnum == IA64_CFM_REGNUM)
{
- CORE_ADDR addr = 0;
-
- if (cache->frameless)
+ CORE_ADDR addr = cache->saved_regs[IA64_CFM_REGNUM];
+
+ if (addr != 0)
{
- CORE_ADDR cfm = 0;
- frame_unwind_register (next_frame, IA64_PFS_REGNUM, valuep);
+ *lvalp = lval_memory;
+ *addrp = addr;
+ read_memory (addr, valuep, register_size (current_gdbarch, regnum));
}
- else
+ else if (cache->prev_cfm)
+ store_unsigned_integer (valuep, register_size (current_gdbarch, regnum), cache->prev_cfm);
+ else if (cache->frameless)
{
- addr = cache->saved_regs[IA64_CFM_REGNUM];
- if (addr != 0)
- read_memory (addr, valuep, register_size (current_gdbarch, regnum));
+ CORE_ADDR cfm = 0;
+ frame_unwind_register (next_frame, IA64_PFS_REGNUM, valuep);
}
}
else if (regnum == IA64_VFP_REGNUM)
@@ -1727,20 +1754,19 @@ ia64_frame_prev_register (struct frame_i
else if (regnum == IA64_IP_REGNUM)
{
CORE_ADDR pc = 0;
+ CORE_ADDR addr = cache->saved_regs[IA64_VRAP_REGNUM];
- if (cache->frameless)
+ if (addr != 0)
{
- frame_unwind_register (next_frame, IA64_BR0_REGNUM, buf);
+ *lvalp = lval_memory;
+ *addrp = addr;
+ read_memory (addr, buf, register_size (current_gdbarch, IA64_IP_REGNUM));
pc = extract_unsigned_integer (buf, 8);
}
- else
+ else if (cache->frameless)
{
- CORE_ADDR addr = cache->saved_regs[IA64_VRAP_REGNUM];
- if (addr != 0)
- {
- read_memory (addr, buf, register_size (current_gdbarch, IA64_IP_REGNUM));
- pc = extract_unsigned_integer (buf, 8);
- }
+ frame_unwind_register (next_frame, IA64_BR0_REGNUM, buf);
+ pc = extract_unsigned_integer (buf, 8);
}
pc &= ~0xf;
store_unsigned_integer (valuep, 8, pc);
@@ -1750,30 +1776,42 @@ ia64_frame_prev_register (struct frame_i
ULONGEST slot_num = 0;
CORE_ADDR pc= 0;
CORE_ADDR psr = 0;
+ CORE_ADDR addr = cache->saved_regs[IA64_VRAP_REGNUM];
frame_unwind_register (next_frame, IA64_PSR_REGNUM, buf);
psr = extract_unsigned_integer (buf, 8);
- if (cache->frameless)
+ if (addr != 0)
{
- CORE_ADDR pc;
- frame_unwind_register (next_frame, IA64_BR0_REGNUM, buf);
+ *lvalp = lval_memory;
+ *addrp = addr;
+ read_memory (addr, buf, register_size (current_gdbarch, IA64_IP_REGNUM));
pc = extract_unsigned_integer (buf, 8);
}
- else
+ else if (cache->frameless)
{
- CORE_ADDR addr = cache->saved_regs[IA64_VRAP_REGNUM];
- if (addr != 0)
- {
- read_memory (addr, buf, register_size (current_gdbarch, IA64_IP_REGNUM));
- pc = extract_unsigned_integer (buf, 8);
- }
+ CORE_ADDR pc;
+ frame_unwind_register (next_frame, IA64_BR0_REGNUM, buf);
+ pc = extract_unsigned_integer (buf, 8);
}
psr &= ~(3LL << 41);
slot_num = pc & 0x3LL;
psr |= (CORE_ADDR)slot_num << 41;
store_unsigned_integer (valuep, 8, psr);
}
+ else if (regnum == IA64_BR0_REGNUM)
+ {
+ CORE_ADDR br0 = 0;
+ CORE_ADDR addr = cache->saved_regs[IA64_BR0_REGNUM];
+ if (addr != 0)
+ {
+ *lvalp = lval_memory;
+ *addrp = addr;
+ read_memory (addr, buf, register_size (current_gdbarch, IA64_BR0_REGNUM));
+ br0 = extract_unsigned_integer (buf, 8);
+ }
+ store_unsigned_integer (valuep, 8, br0);
+ }
else if ((regnum >= IA64_GR32_REGNUM && regnum <= IA64_GR127_REGNUM) ||
(regnum >= V32_REGNUM && regnum <= V127_REGNUM))
{
@@ -1839,6 +1877,12 @@ ia64_frame_prev_register (struct frame_i
else
frame_unwind_register (next_frame, regnum, valuep);
}
+
+ if (gdbarch_debug >= 1)
+ fprintf_unfiltered (gdb_stdlog,
+ "regular prev register <%d> <%s> is %lx\n", regnum,
+ (((unsigned) regnum <= IA64_NAT127_REGNUM)
+ ? ia64_register_names[regnum] : "r??"), extract_unsigned_integer (valuep, 8));
}
static const struct frame_unwind ia64_frame_unwind =
@@ -1869,10 +1913,8 @@ ia64_sigtramp_frame_init_saved_regs (str
SIGCONTEXT_REGISTER_ADDRESS (cache->base, IA64_CFM_REGNUM);
cache->saved_regs[IA64_PSR_REGNUM] =
SIGCONTEXT_REGISTER_ADDRESS (cache->base, IA64_PSR_REGNUM);
-#if 0
cache->saved_regs[IA64_BSP_REGNUM] =
- SIGCONTEXT_REGISTER_ADDRESS (frame->frame, IA64_BSP_REGNUM);
-#endif
+ SIGCONTEXT_REGISTER_ADDRESS (cache->base, IA64_BSP_REGNUM);
cache->saved_regs[IA64_RNAT_REGNUM] =
SIGCONTEXT_REGISTER_ADDRESS (cache->base, IA64_RNAT_REGNUM);
cache->saved_regs[IA64_CCV_REGNUM] =
@@ -1886,9 +1928,8 @@ ia64_sigtramp_frame_init_saved_regs (str
cache->saved_regs[IA64_LC_REGNUM] =
SIGCONTEXT_REGISTER_ADDRESS (cache->base, IA64_LC_REGNUM);
for (regno = IA64_GR1_REGNUM; regno <= IA64_GR31_REGNUM; regno++)
- if (regno != sp_regnum)
- cache->saved_regs[regno] =
- SIGCONTEXT_REGISTER_ADDRESS (cache->base, regno);
+ cache->saved_regs[regno] =
+ SIGCONTEXT_REGISTER_ADDRESS (cache->base, regno);
for (regno = IA64_BR0_REGNUM; regno <= IA64_BR7_REGNUM; regno++)
cache->saved_regs[regno] =
SIGCONTEXT_REGISTER_ADDRESS (cache->base, regno);
@@ -1912,7 +1953,16 @@ ia64_sigtramp_frame_cache (struct frame_
cache = ia64_alloc_frame_cache ();
frame_unwind_register (next_frame, sp_regnum, buf);
- cache->base = extract_unsigned_integer (buf, 8) + cache->mem_stack_frame_size;
+ /* Note that frame size is hard-coded below. We cannot calculate it
+ via prologue examination. */
+ cache->base = extract_unsigned_integer (buf, 8) + 16;
+
+ frame_unwind_register (next_frame, IA64_BSP_REGNUM, buf);
+ cache->bsp = extract_unsigned_integer (buf, 8);
+
+ frame_unwind_register (next_frame, IA64_CFM_REGNUM, buf);
+ cache->cfm = extract_unsigned_integer (buf, 8);
+ cache->sof = cache->cfm & 0x7f;
ia64_sigtramp_frame_init_saved_regs (cache);
@@ -1927,7 +1977,11 @@ ia64_sigtramp_frame_this_id (struct fram
struct ia64_frame_cache *cache =
ia64_sigtramp_frame_cache (next_frame, this_cache);
- (*this_id) = frame_id_build (cache->base, frame_pc_unwind (next_frame));
+ (*this_id) = frame_id_build_special (cache->base, frame_pc_unwind (next_frame), cache->bsp);
+ if (gdbarch_debug >= 1)
+ fprintf_unfiltered (gdb_stdlog,
+ "sigtramp frame id: code %lx, stack %lx, special %lx, next_frame %p\n",
+ this_id->code_addr, this_id->stack_addr, cache->bsp, next_frame);
}
static void
@@ -1937,11 +1991,107 @@ ia64_sigtramp_frame_prev_register (struc
enum lval_type *lvalp, CORE_ADDR *addrp,
int *realnump, void *valuep)
{
- /* Make sure we've initialized the cache. */
- ia64_sigtramp_frame_cache (next_frame, this_cache);
+ char dummy_valp[MAX_REGISTER_SIZE];
+ char buf[MAX_REGISTER_SIZE];
+
+ struct ia64_frame_cache *cache =
+ ia64_sigtramp_frame_cache (next_frame, this_cache);
- ia64_frame_prev_register (next_frame, this_cache, regnum,
- optimizedp, lvalp, addrp, realnump, valuep);
+ gdb_assert (regnum >= 0);
+
+ if (!target_has_registers)
+ error ("No registers.");
+
+ *optimizedp = 0;
+ *addrp = 0;
+ *lvalp = not_lval;
+ *realnump = -1;
+
+ /* Rather than check each time if valuep is non-null, supply a dummy buffer
+ when valuep is not supplied. */
+ if (!valuep)
+ valuep = dummy_valp;
+
+ memset (valuep, 0, register_size (current_gdbarch, regnum));
+
+ if (regnum == IA64_IP_REGNUM)
+ {
+ CORE_ADDR pc = 0;
+ CORE_ADDR addr = cache->saved_regs[IA64_VRAP_REGNUM];
+
+ if (addr != 0)
+ {
+ *lvalp = lval_memory;
+ *addrp = addr;
+ read_memory (addr, buf, register_size (current_gdbarch, IA64_IP_REGNUM));
+ pc = extract_unsigned_integer (buf, 8);
+ }
+ pc &= ~0xf;
+ store_unsigned_integer (valuep, 8, pc);
+ }
+ else if (regnum == IA64_PSR_REGNUM)
+ {
+ ULONGEST slot_num = 0;
+ CORE_ADDR pc= 0;
+ CORE_ADDR psr = 0;
+ CORE_ADDR addr = cache->saved_regs[IA64_VRAP_REGNUM];
+
+ if (addr != 0)
+ {
+ *lvalp = lval_memory;
+ *addrp = addr;
+ read_memory (addr, buf, register_size (current_gdbarch, IA64_IP_REGNUM));
+ pc = extract_unsigned_integer (buf, 8);
+ }
+ psr &= ~(3LL << 41);
+ slot_num = pc & 0x3LL;
+ psr |= (CORE_ADDR)slot_num << 41;
+ store_unsigned_integer (valuep, 8, psr);
+ }
+ else if ((regnum >= IA64_GR32_REGNUM && regnum <= IA64_GR127_REGNUM) ||
+ (regnum >= V32_REGNUM && regnum <= V127_REGNUM))
+ {
+ CORE_ADDR addr = 0;
+ if (regnum >= V32_REGNUM)
+ regnum = IA64_GR32_REGNUM + (regnum - V32_REGNUM);
+ addr = cache->saved_regs[regnum];
+ if (addr != 0)
+ {
+ *lvalp = lval_memory;
+ *addrp = addr;
+ read_memory (addr, valuep, register_size (current_gdbarch, regnum));
+ }
+ }
+ else
+ {
+ CORE_ADDR addr = 0;
+ if (IA64_FR32_REGNUM <= regnum && regnum <= IA64_FR127_REGNUM)
+ {
+ /* Fetch floating point register rename base from current
+ frame marker for this frame. */
+ int rrb_fr = (cache->cfm >> 25) & 0x7f;
+
+ /* Adjust the floating point register number to account for
+ register rotation. */
+ regnum = IA64_FR32_REGNUM
+ + ((regnum - IA64_FR32_REGNUM) + rrb_fr) % 96;
+ }
+
+ /* If we have stored a memory address, access the register. */
+ addr = cache->saved_regs[regnum];
+ if (addr != 0)
+ {
+ *lvalp = lval_memory;
+ *addrp = addr;
+ read_memory (addr, valuep, register_size (current_gdbarch, regnum));
+ }
+ }
+
+ if (gdbarch_debug >= 1)
+ fprintf_unfiltered (gdb_stdlog,
+ "sigtramp prev register <%s> is %lx\n",
+ (((unsigned) regnum <= IA64_NAT127_REGNUM)
+ ? ia64_register_names[regnum] : "r??"), extract_unsigned_integer (valuep, 8));
}
static const struct frame_unwind ia64_sigtramp_frame_unwind =
@@ -2473,12 +2623,20 @@ static struct frame_id
ia64_unwind_dummy_id (struct gdbarch *gdbarch, struct frame_info *next_frame)
{
char buf[8];
- CORE_ADDR sp;
+ CORE_ADDR sp, bsp;
frame_unwind_register (next_frame, sp_regnum, buf);
sp = extract_unsigned_integer (buf, 8);
- return frame_id_build (sp, frame_pc_unwind (next_frame));
+ frame_unwind_register (next_frame, IA64_BSP_REGNUM, buf);
+ bsp = extract_unsigned_integer (buf, 8);
+
+ if (gdbarch_debug >= 1)
+ fprintf_unfiltered (gdb_stdlog,
+ "dummy frame id: code %lx, stack %lx, special %lx\n",
+ frame_pc_unwind (next_frame), sp, bsp);
+
+ return frame_id_build_special (sp, frame_pc_unwind (next_frame), bsp);
}
static CORE_ADDR
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: RFA: ia64 tdep patch
2003-10-20 21:55 ` J. Johnston
@ 2003-10-21 22:22 ` Kevin Buettner
2003-10-21 23:03 ` J. Johnston
0 siblings, 1 reply; 17+ messages in thread
From: Kevin Buettner @ 2003-10-21 22:22 UTC (permalink / raw)
To: J. Johnston, Kevin Buettner; +Cc: gdb-patches
On Oct 20, 5:55pm, J. Johnston wrote:
> 2003-10-20 Jeff Johnston <jjohnstn@redhat.com>
>
> * ia64-tdep.c: (ia64_frame_cache): Add new prev_cfm field.
[...]
> (ia64_sigtramp_frame_init_saved_regs): Bump up base by 16 to get
> sp needed for calling lower level
The entry for ia64_sigtramp_frame_init_saved_regs() doesn't describe
what was done there. AFAICT, the bumping by 16 part of the patch
has been removed. You still do the following though:
@@ -1869,10 +1913,8 @@ ia64_sigtramp_frame_init_saved_regs (str
SIGCONTEXT_REGISTER_ADDRESS (cache->base, IA64_CFM_REGNUM);
cache->saved_regs[IA64_PSR_REGNUM] =
SIGCONTEXT_REGISTER_ADDRESS (cache->base, IA64_PSR_REGNUM);
-#if 0
cache->saved_regs[IA64_BSP_REGNUM] =
- SIGCONTEXT_REGISTER_ADDRESS (frame->frame, IA64_BSP_REGNUM);
-#endif
+ SIGCONTEXT_REGISTER_ADDRESS (cache->base, IA64_BSP_REGNUM);
cache->saved_regs[IA64_RNAT_REGNUM] =
SIGCONTEXT_REGISTER_ADDRESS (cache->base, IA64_RNAT_REGNUM);
cache->saved_regs[IA64_CCV_REGNUM] =
@@ -1886,9 +1928,8 @@ ia64_sigtramp_frame_init_saved_regs (str
cache->saved_regs[IA64_LC_REGNUM] =
SIGCONTEXT_REGISTER_ADDRESS (cache->base, IA64_LC_REGNUM);
for (regno = IA64_GR1_REGNUM; regno <= IA64_GR31_REGNUM; regno++)
- if (regno != sp_regnum)
- cache->saved_regs[regno] =
- SIGCONTEXT_REGISTER_ADDRESS (cache->base, regno);
+ cache->saved_regs[regno] =
+ SIGCONTEXT_REGISTER_ADDRESS (cache->base, regno);
for (regno = IA64_BR0_REGNUM; regno <= IA64_BR7_REGNUM; regno++)
cache->saved_regs[regno] =
SIGCONTEXT_REGISTER_ADDRESS (cache->base, regno);
......
The code (below) in ia64_sigtramp_frame_prev_register() which computes
PSR doesn't look right to me. Could you check it? (If it is right,
please explain it...)
+ else if (regnum == IA64_PSR_REGNUM)
+ {
+ ULONGEST slot_num = 0;
+ CORE_ADDR pc= 0;
+ CORE_ADDR psr = 0;
+ CORE_ADDR addr = cache->saved_regs[IA64_VRAP_REGNUM];
+
+ if (addr != 0)
+ {
+ *lvalp = lval_memory;
+ *addrp = addr;
+ read_memory (addr, buf, register_size (current_gdbarch, IA64_IP_REGNUM));
+ pc = extract_unsigned_integer (buf, 8);
+ }
+ psr &= ~(3LL << 41);
+ slot_num = pc & 0x3LL;
+ psr |= (CORE_ADDR)slot_num << 41;
+ store_unsigned_integer (valuep, 8, psr);
+ }
......
Regarding this hunk of code in ia64_sigtramp_frame_prev_register()...
+ else if ((regnum >= IA64_GR32_REGNUM && regnum <= IA64_GR127_REGNUM) ||
+ (regnum >= V32_REGNUM && regnum <= V127_REGNUM))
+ {
+ CORE_ADDR addr = 0;
+ if (regnum >= V32_REGNUM)
+ regnum = IA64_GR32_REGNUM + (regnum - V32_REGNUM);
+ addr = cache->saved_regs[regnum];
+ if (addr != 0)
+ {
+ *lvalp = lval_memory;
+ *addrp = addr;
+ read_memory (addr, valuep, register_size (current_gdbarch, regnum));
+ }
+ }
Could you add a comment explaining why the normal method of computing
V32 (via ia64_pseudo_register_read()) is inadequate?
Also, I'd prefer to see the following line:
+ if (regnum >= V32_REGNUM)
written as
+ if (regnum >= V32_REGNUM && regnum <= V127_REGNUM)
Hmm... doesn't this hunk of code also need to be concerned with
register renames? (I.e, the rotating register stuff...) I'm
wondering why the floating point registers need it, but the
GRs don't.
......
Regarding...
+ else
+ {
+ CORE_ADDR addr = 0;
+ if (IA64_FR32_REGNUM <= regnum && regnum <= IA64_FR127_REGNUM)
+ {
+ /* Fetch floating point register rename base from current
+ frame marker for this frame. */
+ int rrb_fr = (cache->cfm >> 25) & 0x7f;
+
+ /* Adjust the floating point register number to account for
+ register rotation. */
+ regnum = IA64_FR32_REGNUM
+ + ((regnum - IA64_FR32_REGNUM) + rrb_fr) % 96;
+ }
+
+ /* If we have stored a memory address, access the register. */
+ addr = cache->saved_regs[regnum];
+ if (addr != 0)
+ {
+ *lvalp = lval_memory;
+ *addrp = addr;
+ read_memory (addr, valuep, register_size (current_gdbarch, regnum));
+ }
+ }
...could you add a comment at the top of the block which says that
it's intended to handle all the other registers (not handled by the
previous clauses), floating point included. When I first looked at
this, I saw the floating point register rename stuff and figured it
was for just floating point.
Kevin
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: RFA: ia64 tdep patch
2003-10-21 22:22 ` Kevin Buettner
@ 2003-10-21 23:03 ` J. Johnston
2003-10-22 19:38 ` Kevin Buettner
0 siblings, 1 reply; 17+ messages in thread
From: J. Johnston @ 2003-10-21 23:03 UTC (permalink / raw)
To: Kevin Buettner; +Cc: gdb-patches
Kevin Buettner wrote:
> On Oct 20, 5:55pm, J. Johnston wrote:
>
>
>>2003-10-20 Jeff Johnston <jjohnstn@redhat.com>
>>
>> * ia64-tdep.c: (ia64_frame_cache): Add new prev_cfm field.
>
> [...]
>
>> (ia64_sigtramp_frame_init_saved_regs): Bump up base by 16 to get
>> sp needed for calling lower level
>
>
> The entry for ia64_sigtramp_frame_init_saved_regs() doesn't describe
> what was done there. AFAICT, the bumping by 16 part of the patch
> has been removed. You still do the following though:
>
You're looking at my old ChangeLog. I revised the ChangeLog in the last update.
after cleaning up the patch alot. The truth was that the cache->base was
incorrect so I had to adjust it. The 16 belongs in calculating the base. The
previous code for calculating the base was hoping that cache->mem_stack_size
would be set by examine_prologue(), but that does not occur. I found I had to
fudge the value to make it line up with accessing the sigframe. The new code
adds the hard-coded 16 to the base and I have added a comment about this.
> @@ -1869,10 +1913,8 @@ ia64_sigtramp_frame_init_saved_regs (str
> SIGCONTEXT_REGISTER_ADDRESS (cache->base, IA64_CFM_REGNUM);
> cache->saved_regs[IA64_PSR_REGNUM] =
> SIGCONTEXT_REGISTER_ADDRESS (cache->base, IA64_PSR_REGNUM);
> -#if 0
> cache->saved_regs[IA64_BSP_REGNUM] =
> - SIGCONTEXT_REGISTER_ADDRESS (frame->frame, IA64_BSP_REGNUM);
> -#endif
> + SIGCONTEXT_REGISTER_ADDRESS (cache->base, IA64_BSP_REGNUM);
> cache->saved_regs[IA64_RNAT_REGNUM] =
> SIGCONTEXT_REGISTER_ADDRESS (cache->base, IA64_RNAT_REGNUM);
> cache->saved_regs[IA64_CCV_REGNUM] =
> @@ -1886,9 +1928,8 @@ ia64_sigtramp_frame_init_saved_regs (str
> cache->saved_regs[IA64_LC_REGNUM] =
> SIGCONTEXT_REGISTER_ADDRESS (cache->base, IA64_LC_REGNUM);
> for (regno = IA64_GR1_REGNUM; regno <= IA64_GR31_REGNUM; regno++)
> - if (regno != sp_regnum)
> - cache->saved_regs[regno] =
> - SIGCONTEXT_REGISTER_ADDRESS (cache->base, regno);
> + cache->saved_regs[regno] =
> + SIGCONTEXT_REGISTER_ADDRESS (cache->base, regno);
> for (regno = IA64_BR0_REGNUM; regno <= IA64_BR7_REGNUM; regno++)
> cache->saved_regs[regno] =
> SIGCONTEXT_REGISTER_ADDRESS (cache->base, regno);
>
> ......
>
> The code (below) in ia64_sigtramp_frame_prev_register() which computes
> PSR doesn't look right to me. Could you check it? (If it is right,
> please explain it...)
>
I'll explain my logic. As you know, the VRAP address is the return address.
AFAICT by reading the ABI and insn set, there is no information about what the
return address is set to when the branch is in slot 0 or 1 (i.e. is the return
address the next bundle or the next slot?). The ip register isn't supposed to
contain the slot number; it is encoded in the PSR register. When gdb gets the
pc value, it forms it by unwinding the PSR and IP registers - getting the slot
number from the PSR and the IP register address to form a virtual pc address. I
did not want to get the slot number wrong if it was encoded in the return
address so this is why I masked it off above. The PSR register is only used by
gdb in unwinding the pc.
> + else if (regnum == IA64_PSR_REGNUM)
> + {
> + ULONGEST slot_num = 0;
> + CORE_ADDR pc= 0;
> + CORE_ADDR psr = 0;
> + CORE_ADDR addr = cache->saved_regs[IA64_VRAP_REGNUM];
> +
> + if (addr != 0)
> + {
> + *lvalp = lval_memory;
> + *addrp = addr;
> + read_memory (addr, buf, register_size (current_gdbarch, IA64_IP_REGNUM));
> + pc = extract_unsigned_integer (buf, 8);
> + }
> + psr &= ~(3LL << 41);
> + slot_num = pc & 0x3LL;
> + psr |= (CORE_ADDR)slot_num << 41;
> + store_unsigned_integer (valuep, 8, psr);
> + }
>
> ......
>
> Regarding this hunk of code in ia64_sigtramp_frame_prev_register()...
>
> + else if ((regnum >= IA64_GR32_REGNUM && regnum <= IA64_GR127_REGNUM) ||
> + (regnum >= V32_REGNUM && regnum <= V127_REGNUM))
> + {
> + CORE_ADDR addr = 0;
> + if (regnum >= V32_REGNUM)
> + regnum = IA64_GR32_REGNUM + (regnum - V32_REGNUM);
> + addr = cache->saved_regs[regnum];
> + if (addr != 0)
> + {
> + *lvalp = lval_memory;
> + *addrp = addr;
> + read_memory (addr, valuep, register_size (current_gdbarch, regnum));
> + }
> + }
>
> Could you add a comment explaining why the normal method of computing
> V32 (via ia64_pseudo_register_read()) is inadequate?
>
I don't know. I had this for safety reasons already in the
ia64_frame_prev_register() because I didn't know if it might be called with the
pseudo register number or not. This code was copied. Should it be removed in
both places?
> Also, I'd prefer to see the following line:
>
> + if (regnum >= V32_REGNUM)
>
> written as
>
> + if (regnum >= V32_REGNUM && regnum <= V127_REGNUM)
>
Ok.
>
> Hmm... doesn't this hunk of code also need to be concerned with
> register renames? (I.e, the rotating register stuff...) I'm
> wondering why the floating point registers need it, but the
> GRs don't.
>
This code was copied from ia64_frame_prev_register() as it used to be called to
do the underlying work.
The stuff at the end of examine_prologue() handles rotating GRs for the normal
case but doesn't for floating point registers. I would doubt very much that the
signal trampoline uses rotating registers so I probably should remove it for the
floating-point case.
> ......
>
> Regarding...
>
> + else
> + {
> + CORE_ADDR addr = 0;
> + if (IA64_FR32_REGNUM <= regnum && regnum <= IA64_FR127_REGNUM)
> + {
> + /* Fetch floating point register rename base from current
> + frame marker for this frame. */
> + int rrb_fr = (cache->cfm >> 25) & 0x7f;
> +
> + /* Adjust the floating point register number to account for
> + register rotation. */
> + regnum = IA64_FR32_REGNUM
> + + ((regnum - IA64_FR32_REGNUM) + rrb_fr) % 96;
> + }
> +
> + /* If we have stored a memory address, access the register. */
> + addr = cache->saved_regs[regnum];
> + if (addr != 0)
> + {
> + *lvalp = lval_memory;
> + *addrp = addr;
> + read_memory (addr, valuep, register_size (current_gdbarch, regnum));
> + }
> + }
>
> ...could you add a comment at the top of the block which says that
> it's intended to handle all the other registers (not handled by the
> previous clauses), floating point included. When I first looked at
> this, I saw the floating point register rename stuff and figured it
> was for just floating point.
>
Yes.
> Kevin
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: RFA: ia64 tdep patch
2003-10-21 23:03 ` J. Johnston
@ 2003-10-22 19:38 ` Kevin Buettner
2003-10-22 20:57 ` J. Johnston
0 siblings, 1 reply; 17+ messages in thread
From: Kevin Buettner @ 2003-10-22 19:38 UTC (permalink / raw)
To: J. Johnston, Kevin Buettner; +Cc: gdb-patches
On Oct 21, 7:03pm, J. Johnston wrote:
> You're looking at my old ChangeLog.
I've looked at your most recent patch, but do not see the new ChangeLog
entry. Would you mind posting it?
> > The code (below) in ia64_sigtramp_frame_prev_register() which computes
> > PSR doesn't look right to me. Could you check it? (If it is right,
> > please explain it...)
>
> I'll explain my logic. As you know, the VRAP address is the return address.
> AFAICT by reading the ABI and insn set, there is no information about what the
> return address is set to when the branch is in slot 0 or 1 (i.e. is the return
> address the next bundle or the next slot?). The ip register isn't supposed to
> contain the slot number; it is encoded in the PSR register. When gdb gets the
> pc value, it forms it by unwinding the PSR and IP registers - getting the slot
> number from the PSR and the IP register address to form a virtual pc address. I
> did not want to get the slot number wrong if it was encoded in the return
> address so this is why I masked it off above. The PSR register is only used by
> gdb in unwinding the pc.
Thanks for the explanation. Could you please add a brief comment to the
code.
> > Regarding this hunk of code in ia64_sigtramp_frame_prev_register()...
> >
> > + else if ((regnum >= IA64_GR32_REGNUM && regnum <= IA64_GR127_REGNUM) ||
> > + (regnum >= V32_REGNUM && regnum <= V127_REGNUM))
> > + {
> > + CORE_ADDR addr = 0;
> > + if (regnum >= V32_REGNUM)
> > + regnum = IA64_GR32_REGNUM + (regnum - V32_REGNUM);
> > + addr = cache->saved_regs[regnum];
> > + if (addr != 0)
> > + {
> > + *lvalp = lval_memory;
> > + *addrp = addr;
> > + read_memory (addr, valuep, register_size (current_gdbarch, regnum));
> > + }
> > + }
> >
> > Could you add a comment explaining why the normal method of computing
> > V32 (via ia64_pseudo_register_read()) is inadequate?
>
> I don't know. I had this for safety reasons already in the
> ia64_frame_prev_register() because I didn't know if it might be
> called with the pseudo register number or not. This code was
> copied. Should it be removed in both places?
I don't know. I've been studying the code and am wondering why the
V32 ... V127 pseudo regs were introduced at all. Could you remind
me of the reason?
> > Hmm... doesn't this hunk of code also need to be concerned with
> > register renames? (I.e, the rotating register stuff...) I'm
> > wondering why the floating point registers need it, but the
> > GRs don't.
> >
>
> This code was copied from ia64_frame_prev_register() as it used to be called to
> do the underlying work.
>
> The stuff at the end of examine_prologue() handles rotating GRs for
> the normal case but doesn't for floating point registers. I would
> doubt very much that the signal trampoline uses rotating registers
> so I probably should remove it for the floating-point case.
I think you're probably right.
It'd be nice though if we could arrange for the code which handles
rotating registers for floating point and general regs to appear
in the same function. (This can happen in a different patch though.)
Kevin
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: RFA: ia64 tdep patch
2003-10-22 19:38 ` Kevin Buettner
@ 2003-10-22 20:57 ` J. Johnston
2003-10-22 22:01 ` J. Johnston
2003-10-22 22:03 ` Marcel Moolenaar
0 siblings, 2 replies; 17+ messages in thread
From: J. Johnston @ 2003-10-22 20:57 UTC (permalink / raw)
To: Kevin Buettner; +Cc: gdb-patches
Kevin Buettner wrote:
> On Oct 21, 7:03pm, J. Johnston wrote:
>
>
>>You're looking at my old ChangeLog.
>
>
> I've looked at your most recent patch, but do not see the new ChangeLog
> entry. Would you mind posting it?
>
2003-10-20 Jeff Johnston <jjohnstn@redhat.com>
* ia64-tdep.c: (ia64_frame_cache): Add new prev_cfm field.
(ia64_alloc_frame_cache): Initialize new prev_cfm field to 0.
(floatformat_valid): New static routine.
(floatformat_ia64_ext): Add name field and set up is_valid routine
to floatformat_valid().
(examine_prologue): For the previous cfm, use frame_unwind_register()
if the cfm is not stored in a register-stack register. Save the
previous cfm value in the prev_cfm field. Add debug output.
(ia64_frame_this_id): Use frame_id_build_special() to also register
the bsp. Add debug output.
(ia64_sigtramp_frame_this_id): Ditto.
(ia64_frame_prev_register): Look at cache saved_regs for a few more
registers and also add some checks for framelessness before accepting
current register values for fields such as return address. For cfm,
use the cached prev_cfm field if available. Add debug output.
(ia64_sigtramp_frame_init_saved_regs): Bump up base by 16 to get
sp needed for calling lower level
ia64_linux_sigcontext_register_address(). Also save the
bsp and sp address as part of initialization.
(ia64_sigtramp_frame_cache): Hard-code stack size as it can't be
calculated. Cache the bsp and cfm values.
(ia64_sigtramp_frame_prev_register): Flesh this routine out instead of
using ia64_frame_prev_register(). The saved values for bsp and sp can
be taken from the cache. Add debug output.
(ia64_push_dummy_call): Use frame_id_build_special() to also register
the bsp.
>
>>>The code (below) in ia64_sigtramp_frame_prev_register() which computes
>>>PSR doesn't look right to me. Could you check it? (If it is right,
>>>please explain it...)
>>
>>I'll explain my logic. As you know, the VRAP address is the return address.
>>AFAICT by reading the ABI and insn set, there is no information about what the
>>return address is set to when the branch is in slot 0 or 1 (i.e. is the return
>>address the next bundle or the next slot?). The ip register isn't supposed to
>>contain the slot number; it is encoded in the PSR register. When gdb gets the
>>pc value, it forms it by unwinding the PSR and IP registers - getting the slot
>>number from the PSR and the IP register address to form a virtual pc address. I
>>did not want to get the slot number wrong if it was encoded in the return
>>address so this is why I masked it off above. The PSR register is only used by
>>gdb in unwinding the pc.
>
>
> Thanks for the explanation. Could you please add a brief comment to the
> code.
>
Will do.
>
>>>Regarding this hunk of code in ia64_sigtramp_frame_prev_register()...
>>>
>>>+ else if ((regnum >= IA64_GR32_REGNUM && regnum <= IA64_GR127_REGNUM) ||
>>>+ (regnum >= V32_REGNUM && regnum <= V127_REGNUM))
>>>+ {
>>>+ CORE_ADDR addr = 0;
>>>+ if (regnum >= V32_REGNUM)
>>>+ regnum = IA64_GR32_REGNUM + (regnum - V32_REGNUM);
>>>+ addr = cache->saved_regs[regnum];
>>>+ if (addr != 0)
>>>+ {
>>>+ *lvalp = lval_memory;
>>>+ *addrp = addr;
>>>+ read_memory (addr, valuep, register_size (current_gdbarch, regnum));
>>>+ }
>>>+ }
>>>
>>>Could you add a comment explaining why the normal method of computing
>>>V32 (via ia64_pseudo_register_read()) is inadequate?
>>
>>I don't know. I had this for safety reasons already in the
>>ia64_frame_prev_register() because I didn't know if it might be
>>called with the pseudo register number or not. This code was
>>copied. Should it be removed in both places?
>
>
> I don't know. I've been studying the code and am wondering why the
> V32 ... V127 pseudo regs were introduced at all. Could you remind
> me of the reason?
>
They are needed because r32 to r127 are not accessible via the PTRACE interface.
They are accessed via the bsp. Without flagging them as pseudo-registers, the
regcache code returns 0 for all these registers.
I tell the dwarf to convert references to r32-r127 to be V32-V127 in
ia64_dwarf_reg_to_regnum(). I would guess that a parameter reference in a
previous frame would cause this conversion to occur.
>
>>>Hmm... doesn't this hunk of code also need to be concerned with
>>>register renames? (I.e, the rotating register stuff...) I'm
>>>wondering why the floating point registers need it, but the
>>>GRs don't.
>>>
>>
>>This code was copied from ia64_frame_prev_register() as it used to be called to
>>do the underlying work.
>>
>>The stuff at the end of examine_prologue() handles rotating GRs for
>>the normal case but doesn't for floating point registers. I would
>>doubt very much that the signal trampoline uses rotating registers
>>so I probably should remove it for the floating-point case.
>
>
> I think you're probably right.
>
> It'd be nice though if we could arrange for the code which handles
> rotating registers for floating point and general regs to appear
> in the same function. (This can happen in a different patch though.)
>
> Kevin
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: RFA: ia64 tdep patch
2003-10-22 20:57 ` J. Johnston
@ 2003-10-22 22:01 ` J. Johnston
2003-10-23 16:22 ` J. Johnston
2003-10-22 22:03 ` Marcel Moolenaar
1 sibling, 1 reply; 17+ messages in thread
From: J. Johnston @ 2003-10-22 22:01 UTC (permalink / raw)
To: J. Johnston; +Cc: Kevin Buettner, gdb-patches
J. Johnston wrote:
> Kevin Buettner wrote:
>
>> On Oct 21, 7:03pm, J. Johnston wrote:
>>
>>
>>> You're looking at my old ChangeLog.
>>
>>
>>
>> I've looked at your most recent patch, but do not see the new ChangeLog
>> entry. Would you mind posting it?
>>
>
> 2003-10-20 Jeff Johnston <jjohnstn@redhat.com>
>
> * ia64-tdep.c: (ia64_frame_cache): Add new prev_cfm field.
> (ia64_alloc_frame_cache): Initialize new prev_cfm field to 0.
> (floatformat_valid): New static routine.
> (floatformat_ia64_ext): Add name field and set up is_valid routine
> to floatformat_valid().
> (examine_prologue): For the previous cfm, use frame_unwind_register()
> if the cfm is not stored in a register-stack register. Save the
> previous cfm value in the prev_cfm field. Add debug output.
> (ia64_frame_this_id): Use frame_id_build_special() to also register
> the bsp. Add debug output.
> (ia64_sigtramp_frame_this_id): Ditto.
> (ia64_frame_prev_register): Look at cache saved_regs for a few more
> registers and also add some checks for framelessness before accepting
> current register values for fields such as return address. For cfm,
> use the cached prev_cfm field if available. Add debug output.
> (ia64_sigtramp_frame_init_saved_regs): Bump up base by 16 to get
> sp needed for calling lower level
> ia64_linux_sigcontext_register_address(). Also save the
> bsp and sp address as part of initialization.
> (ia64_sigtramp_frame_cache): Hard-code stack size as it can't be
> calculated. Cache the bsp and cfm values.
> (ia64_sigtramp_frame_prev_register): Flesh this routine out instead of
> using ia64_frame_prev_register(). The saved values for bsp and sp can
> be taken from the cache. Add debug output.
> (ia64_push_dummy_call): Use frame_id_build_special() to also register
> the bsp.
>
>
>>
>>>> The code (below) in ia64_sigtramp_frame_prev_register() which computes
>>>> PSR doesn't look right to me. Could you check it? (If it is right,
>>>> please explain it...)
>>>
>>>
>>> I'll explain my logic. As you know, the VRAP address is the return
>>> address. AFAICT by reading the ABI and insn set, there is no
>>> information about what the return address is set to when the branch
>>> is in slot 0 or 1 (i.e. is the return address the next bundle or the
>>> next slot?). The ip register isn't supposed to contain the slot
>>> number; it is encoded in the PSR register. When gdb gets the pc
>>> value, it forms it by unwinding the PSR and IP registers - getting
>>> the slot number from the PSR and the IP register address to form a
>>> virtual pc address. I did not want to get the slot number wrong if
>>> it was encoded in the return address so this is why I masked it off
>>> above. The PSR register is only used by gdb in unwinding the pc.
>>
>>
>>
>> Thanks for the explanation. Could you please add a brief comment to the
>> code.
>>
>
> Will do.
>
Actually, you were right, I was wrong. The VRAP for sigtramp is the IP register
so it won't have the PSR info in it. The sigcontext stores the PSR so we don't
have to reconstruct it in this case. I have removed the code.
>>
>>>> Regarding this hunk of code in ia64_sigtramp_frame_prev_register()...
>>>>
>>>> + else if ((regnum >= IA64_GR32_REGNUM && regnum <=
>>>> IA64_GR127_REGNUM) ||
>>>> + (regnum >= V32_REGNUM && regnum <= V127_REGNUM))
>>>> + {
>>>> + CORE_ADDR addr = 0;
>>>> + if (regnum >= V32_REGNUM)
>>>> + regnum = IA64_GR32_REGNUM + (regnum - V32_REGNUM);
>>>> + addr = cache->saved_regs[regnum];
>>>> + if (addr != 0)
>>>> + {
>>>> + *lvalp = lval_memory;
>>>> + *addrp = addr;
>>>> + read_memory (addr, valuep, register_size (current_gdbarch,
>>>> regnum));
>>>> + }
>>>> + }
>>>>
>>>> Could you add a comment explaining why the normal method of computing
>>>> V32 (via ia64_pseudo_register_read()) is inadequate?
>>>
>>>
>>> I don't know. I had this for safety reasons already in the
>>> ia64_frame_prev_register() because I didn't know if it might be
>>> called with the pseudo register number or not. This code was
>>> copied. Should it be removed in both places?
>>
>>
>>
>> I don't know. I've been studying the code and am wondering why the
>> V32 ... V127 pseudo regs were introduced at all. Could you remind
>> me of the reason?
>>
>
> They are needed because r32 to r127 are not accessible via the PTRACE
> interface. They are accessed via the bsp. Without flagging them as
> pseudo-registers, the regcache code returns 0 for all these registers.
>
> I tell the dwarf to convert references to r32-r127 to be V32-V127 in
> ia64_dwarf_reg_to_regnum(). I would guess that a parameter reference in
> a previous frame would cause this conversion to occur.
>
I have updated the comment for the pseudo-register enums regarding the fact that
r32 - r127 are not accessible via the ptrace register get/set interfaces.
>>
>>>> Hmm... doesn't this hunk of code also need to be concerned with
>>>> register renames? (I.e, the rotating register stuff...) I'm
>>>> wondering why the floating point registers need it, but the
>>>> GRs don't.
>>>>
>>>
>>> This code was copied from ia64_frame_prev_register() as it used to be
>>> called to do the underlying work.
>>>
>>> The stuff at the end of examine_prologue() handles rotating GRs for
>>> the normal case but doesn't for floating point registers. I would
>>> doubt very much that the signal trampoline uses rotating registers
>>> so I probably should remove it for the floating-point case.
>>
>>
>>
>> I think you're probably right.
>>
>> It'd be nice though if we could arrange for the code which handles
>> rotating registers for floating point and general regs to appear
>> in the same function. (This can happen in a different patch though.)
>>
>> Kevin
>>
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: RFA: ia64 tdep patch
2003-10-22 22:01 ` J. Johnston
@ 2003-10-23 16:22 ` J. Johnston
2003-10-23 17:46 ` Kevin Buettner
0 siblings, 1 reply; 17+ messages in thread
From: J. Johnston @ 2003-10-23 16:22 UTC (permalink / raw)
To: J. Johnston; +Cc: Kevin Buettner, gdb-patches
[-- Attachment #1: Type: text/plain, Size: 7913 bytes --]
I have included a patch which contains responses to your comments thus far.
The ChangeLog is:
2003-10-23 Jeff Johnston <jjohnstn@redhat.com>
* ia64-tdep.c: (ia64_frame_cache): Add new prev_cfm field.
(pseudo_regs): Add comment regarding register stack registers.
(ia64_alloc_frame_cache): Initialize new prev_cfm field to 0.
(floatformat_valid): New static routine.
(floatformat_ia64_ext): Add name field and set up is_valid routine
to floatformat_valid().
(examine_prologue): For the previous cfm, use
frame_unwind_register()
if the cfm is not stored in a register-stack register. Save the
previous cfm value in the prev_cfm field. Add debug output.
(ia64_frame_this_id): Use frame_id_build_special() to also register
the bsp. Add debug output.
(ia64_sigtramp_frame_this_id): Ditto.
(ia64_frame_prev_register): Look at cache saved_regs for a few more
registers and also add some checks for framelessness before accepting
current register values for fields such as return address. For cfm,
use the cached prev_cfm field if available. Add comment to explain
PSR logic. Add debug output.
(ia64_sigtramp_frame_init_saved_regs): Bump up base by 16 to get
sp needed for calling lower level
ia64_linux_sigcontext_register_address(). Also save the
bsp and sp address as part of initialization.
(ia64_sigtramp_frame_cache): Hard-code stack size as it can't be
calculated. Cache the bsp and cfm values.
(ia64_sigtramp_frame_prev_register): Add logic to this routine out
instead of using ia64_frame_prev_register() which doesn't expect most
registers to be saved. The saved values for bsp and sp
can be taken from the cache. Add debug output.
(ia64_push_dummy_call): Use frame_id_build_special() to also register
the bsp.
Ok?
-- Jeff J.
J. Johnston wrote:
> J. Johnston wrote:
>
>> Kevin Buettner wrote:
>>
>>> On Oct 21, 7:03pm, J. Johnston wrote:
>>>
>>>
>>>> You're looking at my old ChangeLog.
>>>
>>>
>>>
>>>
>>> I've looked at your most recent patch, but do not see the new ChangeLog
>>> entry. Would you mind posting it?
>>>
>>
>> 2003-10-20 Jeff Johnston <jjohnstn@redhat.com>
>>
>> * ia64-tdep.c: (ia64_frame_cache): Add new prev_cfm field.
>> (ia64_alloc_frame_cache): Initialize new prev_cfm field to 0.
>> (floatformat_valid): New static routine.
>> (floatformat_ia64_ext): Add name field and set up is_valid routine
>> to floatformat_valid().
>> (examine_prologue): For the previous cfm, use
>> frame_unwind_register()
>> if the cfm is not stored in a register-stack register. Save the
>> previous cfm value in the prev_cfm field. Add debug output.
>> (ia64_frame_this_id): Use frame_id_build_special() to also register
>> the bsp. Add debug output.
>> (ia64_sigtramp_frame_this_id): Ditto.
>> (ia64_frame_prev_register): Look at cache saved_regs for a few more
>> registers and also add some checks for framelessness before accepting
>> current register values for fields such as return address. For cfm,
>> use the cached prev_cfm field if available. Add debug output.
>> (ia64_sigtramp_frame_init_saved_regs): Bump up base by 16 to get
>> sp needed for calling lower level
>> ia64_linux_sigcontext_register_address(). Also save the
>> bsp and sp address as part of initialization.
>> (ia64_sigtramp_frame_cache): Hard-code stack size as it can't be
>> calculated. Cache the bsp and cfm values.
>> (ia64_sigtramp_frame_prev_register): Flesh this routine out
>> instead of
>> using ia64_frame_prev_register(). The saved values for bsp and sp
>> can
>> be taken from the cache. Add debug output.
>> (ia64_push_dummy_call): Use frame_id_build_special() to also register
>> the bsp.
>>
>>
>>>
>>>>> The code (below) in ia64_sigtramp_frame_prev_register() which computes
>>>>> PSR doesn't look right to me. Could you check it? (If it is right,
>>>>> please explain it...)
>>>>
>>>>
>>>>
>>>> I'll explain my logic. As you know, the VRAP address is the return
>>>> address. AFAICT by reading the ABI and insn set, there is no
>>>> information about what the return address is set to when the branch
>>>> is in slot 0 or 1 (i.e. is the return address the next bundle or the
>>>> next slot?). The ip register isn't supposed to contain the slot
>>>> number; it is encoded in the PSR register. When gdb gets the pc
>>>> value, it forms it by unwinding the PSR and IP registers - getting
>>>> the slot number from the PSR and the IP register address to form a
>>>> virtual pc address. I did not want to get the slot number wrong if
>>>> it was encoded in the return address so this is why I masked it off
>>>> above. The PSR register is only used by gdb in unwinding the pc.
>>>
>>>
>>>
>>>
>>> Thanks for the explanation. Could you please add a brief comment to the
>>> code.
>>>
>>
>> Will do.
>>
>
> Actually, you were right, I was wrong. The VRAP for sigtramp is the IP
> register so it won't have the PSR info in it. The sigcontext stores the
> PSR so we don't have to reconstruct it in this case. I have removed the
> code.
>
>>>
>>>>> Regarding this hunk of code in ia64_sigtramp_frame_prev_register()...
>>>>>
>>>>> + else if ((regnum >= IA64_GR32_REGNUM && regnum <=
>>>>> IA64_GR127_REGNUM) ||
>>>>> + (regnum >= V32_REGNUM && regnum <= V127_REGNUM))
>>>>> + {
>>>>> + CORE_ADDR addr = 0;
>>>>> + if (regnum >= V32_REGNUM)
>>>>> + regnum = IA64_GR32_REGNUM + (regnum - V32_REGNUM);
>>>>> + addr = cache->saved_regs[regnum];
>>>>> + if (addr != 0)
>>>>> + {
>>>>> + *lvalp = lval_memory;
>>>>> + *addrp = addr;
>>>>> + read_memory (addr, valuep, register_size (current_gdbarch,
>>>>> regnum));
>>>>> + }
>>>>> + }
>>>>>
>>>>> Could you add a comment explaining why the normal method of computing
>>>>> V32 (via ia64_pseudo_register_read()) is inadequate?
>>>>
>>>>
>>>>
>>>> I don't know. I had this for safety reasons already in the
>>>> ia64_frame_prev_register() because I didn't know if it might be
>>>> called with the pseudo register number or not. This code was
>>>> copied. Should it be removed in both places?
>>>
>>>
>>>
>>>
>>> I don't know. I've been studying the code and am wondering why the
>>> V32 ... V127 pseudo regs were introduced at all. Could you remind
>>> me of the reason?
>>>
>>
>> They are needed because r32 to r127 are not accessible via the PTRACE
>> interface. They are accessed via the bsp. Without flagging them as
>> pseudo-registers, the regcache code returns 0 for all these registers.
>>
>> I tell the dwarf to convert references to r32-r127 to be V32-V127 in
>> ia64_dwarf_reg_to_regnum(). I would guess that a parameter reference
>> in a previous frame would cause this conversion to occur.
>>
>
> I have updated the comment for the pseudo-register enums regarding the
> fact that r32 - r127 are not accessible via the ptrace register get/set
> interfaces.
>
>>>
>>>>> Hmm... doesn't this hunk of code also need to be concerned with
>>>>> register renames? (I.e, the rotating register stuff...) I'm
>>>>> wondering why the floating point registers need it, but the
>>>>> GRs don't.
>>>>>
>>>>
>>>> This code was copied from ia64_frame_prev_register() as it used to
>>>> be called to do the underlying work.
>>>>
>>>> The stuff at the end of examine_prologue() handles rotating GRs for
>>>> the normal case but doesn't for floating point registers. I would
>>>> doubt very much that the signal trampoline uses rotating registers
>>>> so I probably should remove it for the floating-point case.
>>>
>>>
>>>
>>>
>>> I think you're probably right.
>>>
>>> It'd be nice though if we could arrange for the code which handles
>>> rotating registers for floating point and general regs to appear
>>> in the same function. (This can happen in a different patch though.)
>>>
>>> Kevin
>>>
>>
>>
>
>
[-- Attachment #2: ia64-tdep.patch2 --]
[-- Type: text/plain, Size: 12811 bytes --]
Index: ia64-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/ia64-tdep.c,v
retrieving revision 1.99
diff -u -p -r1.99 ia64-tdep.c
--- ia64-tdep.c 20 Oct 2003 20:38:07 -0000 1.99
+++ ia64-tdep.c 23 Oct 2003 16:16:56 -0000
@@ -108,7 +108,7 @@ static int fp_regnum = IA64_VFP_REGNUM;
static int lr_regnum = IA64_VRAP_REGNUM;
/* NOTE: we treat the register stack registers r32-r127 as pseudo-registers because
- they are in memory and must be calculated via the bsp register. */
+ they may not be accessible via the ptrace register get/set interfaces. */
enum pseudo_regs { FIRST_PSEUDO_REGNUM = NUM_IA64_RAW_REGS, VBOF_REGNUM = IA64_NAT127_REGNUM + 1, V32_REGNUM,
V127_REGNUM = V32_REGNUM + 95,
VP0_REGNUM, VP16_REGNUM = VP0_REGNUM + 16, VP63_REGNUM = VP0_REGNUM + 63, LAST_PSEUDO_REGNUM };
@@ -232,6 +232,7 @@ struct ia64_frame_cache
CORE_ADDR saved_sp; /* stack pointer for frame */
CORE_ADDR bsp; /* points at r32 for the current frame */
CORE_ADDR cfm; /* cfm value for current frame */
+ CORE_ADDR prev_cfm; /* cfm value for previous frame */
int frameless;
int sof; /* Size of frame (decoded from cfm value) */
int sol; /* Size of locals (decoded from cfm value) */
@@ -316,10 +317,18 @@ ia64_dwarf_reg_to_regnum (int reg)
return reg;
}
+static int
+floatformat_valid (fmt, from)
+ const struct floatformat *fmt;
+ const char *from;
+{
+ return 1;
+}
+
const struct floatformat floatformat_ia64_ext =
{
floatformat_little, 82, 0, 1, 17, 65535, 0x1ffff, 18, 64,
- floatformat_intbit_yes
+ floatformat_intbit_yes, "floatformat_ia64_ext", floatformat_valid
};
@@ -1030,6 +1039,7 @@ ia64_alloc_frame_cache (void)
cache->base = 0;
cache->pc = 0;
cache->cfm = 0;
+ cache->prev_cfm = 0;
cache->sof = 0;
cache->sol = 0;
cache->sor = 0;
@@ -1450,9 +1460,20 @@ examine_prologue (CORE_ADDR pc, CORE_ADD
/* For the previous argument registers we require the previous bof.
If we can't find the previous cfm, then we can do nothing. */
+ cfm = 0;
if (cache->saved_regs[IA64_CFM_REGNUM] != 0)
{
cfm = read_memory_integer (cache->saved_regs[IA64_CFM_REGNUM], 8);
+ }
+ else if (cfm_reg != 0)
+ {
+ frame_unwind_register (next_frame, cfm_reg, buf);
+ cfm = extract_unsigned_integer (buf, 8);
+ }
+ cache->prev_cfm = cfm;
+
+ if (cfm != 0)
+ {
sor = ((cfm >> 14) & 0xf) * 8;
sof = (cfm & 0x7f);
sol = (cfm >> 7) & 0x7f;
@@ -1564,7 +1585,11 @@ ia64_frame_this_id (struct frame_info *n
if (cache->base == 0)
return;
- (*this_id) = frame_id_build (cache->base, cache->pc);
+ (*this_id) = frame_id_build_special (cache->base, cache->pc, cache->bsp);
+ if (gdbarch_debug >= 1)
+ fprintf_unfiltered (gdb_stdlog,
+ "regular frame id: code %lx, stack %lx, special %lx, next_frame %p\n",
+ this_id->code_addr, this_id->stack_addr, cache->bsp, next_frame);
}
static void
@@ -1628,18 +1653,20 @@ ia64_frame_prev_register (struct frame_i
}
else if (regnum == IA64_CFM_REGNUM)
{
- CORE_ADDR addr = 0;
-
- if (cache->frameless)
+ CORE_ADDR addr = cache->saved_regs[IA64_CFM_REGNUM];
+
+ if (addr != 0)
{
- CORE_ADDR cfm = 0;
- frame_unwind_register (next_frame, IA64_PFS_REGNUM, valuep);
+ *lvalp = lval_memory;
+ *addrp = addr;
+ read_memory (addr, valuep, register_size (current_gdbarch, regnum));
}
- else
+ else if (cache->prev_cfm)
+ store_unsigned_integer (valuep, register_size (current_gdbarch, regnum), cache->prev_cfm);
+ else if (cache->frameless)
{
- addr = cache->saved_regs[IA64_CFM_REGNUM];
- if (addr != 0)
- read_memory (addr, valuep, register_size (current_gdbarch, regnum));
+ CORE_ADDR cfm = 0;
+ frame_unwind_register (next_frame, IA64_PFS_REGNUM, valuep);
}
}
else if (regnum == IA64_VFP_REGNUM)
@@ -1727,53 +1754,68 @@ ia64_frame_prev_register (struct frame_i
else if (regnum == IA64_IP_REGNUM)
{
CORE_ADDR pc = 0;
+ CORE_ADDR addr = cache->saved_regs[IA64_VRAP_REGNUM];
- if (cache->frameless)
+ if (addr != 0)
{
- frame_unwind_register (next_frame, IA64_BR0_REGNUM, buf);
+ *lvalp = lval_memory;
+ *addrp = addr;
+ read_memory (addr, buf, register_size (current_gdbarch, IA64_IP_REGNUM));
pc = extract_unsigned_integer (buf, 8);
}
- else
+ else if (cache->frameless)
{
- CORE_ADDR addr = cache->saved_regs[IA64_VRAP_REGNUM];
- if (addr != 0)
- {
- read_memory (addr, buf, register_size (current_gdbarch, IA64_IP_REGNUM));
- pc = extract_unsigned_integer (buf, 8);
- }
+ frame_unwind_register (next_frame, IA64_BR0_REGNUM, buf);
+ pc = extract_unsigned_integer (buf, 8);
}
pc &= ~0xf;
store_unsigned_integer (valuep, 8, pc);
}
else if (regnum == IA64_PSR_REGNUM)
{
+ /* We don't know how to get the complete previous PSR, but we need it for
+ the slot information when we unwind the pc (pc is formed of IP register
+ plus slot information from PSR). To get the previous slot information,
+ we mask it off the return address. */
ULONGEST slot_num = 0;
CORE_ADDR pc= 0;
CORE_ADDR psr = 0;
+ CORE_ADDR addr = cache->saved_regs[IA64_VRAP_REGNUM];
frame_unwind_register (next_frame, IA64_PSR_REGNUM, buf);
psr = extract_unsigned_integer (buf, 8);
- if (cache->frameless)
+ if (addr != 0)
{
- CORE_ADDR pc;
- frame_unwind_register (next_frame, IA64_BR0_REGNUM, buf);
+ *lvalp = lval_memory;
+ *addrp = addr;
+ read_memory (addr, buf, register_size (current_gdbarch, IA64_IP_REGNUM));
pc = extract_unsigned_integer (buf, 8);
}
- else
+ else if (cache->frameless)
{
- CORE_ADDR addr = cache->saved_regs[IA64_VRAP_REGNUM];
- if (addr != 0)
- {
- read_memory (addr, buf, register_size (current_gdbarch, IA64_IP_REGNUM));
- pc = extract_unsigned_integer (buf, 8);
- }
+ CORE_ADDR pc;
+ frame_unwind_register (next_frame, IA64_BR0_REGNUM, buf);
+ pc = extract_unsigned_integer (buf, 8);
}
psr &= ~(3LL << 41);
slot_num = pc & 0x3LL;
psr |= (CORE_ADDR)slot_num << 41;
store_unsigned_integer (valuep, 8, psr);
}
+ else if (regnum == IA64_BR0_REGNUM)
+ {
+ CORE_ADDR br0 = 0;
+ CORE_ADDR addr = cache->saved_regs[IA64_BR0_REGNUM];
+ if (addr != 0)
+ {
+ *lvalp = lval_memory;
+ *addrp = addr;
+ read_memory (addr, buf, register_size (current_gdbarch, IA64_BR0_REGNUM));
+ br0 = extract_unsigned_integer (buf, 8);
+ }
+ store_unsigned_integer (valuep, 8, br0);
+ }
else if ((regnum >= IA64_GR32_REGNUM && regnum <= IA64_GR127_REGNUM) ||
(regnum >= V32_REGNUM && regnum <= V127_REGNUM))
{
@@ -1839,6 +1881,12 @@ ia64_frame_prev_register (struct frame_i
else
frame_unwind_register (next_frame, regnum, valuep);
}
+
+ if (gdbarch_debug >= 1)
+ fprintf_unfiltered (gdb_stdlog,
+ "regular prev register <%d> <%s> is %lx\n", regnum,
+ (((unsigned) regnum <= IA64_NAT127_REGNUM)
+ ? ia64_register_names[regnum] : "r??"), extract_unsigned_integer (valuep, 8));
}
static const struct frame_unwind ia64_frame_unwind =
@@ -1869,10 +1917,8 @@ ia64_sigtramp_frame_init_saved_regs (str
SIGCONTEXT_REGISTER_ADDRESS (cache->base, IA64_CFM_REGNUM);
cache->saved_regs[IA64_PSR_REGNUM] =
SIGCONTEXT_REGISTER_ADDRESS (cache->base, IA64_PSR_REGNUM);
-#if 0
cache->saved_regs[IA64_BSP_REGNUM] =
- SIGCONTEXT_REGISTER_ADDRESS (frame->frame, IA64_BSP_REGNUM);
-#endif
+ SIGCONTEXT_REGISTER_ADDRESS (cache->base, IA64_BSP_REGNUM);
cache->saved_regs[IA64_RNAT_REGNUM] =
SIGCONTEXT_REGISTER_ADDRESS (cache->base, IA64_RNAT_REGNUM);
cache->saved_regs[IA64_CCV_REGNUM] =
@@ -1886,9 +1932,8 @@ ia64_sigtramp_frame_init_saved_regs (str
cache->saved_regs[IA64_LC_REGNUM] =
SIGCONTEXT_REGISTER_ADDRESS (cache->base, IA64_LC_REGNUM);
for (regno = IA64_GR1_REGNUM; regno <= IA64_GR31_REGNUM; regno++)
- if (regno != sp_regnum)
- cache->saved_regs[regno] =
- SIGCONTEXT_REGISTER_ADDRESS (cache->base, regno);
+ cache->saved_regs[regno] =
+ SIGCONTEXT_REGISTER_ADDRESS (cache->base, regno);
for (regno = IA64_BR0_REGNUM; regno <= IA64_BR7_REGNUM; regno++)
cache->saved_regs[regno] =
SIGCONTEXT_REGISTER_ADDRESS (cache->base, regno);
@@ -1912,7 +1957,16 @@ ia64_sigtramp_frame_cache (struct frame_
cache = ia64_alloc_frame_cache ();
frame_unwind_register (next_frame, sp_regnum, buf);
- cache->base = extract_unsigned_integer (buf, 8) + cache->mem_stack_frame_size;
+ /* Note that frame size is hard-coded below. We cannot calculate it
+ via prologue examination. */
+ cache->base = extract_unsigned_integer (buf, 8) + 16;
+
+ frame_unwind_register (next_frame, IA64_BSP_REGNUM, buf);
+ cache->bsp = extract_unsigned_integer (buf, 8);
+
+ frame_unwind_register (next_frame, IA64_CFM_REGNUM, buf);
+ cache->cfm = extract_unsigned_integer (buf, 8);
+ cache->sof = cache->cfm & 0x7f;
ia64_sigtramp_frame_init_saved_regs (cache);
@@ -1927,7 +1981,11 @@ ia64_sigtramp_frame_this_id (struct fram
struct ia64_frame_cache *cache =
ia64_sigtramp_frame_cache (next_frame, this_cache);
- (*this_id) = frame_id_build (cache->base, frame_pc_unwind (next_frame));
+ (*this_id) = frame_id_build_special (cache->base, frame_pc_unwind (next_frame), cache->bsp);
+ if (gdbarch_debug >= 1)
+ fprintf_unfiltered (gdb_stdlog,
+ "sigtramp frame id: code %lx, stack %lx, special %lx, next_frame %p\n",
+ this_id->code_addr, this_id->stack_addr, cache->bsp, next_frame);
}
static void
@@ -1937,11 +1995,75 @@ ia64_sigtramp_frame_prev_register (struc
enum lval_type *lvalp, CORE_ADDR *addrp,
int *realnump, void *valuep)
{
- /* Make sure we've initialized the cache. */
- ia64_sigtramp_frame_cache (next_frame, this_cache);
+ char dummy_valp[MAX_REGISTER_SIZE];
+ char buf[MAX_REGISTER_SIZE];
+
+ struct ia64_frame_cache *cache =
+ ia64_sigtramp_frame_cache (next_frame, this_cache);
+
+ gdb_assert (regnum >= 0);
+
+ if (!target_has_registers)
+ error ("No registers.");
+
+ *optimizedp = 0;
+ *addrp = 0;
+ *lvalp = not_lval;
+ *realnump = -1;
+
+ /* Rather than check each time if valuep is non-null, supply a dummy buffer
+ when valuep is not supplied. */
+ if (!valuep)
+ valuep = dummy_valp;
+
+ memset (valuep, 0, register_size (current_gdbarch, regnum));
+
+ if (regnum == IA64_IP_REGNUM)
+ {
+ CORE_ADDR pc = 0;
+ CORE_ADDR addr = cache->saved_regs[IA64_VRAP_REGNUM];
+
+ if (addr != 0)
+ {
+ *lvalp = lval_memory;
+ *addrp = addr;
+ read_memory (addr, buf, register_size (current_gdbarch, IA64_IP_REGNUM));
+ pc = extract_unsigned_integer (buf, 8);
+ }
+ pc &= ~0xf;
+ store_unsigned_integer (valuep, 8, pc);
+ }
+ else if ((regnum >= IA64_GR32_REGNUM && regnum <= IA64_GR127_REGNUM) ||
+ (regnum >= V32_REGNUM && regnum <= V127_REGNUM))
+ {
+ CORE_ADDR addr = 0;
+ if (regnum >= V32_REGNUM)
+ regnum = IA64_GR32_REGNUM + (regnum - V32_REGNUM);
+ addr = cache->saved_regs[regnum];
+ if (addr != 0)
+ {
+ *lvalp = lval_memory;
+ *addrp = addr;
+ read_memory (addr, valuep, register_size (current_gdbarch, regnum));
+ }
+ }
+ else
+ {
+ /* All other registers not listed above. */
+ CORE_ADDR addr = cache->saved_regs[regnum];
+ if (addr != 0)
+ {
+ *lvalp = lval_memory;
+ *addrp = addr;
+ read_memory (addr, valuep, register_size (current_gdbarch, regnum));
+ }
+ }
- ia64_frame_prev_register (next_frame, this_cache, regnum,
- optimizedp, lvalp, addrp, realnump, valuep);
+ if (gdbarch_debug >= 1)
+ fprintf_unfiltered (gdb_stdlog,
+ "sigtramp prev register <%s> is %lx\n",
+ (((unsigned) regnum <= IA64_NAT127_REGNUM)
+ ? ia64_register_names[regnum] : "r??"), extract_unsigned_integer (valuep, 8));
}
static const struct frame_unwind ia64_sigtramp_frame_unwind =
@@ -2473,12 +2595,20 @@ static struct frame_id
ia64_unwind_dummy_id (struct gdbarch *gdbarch, struct frame_info *next_frame)
{
char buf[8];
- CORE_ADDR sp;
+ CORE_ADDR sp, bsp;
frame_unwind_register (next_frame, sp_regnum, buf);
sp = extract_unsigned_integer (buf, 8);
- return frame_id_build (sp, frame_pc_unwind (next_frame));
+ frame_unwind_register (next_frame, IA64_BSP_REGNUM, buf);
+ bsp = extract_unsigned_integer (buf, 8);
+
+ if (gdbarch_debug >= 1)
+ fprintf_unfiltered (gdb_stdlog,
+ "dummy frame id: code %lx, stack %lx, special %lx\n",
+ frame_pc_unwind (next_frame), sp, bsp);
+
+ return frame_id_build_special (sp, frame_pc_unwind (next_frame), bsp);
}
static CORE_ADDR
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: RFA: ia64 tdep patch
2003-10-23 16:22 ` J. Johnston
@ 2003-10-23 17:46 ` Kevin Buettner
2003-10-23 22:07 ` J. Johnston
0 siblings, 1 reply; 17+ messages in thread
From: Kevin Buettner @ 2003-10-23 17:46 UTC (permalink / raw)
To: J. Johnston; +Cc: Kevin Buettner, gdb-patches
On Oct 23, 12:22pm, J. Johnston wrote:
> I have included a patch which contains responses to your comments thus far.
>
> The ChangeLog is:
>
> 2003-10-23 Jeff Johnston <jjohnstn@redhat.com>
> * ia64-tdep.c: (ia64_frame_cache): Add new prev_cfm field.
> (pseudo_regs): Add comment regarding register stack registers.
> (ia64_alloc_frame_cache): Initialize new prev_cfm field to 0.
> (floatformat_valid): New static routine.
> (floatformat_ia64_ext): Add name field and set up is_valid routine
> to floatformat_valid().
> (examine_prologue): For the previous cfm, use
> frame_unwind_register()
> if the cfm is not stored in a register-stack register. Save the
> previous cfm value in the prev_cfm field. Add debug output.
> (ia64_frame_this_id): Use frame_id_build_special() to also register
> the bsp. Add debug output.
> (ia64_sigtramp_frame_this_id): Ditto.
> (ia64_frame_prev_register): Look at cache saved_regs for a few more
> registers and also add some checks for framelessness before accepting
> current register values for fields such as return address. For cfm,
> use the cached prev_cfm field if available. Add comment to explain
> PSR logic. Add debug output.
> (ia64_sigtramp_frame_init_saved_regs): Bump up base by 16 to get
> sp needed for calling lower level
> ia64_linux_sigcontext_register_address(). Also save the
> bsp and sp address as part of initialization.
> (ia64_sigtramp_frame_cache): Hard-code stack size as it can't be
> calculated. Cache the bsp and cfm values.
> (ia64_sigtramp_frame_prev_register): Add logic to this routine out
> instead of using ia64_frame_prev_register() which doesn't expect most
> registers to be saved. The saved values for bsp and sp
> can be taken from the cache. Add debug output.
> (ia64_push_dummy_call): Use frame_id_build_special() to also register
> the bsp.
>
> Ok?
Okay.
Kevin
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: RFA: ia64 tdep patch
2003-10-23 17:46 ` Kevin Buettner
@ 2003-10-23 22:07 ` J. Johnston
0 siblings, 0 replies; 17+ messages in thread
From: J. Johnston @ 2003-10-23 22:07 UTC (permalink / raw)
To: Kevin Buettner; +Cc: gdb-patches
Kevin Buettner wrote:
> On Oct 23, 12:22pm, J. Johnston wrote:
>
>
>>I have included a patch which contains responses to your comments thus far.
>>
>>The ChangeLog is:
>>
>>2003-10-23 Jeff Johnston <jjohnstn@redhat.com>
>> * ia64-tdep.c: (ia64_frame_cache): Add new prev_cfm field.
>> (pseudo_regs): Add comment regarding register stack registers.
>> (ia64_alloc_frame_cache): Initialize new prev_cfm field to 0.
>> (floatformat_valid): New static routine.
>> (floatformat_ia64_ext): Add name field and set up is_valid routine
>> to floatformat_valid().
>> (examine_prologue): For the previous cfm, use
>> frame_unwind_register()
>> if the cfm is not stored in a register-stack register. Save the
>> previous cfm value in the prev_cfm field. Add debug output.
>> (ia64_frame_this_id): Use frame_id_build_special() to also register
>> the bsp. Add debug output.
>> (ia64_sigtramp_frame_this_id): Ditto.
>> (ia64_frame_prev_register): Look at cache saved_regs for a few more
>> registers and also add some checks for framelessness before accepting
>> current register values for fields such as return address. For cfm,
>> use the cached prev_cfm field if available. Add comment to explain
>> PSR logic. Add debug output.
>> (ia64_sigtramp_frame_init_saved_regs): Bump up base by 16 to get
>> sp needed for calling lower level
>> ia64_linux_sigcontext_register_address(). Also save the
>> bsp and sp address as part of initialization.
>> (ia64_sigtramp_frame_cache): Hard-code stack size as it can't be
>> calculated. Cache the bsp and cfm values.
>> (ia64_sigtramp_frame_prev_register): Add logic to this routine out
>> instead of using ia64_frame_prev_register() which doesn't expect most
>> registers to be saved. The saved values for bsp and sp
>> can be taken from the cache. Add debug output.
>> (ia64_push_dummy_call): Use frame_id_build_special() to also register
>> the bsp.
>>
>>Ok?
>
>
> Okay.
>
Thanks. Patch checked into mainline.
-- Jeff J.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: RFA: ia64 tdep patch
2003-10-22 20:57 ` J. Johnston
2003-10-22 22:01 ` J. Johnston
@ 2003-10-22 22:03 ` Marcel Moolenaar
2003-10-23 21:01 ` Kevin Buettner
1 sibling, 1 reply; 17+ messages in thread
From: Marcel Moolenaar @ 2003-10-22 22:03 UTC (permalink / raw)
To: J. Johnston; +Cc: Kevin Buettner, gdb-patches
On Wed, Oct 22, 2003 at 04:57:34PM -0400, J. Johnston wrote:
> >>>
> >>>Could you add a comment explaining why the normal method of computing
> >>>V32 (via ia64_pseudo_register_read()) is inadequate?
> >>
> >>I don't know. I had this for safety reasons already in the
> >>ia64_frame_prev_register() because I didn't know if it might be
> >>called with the pseudo register number or not. This code was
> >>copied. Should it be removed in both places?
> >
> >
> >I don't know. I've been studying the code and am wondering why the
> >V32 ... V127 pseudo regs were introduced at all. Could you remind
> >me of the reason?
> >
>
> They are needed because r32 to r127 are not accessible via the PTRACE
> interface. They are accessed via the bsp. Without flagging them as
> pseudo-registers, the regcache code returns 0 for all these registers.
It depends. For FreeBSD I added ptrace(2) functions to get and set
stacked registers that are on the kernel stack. The problem more
generally is that registers above bspstore (but below bsp) are
not accessable in memory. I think it's better for gdb to keep the
distinction between stacked registers on the backing store and
"dirty" stacked registers. The distinction avoids that gdb makes
assumptions that are only valid on Linux or even only for the native
code.
BTW: I have partial support for FreeBSD/ia64. I'll send patches as
soon as I feel that the backtrace is reliable enough.
--
Marcel Moolenaar USPA: A-39004 marcel@xcllnt.net
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: RFA: ia64 tdep patch
2003-10-22 22:03 ` Marcel Moolenaar
@ 2003-10-23 21:01 ` Kevin Buettner
2003-10-23 23:23 ` Marcel Moolenaar
0 siblings, 1 reply; 17+ messages in thread
From: Kevin Buettner @ 2003-10-23 21:01 UTC (permalink / raw)
To: Marcel Moolenaar, J. Johnston; +Cc: Kevin Buettner, gdb-patches
On Oct 22, 3:02pm, Marcel Moolenaar wrote:
> > They are needed because r32 to r127 are not accessible via the PTRACE
> > interface. They are accessed via the bsp. Without flagging them as
> > pseudo-registers, the regcache code returns 0 for all these registers.
>
> It depends. For FreeBSD I added ptrace(2) functions to get and set
> stacked registers that are on the kernel stack. The problem more
> generally is that registers above bspstore (but below bsp) are
> not accessable in memory. I think it's better for gdb to keep the
> distinction between stacked registers on the backing store and
> "dirty" stacked registers. The distinction avoids that gdb makes
> assumptions that are only valid on Linux or even only for the native
> code.
Unfortunately, the assumptions that you mention are already in place.
(And have been in place for quite some time).
> BTW: I have partial support for FreeBSD/ia64. I'll send patches as
> soon as I feel that the backtrace is reliable enough.
Patches will most certainly be welcome. Do you have an FSF copyright
assignment for GDB yet? If not, you might want to start working on
the paperwork now...
Kevin
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: RFA: ia64 tdep patch
2003-10-23 21:01 ` Kevin Buettner
@ 2003-10-23 23:23 ` Marcel Moolenaar
2003-10-24 4:30 ` Kevin Buettner
0 siblings, 1 reply; 17+ messages in thread
From: Marcel Moolenaar @ 2003-10-23 23:23 UTC (permalink / raw)
To: Kevin Buettner; +Cc: J. Johnston, gdb-patches
On Thu, Oct 23, 2003 at 02:00:49PM -0700, Kevin Buettner wrote:
> On Oct 22, 3:02pm, Marcel Moolenaar wrote:
>
> > > They are needed because r32 to r127 are not accessible via the PTRACE
> > > interface. They are accessed via the bsp. Without flagging them as
> > > pseudo-registers, the regcache code returns 0 for all these registers.
> >
> > It depends. For FreeBSD I added ptrace(2) functions to get and set
> > stacked registers that are on the kernel stack. The problem more
> > generally is that registers above bspstore (but below bsp) are
> > not accessable in memory. I think it's better for gdb to keep the
> > distinction between stacked registers on the backing store and
> > "dirty" stacked registers. The distinction avoids that gdb makes
> > assumptions that are only valid on Linux or even only for the native
> > code.
>
> Unfortunately, the assumptions that you mention are already in place.
> (And have been in place for quite some time).
Yes, and it is one of the pickles I'm working on. Do I change FreeBSD
to match the assumption in gdb or do I change gdb to remove the
assumption?
One technical reason for removing the assumption in gdb is that it
is not always possible to flush the dirty registers onto the user
backing store. It could fail when BSPSTORE is close to or at the
boundary of the register stack. This is a border case, but it would
be impossible to debug a process when it actually operates under
these conditions. Also, when flushing the dirty registers onto the
user backing store, we change the state of the process, which may
hide the problem and interfere with debugging. It's mostly academic,
but still a fundamental "flaw" in debugging on ia64.
A technical reason for changing FreeBSD is that it avoids changing
gdb and keeps access to the stacked registers uniform. However, even
though debugging is not performance critical, moving the complexity
into the debugger may avoid unnecessary and unconditional copying
from the kernel stack to the user stack and gives gdb (or any other
program that needs this) control over it...
I'm leaning towards changing gdb. I just need to underdstand better
what I'm getting into. I have little experience with gdb...
> > BTW: I have partial support for FreeBSD/ia64. I'll send patches as
> > soon as I feel that the backtrace is reliable enough.
>
> Patches will most certainly be welcome. Do you have an FSF copyright
> assignment for GDB yet? If not, you might want to start working on
> the paperwork now...
I do not have such assignment, but it's in the pipeline.
--
Marcel Moolenaar USPA: A-39004 marcel@xcllnt.net
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: RFA: ia64 tdep patch
2003-10-23 23:23 ` Marcel Moolenaar
@ 2003-10-24 4:30 ` Kevin Buettner
2003-10-24 5:40 ` Marcel Moolenaar
0 siblings, 1 reply; 17+ messages in thread
From: Kevin Buettner @ 2003-10-24 4:30 UTC (permalink / raw)
To: Marcel Moolenaar, Kevin Buettner; +Cc: J. Johnston, gdb-patches
On Oct 23, 4:22pm, Marcel Moolenaar wrote:
> On Thu, Oct 23, 2003 at 02:00:49PM -0700, Kevin Buettner wrote:
> > On Oct 22, 3:02pm, Marcel Moolenaar wrote:
> >
> > > > They are needed because r32 to r127 are not accessible via the PTRACE
> > > > interface. They are accessed via the bsp. Without flagging them as
> > > > pseudo-registers, the regcache code returns 0 for all these registers.
> > >
> > > It depends. For FreeBSD I added ptrace(2) functions to get and set
> > > stacked registers that are on the kernel stack. The problem more
> > > generally is that registers above bspstore (but below bsp) are
> > > not accessable in memory. I think it's better for gdb to keep the
> > > distinction between stacked registers on the backing store and
> > > "dirty" stacked registers. The distinction avoids that gdb makes
> > > assumptions that are only valid on Linux or even only for the native
> > > code.
> >
> > Unfortunately, the assumptions that you mention are already in place.
> > (And have been in place for quite some time).
>
> Yes, and it is one of the pickles I'm working on. Do I change FreeBSD
> to match the assumption in gdb or do I change gdb to remove the
> assumption?
>
> One technical reason for removing the assumption in gdb is that it
> is not always possible to flush the dirty registers onto the user
> backing store. It could fail when BSPSTORE is close to or at the
> boundary of the register stack. This is a border case, but it would
> be impossible to debug a process when it actually operates under
> these conditions. Also, when flushing the dirty registers onto the
> user backing store, we change the state of the process, which may
> hide the problem and interfere with debugging. It's mostly academic,
> but still a fundamental "flaw" in debugging on ia64.
>
> A technical reason for changing FreeBSD is that it avoids changing
> gdb and keeps access to the stacked registers uniform. However, even
> though debugging is not performance critical, moving the complexity
> into the debugger may avoid unnecessary and unconditional copying
> from the kernel stack to the user stack and gives gdb (or any other
> program that needs this) control over it...
>
> I'm leaning towards changing gdb. I just need to underdstand better
> what I'm getting into. I have little experience with gdb...
If you take a look at the Linux/ia64 implementation of ptrace(),
you'll see that some of this complexity has been moved into the
ptrace() implementation. ptrace() will get/set the user portion of
registers stored in the kernel's register backing store.
From what you said earlier, it sounds as though you'd already
implemented something like this for FreeBSD...
> > > BTW: I have partial support for FreeBSD/ia64. I'll send patches as
> > > soon as I feel that the backtrace is reliable enough.
> >
> > Patches will most certainly be welcome. Do you have an FSF copyright
> > assignment for GDB yet? If not, you might want to start working on
> > the paperwork now...
>
> I do not have such assignment, but it's in the pipeline.
Cool.
Kevin
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: RFA: ia64 tdep patch
2003-10-24 4:30 ` Kevin Buettner
@ 2003-10-24 5:40 ` Marcel Moolenaar
2003-10-24 7:08 ` Kevin Buettner
0 siblings, 1 reply; 17+ messages in thread
From: Marcel Moolenaar @ 2003-10-24 5:40 UTC (permalink / raw)
To: Kevin Buettner; +Cc: J. Johnston, gdb-patches
On Thu, Oct 23, 2003 at 09:30:16PM -0700, Kevin Buettner wrote:
> >
> > I'm leaning towards changing gdb. I just need to underdstand better
> > what I'm getting into. I have little experience with gdb...
>
> If you take a look at the Linux/ia64 implementation of ptrace(),
> you'll see that some of this complexity has been moved into the
> ptrace() implementation. ptrace() will get/set the user portion of
> registers stored in the kernel's register backing store.
Yes. The PEEK{DATA|TEXT} and POKE{DATA|TEXT} functions interpret
addresses between [bspstore, bsp> differently. Besides hiding the
fact that there are registers on the kernel stack that belong to
the process it also has the drawback that it makes it impossible
to actually read from the backing store between [bspstore, bsp>.
Also, reading from and writing to user memory is common code in
FreeBSD. I hate to make ia64 a special case only to hide some
details that a ptrace(2) user can deal with without too much
problems.
> >From what you said earlier, it sounds as though you'd already
> implemented something like this for FreeBSD...
It still requires a special ptrace(2) call, making the distinction
between flushed and dirty stacked registers explicit to gdb. It also
doesn't address how to handle coredumps. Having the dirty registers
in some note section to avoid modifying the state of the process
represented by the core file is appealing to me. This too makes it
explicit that dirty registers are special WRT flushed registers.
Again: by hiding the details, we obfuscate the true state of a
process. I can't stop thinking how that can make some class of
failures undebuggable (e.g. bugs related to NaT collections, near
stack overflow conditions, context switching bugs or maybe even
setjmp/longjmp bugs).
Thoughts?
--
Marcel Moolenaar USPA: A-39004 marcel@xcllnt.net
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: RFA: ia64 tdep patch
2003-10-24 5:40 ` Marcel Moolenaar
@ 2003-10-24 7:08 ` Kevin Buettner
0 siblings, 0 replies; 17+ messages in thread
From: Kevin Buettner @ 2003-10-24 7:08 UTC (permalink / raw)
To: Marcel Moolenaar, Kevin Buettner; +Cc: J. Johnston, gdb-patches
On Oct 23, 10:40pm, Marcel Moolenaar wrote:
> > >From what you said earlier, it sounds as though you'd already
> > implemented something like this for FreeBSD...
>
> It still requires a special ptrace(2) call, making the distinction
> between flushed and dirty stacked registers explicit to gdb. It also
> doesn't address how to handle coredumps. Having the dirty registers
> in some note section to avoid modifying the state of the process
> represented by the core file is appealing to me. This too makes it
> explicit that dirty registers are special WRT flushed registers.
>
> Again: by hiding the details, we obfuscate the true state of a
> process. I can't stop thinking how that can make some class of
> failures undebuggable (e.g. bugs related to NaT collections, near
> stack overflow conditions, context switching bugs or maybe even
> setjmp/longjmp bugs).
>
> Thoughts?
I believe that the Linux/ia64 ptrace() code handles the problems with
NaT collections. I'm not sure how it behaves with regard to the other
conditions that you mention. (Though for things like context
switching bugs, that's really not a user space problem.)
IMO, this stuff is really arcane and there's going to be some hair
somewhere. When I was doing the Linux/ia64 port for GDB, I was happy
that a lot of the hair was in ptrace() so that GDB could be presented
with a somewhat simplified interface. I also did the now defunct
IA-64 GDB port for AIX and was pleased to find that the kernel authors
seemed to handle these issues in much the same way as Linux. That
said, I've been expecting some kind of debug interface for IA-64 to
emerge which wants to use a model which is closer to the metal than
what GDB currently expects. If that's the way you decide to go, I'm
willing to review the necessary IA-64 related patches to make it
happen.
Kevin
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2003-10-24 7:08 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-10-17 19:58 RFA: ia64 tdep patch J. Johnston
2003-10-20 20:13 ` Kevin Buettner
2003-10-20 21:55 ` J. Johnston
2003-10-21 22:22 ` Kevin Buettner
2003-10-21 23:03 ` J. Johnston
2003-10-22 19:38 ` Kevin Buettner
2003-10-22 20:57 ` J. Johnston
2003-10-22 22:01 ` J. Johnston
2003-10-23 16:22 ` J. Johnston
2003-10-23 17:46 ` Kevin Buettner
2003-10-23 22:07 ` J. Johnston
2003-10-22 22:03 ` Marcel Moolenaar
2003-10-23 21:01 ` Kevin Buettner
2003-10-23 23:23 ` Marcel Moolenaar
2003-10-24 4:30 ` Kevin Buettner
2003-10-24 5:40 ` Marcel Moolenaar
2003-10-24 7:08 ` Kevin Buettner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox