This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
[PATCH] Fix s390 problems with current GCC
- From: "Ulrich Weigand" <weigand at i1 dot informatik dot uni-erlangen dot de>
- To: gdb-patches at sources dot redhat dot com
- Date: Sat, 23 Oct 2004 01:36:39 +0200 (CEST)
- Subject: [PATCH] Fix s390 problems with current GCC
Hello,
we recently changed GCC to use the stack more efficiently; in particular it
now no longer saves/restores registers into fixed stack frame locations.
Unfortunately, gdb currently makes assumptions that rely on the old GCC
behaviour, and thus break when using current GCC. In particular:
- s390_analyze_prologue assumes registers are saved only to within a
particular range of locations in the stack frame; new code may save
outside that range.
- s390_push_dummy_call assumes it can store a back chain into the
register save area presented to the inferior function, and that
s390_unwind_dummy_id can read it back; new code may use the whole
register save area for any purpose (as according to the ABI) and
may thus clobber that back chain.
The following patch fixes both problems. I've rewritten the prologue
analyzer to track any store of an original incoming register value to
the stack at a constant offset relative to the CFA; this allows gdb
to correctly recognize both old-style and new-style prologs.
As to dummy calls, I'm now using the bottom of the argument save area
as 'stack' part of the dummy frame ID, not the top. This is just as
well for the rest of gdb, but has the advantage that s390_unwind_dummy_id
now no longer needs to know the size of the argument area, and thus no
longer needs to read anything from the inferior stack.
I've tested the patch on s390-ibm-linux and s390x-ibm-linux, both against
GCC 3.4 (old-style) and current GCC head (new-style), both as-is and with
the DWARF-2 frame analyzer forcibly disabled (to exercise the prolog-
analyzer based frame handling). In all cases the patch fixes test case
failures, but introduces no regressions.
OK to commit?
(B.t.w. do I require approval for s390-only patches?)
Bye,
Ulrich
ChangeLog:
* s390-tdep.c (enum pv_boolean): Remove.
(pv_is_array_ref): Remove.
(s390_on_stack): Remove.
(S390_NUM_SPILL_SLOTS): Remove.
(struct s390_prologue_data): Remove members 'spill' and 'back_chain',
add members 'gpr_slot', 'fpr_slot', and 'back_chain_saved_p'.
(s390_store): Track all stores of orginal incoming registers to the
stack constant offsets relative to the CFA, instead of only tracking
stores into specific spill slots.
(s390_load): Likewise.
(s390_analyze_prologue): Adapt to struct s390_prologue_data changes.
(s390_prologue_frame_unwind_cache): Likewise. Only track registers
defined as call-saved by the ABI.
(s390_push_dummy_call): Use bottom of dummy call argument save area
as return value, not the top. Do not store to the called function's
register save area.
(s390_unwind_dummy_id): Adapt accordingly.
Index: gdb/s390-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/s390-tdep.c,v
retrieving revision 1.135
diff -c -p -r1.135 s390-tdep.c
*** gdb/s390-tdep.c 31 Jul 2004 21:53:17 -0000 1.135
--- gdb/s390-tdep.c 22 Oct 2004 23:32:17 -0000
*************** pv_is_register (struct prologue_value *a
*** 801,866 ****
}
- /* A prologue-value-esque boolean type, including "maybe", when we
- can't figure out whether something is true or not. */
- enum pv_boolean {
- pv_maybe,
- pv_definite_yes,
- pv_definite_no,
- };
-
-
- /* Decide whether a reference to SIZE bytes at ADDR refers exactly to
- an element of an array. The array starts at ARRAY_ADDR, and has
- ARRAY_LEN values of ELT_SIZE bytes each. If ADDR definitely does
- refer to an array element, set *I to the index of the referenced
- element in the array, and return pv_definite_yes. If it definitely
- doesn't, return pv_definite_no. If we can't tell, return pv_maybe.
-
- If the reference does touch the array, but doesn't fall exactly on
- an element boundary, or doesn't refer to the whole element, return
- pv_maybe. */
- static enum pv_boolean
- pv_is_array_ref (struct prologue_value *addr,
- CORE_ADDR size,
- struct prologue_value *array_addr,
- CORE_ADDR array_len,
- CORE_ADDR elt_size,
- int *i)
- {
- struct prologue_value offset;
-
- /* Note that, since ->k is a CORE_ADDR, and CORE_ADDR is unsigned,
- if addr is *before* the start of the array, then this isn't going
- to be negative... */
- pv_subtract (&offset, addr, array_addr);
-
- if (offset.kind == pv_constant)
- {
- /* This is a rather odd test. We want to know if the SIZE bytes
- at ADDR don't overlap the array at all, so you'd expect it to
- be an || expression: "if we're completely before || we're
- completely after". But with unsigned arithmetic, things are
- different: since it's a number circle, not a number line, the
- right values for offset.k are actually one contiguous range. */
- if (offset.k <= -size
- && offset.k >= array_len * elt_size)
- return pv_definite_no;
- else if (offset.k % elt_size != 0
- || size != elt_size)
- return pv_maybe;
- else
- {
- *i = offset.k / elt_size;
- return pv_definite_yes;
- }
- }
- else
- return pv_maybe;
- }
-
-
-
/* Decoding S/390 instructions. */
/* Named opcode values for the S/390 instructions we recognize. Some
--- 801,806 ----
*************** compute_x_addr (struct prologue_value *a
*** 1116,1125 ****
}
- /* The number of GPR and FPR spill slots in an S/390 stack frame. We
- track general-purpose registers r2 -- r15, and floating-point
- registers f0, f2, f4, and f6. */
- #define S390_NUM_SPILL_SLOTS (14 + 4)
#define S390_NUM_GPRS 16
#define S390_NUM_FPRS 16
--- 1056,1061 ----
*************** struct s390_prologue_data {
*** 1135,1218 ****
/* The floating-point registers. */
struct prologue_value fpr[S390_NUM_FPRS];
! /* The register spill stack slots in the caller's frame ---
! general-purpose registers r2 through r15, and floating-point
! registers. spill[i] is where gpr i+2 gets spilled;
! spill[(14, 15, 16, 17)] is where (f0, f2, f4, f6) get spilled. */
! struct prologue_value spill[S390_NUM_SPILL_SLOTS];
!
! /* The value of the back chain slot. This is only valid if the stack
! pointer is known to be less than its original value --- that is,
! if we have indeed allocated space on the stack. */
! struct prologue_value back_chain;
};
-
- /* If the SIZE bytes at ADDR are a stack slot we're actually tracking,
- return pv_definite_yes and set *STACK to point to the slot. If
- we're sure that they are not any of our stack slots, then return
- pv_definite_no. Otherwise, return pv_maybe.
-
- DATA describes our current state (registers and stack slots). */
- static enum pv_boolean
- s390_on_stack (struct prologue_value *addr,
- CORE_ADDR size,
- struct s390_prologue_data *data,
- struct prologue_value **stack)
- {
- struct prologue_value gpr_spill_addr;
- struct prologue_value fpr_spill_addr;
- struct prologue_value back_chain_addr;
- int i;
- enum pv_boolean b;
-
- /* Construct the addresses of the spill arrays and the back chain. */
- pv_set_to_register (&gpr_spill_addr, S390_SP_REGNUM, 2 * data->gpr_size);
- pv_set_to_register (&fpr_spill_addr, S390_SP_REGNUM, 16 * data->gpr_size);
- back_chain_addr = data->gpr[S390_SP_REGNUM - S390_R0_REGNUM];
-
- /* We have to check for GPR and FPR references using two separate
- calls to pv_is_array_ref, since the GPR and FPR spill slots are
- different sizes. (SPILL is an array, but the thing it tracks
- isn't really an array.) */
-
- /* Was it a reference to the GPR spill array? */
- b = pv_is_array_ref (addr, size, &gpr_spill_addr, 14, data->gpr_size, &i);
- if (b == pv_definite_yes)
- {
- *stack = &data->spill[i];
- return pv_definite_yes;
- }
- if (b == pv_maybe)
- return pv_maybe;
-
- /* Was it a reference to the FPR spill array? */
- b = pv_is_array_ref (addr, size, &fpr_spill_addr, 4, data->fpr_size, &i);
- if (b == pv_definite_yes)
- {
- *stack = &data->spill[14 + i];
- return pv_definite_yes;
- }
- if (b == pv_maybe)
- return pv_maybe;
-
- /* Was it a reference to the back chain?
- This isn't quite right. We ought to check whether we have
- actually allocated any new frame at all. */
- b = pv_is_array_ref (addr, size, &back_chain_addr, 1, data->gpr_size, &i);
- if (b == pv_definite_yes)
- {
- *stack = &data->back_chain;
- return pv_definite_yes;
- }
- if (b == pv_maybe)
- return pv_maybe;
-
- /* All the above queries returned definite 'no's. */
- return pv_definite_no;
- }
-
-
/* Do a SIZE-byte store of VALUE to ADDR. */
static void
s390_store (struct prologue_value *addr,
--- 1071,1088 ----
/* The floating-point registers. */
struct prologue_value fpr[S390_NUM_FPRS];
! /* The offset relative to the CFA where the incoming GPR N was saved
! by the function prologue. 0 if not saved or unknown. */
! int gpr_slot[S390_NUM_GPRS];
!
! /* Likewise for FPRs. */
! int fpr_slot[S390_NUM_FPRS];
!
! /* Nonzero if the backchain was saved. This is assumed to be the
! case when the incoming SP is saved at the current SP location. */
! int back_chain_saved_p;
};
/* Do a SIZE-byte store of VALUE to ADDR. */
static void
s390_store (struct prologue_value *addr,
*************** s390_store (struct prologue_value *addr,
*** 1220,1241 ****
struct prologue_value *value,
struct s390_prologue_data *data)
{
! struct prologue_value *stack;
! /* We can do it if it's definitely a reference to something on the stack. */
! if (s390_on_stack (addr, size, data, &stack) == pv_definite_yes)
! {
! *stack = *value;
! return;
}
- /* Note: If s390_on_stack returns pv_maybe, you might think we should
- forget our cached values, as any of those might have been hit.
! However, we make the assumption that --since the fields we track
! are save areas private to compiler, and never directly exposed to
! the user-- every access to our data is explicit. Hence, every
! memory access we cannot follow can't hit our data. */
}
/* Do a SIZE-byte load from ADDR into VALUE. */
--- 1090,1150 ----
struct prologue_value *value,
struct s390_prologue_data *data)
{
! struct prologue_value cfa, offset;
! int i;
! /* Check whether we are storing the backchain. */
! pv_subtract (&offset, &data->gpr[S390_SP_REGNUM - S390_R0_REGNUM], addr);
!
! if (offset.kind == pv_constant && offset.k == 0)
! if (size == data->gpr_size
! && pv_is_register (value, S390_SP_REGNUM, 0))
! {
! data->back_chain_saved_p = 1;
! return;
! }
!
!
! /* Check whether we are storing a register into the stack. */
! pv_set_to_register (&cfa, S390_SP_REGNUM, 16 * data->gpr_size + 32);
! pv_subtract (&offset, &cfa, addr);
!
! if (offset.kind == pv_constant
! && offset.k < INT_MAX && offset.k > 0
! && offset.k % data->gpr_size == 0)
! {
! /* If we are storing the original value of a register, we want to
! record the CFA offset. If the same register is stored multiple
! times, the stack slot with the highest address counts. */
!
! for (i = 0; i < S390_NUM_GPRS; i++)
! if (size == data->gpr_size
! && pv_is_register (value, S390_R0_REGNUM + i, 0))
! if (data->gpr_slot[i] == 0
! || data->gpr_slot[i] > offset.k)
! {
! data->gpr_slot[i] = offset.k;
! return;
! }
!
! for (i = 0; i < S390_NUM_FPRS; i++)
! if (size == data->fpr_size
! && pv_is_register (value, S390_F0_REGNUM + i, 0))
! if (data->fpr_slot[i] == 0
! || data->fpr_slot[i] > offset.k)
! {
! data->fpr_slot[i] = offset.k;
! return;
! }
}
! /* Note: If this is some store we cannot identify, you might think we
! should forget our cached values, as any of those might have been hit.
!
! However, we make the assumption that the register save areas are only
! ever stored to once in any given function, and we do recognize these
! stores. Thus every store we cannot recognize does not hit our data. */
}
/* Do a SIZE-byte load from ADDR into VALUE. */
*************** s390_load (struct prologue_value *addr,
*** 1245,1251 ****
struct prologue_value *value,
struct s390_prologue_data *data)
{
! struct prologue_value *stack;
/* If it's a load from an in-line constant pool, then we can
simulate that, under the assumption that the code isn't
--- 1154,1161 ----
struct prologue_value *value,
struct s390_prologue_data *data)
{
! struct prologue_value cfa, offset;
! int i;
/* If it's a load from an in-line constant pool, then we can
simulate that, under the assumption that the code isn't
*************** s390_load (struct prologue_value *addr,
*** 1265,1276 ****
}
}
! /* If it's definitely a reference to something on the stack,
! we can do that. */
! if (s390_on_stack (addr, size, data, &stack) == pv_definite_yes)
{
! *value = *stack;
! return;
}
/* Otherwise, we don't know the value. */
--- 1175,1200 ----
}
}
! /* Check whether we are accessing one of our save slots. */
! pv_set_to_register (&cfa, S390_SP_REGNUM, 16 * data->gpr_size + 32);
! pv_subtract (&offset, &cfa, addr);
!
! if (offset.kind == pv_constant
! && offset.k < INT_MAX && offset.k > 0)
{
! for (i = 0; i < S390_NUM_GPRS; i++)
! if (offset.k == data->gpr_slot[i])
! {
! pv_set_to_register (value, S390_R0_REGNUM + i, 0);
! return;
! }
!
! for (i = 0; i < S390_NUM_FPRS; i++)
! if (offset.k == data->fpr_slot[i])
! {
! pv_set_to_register (value, S390_F0_REGNUM + i, 0);
! return;
! }
}
/* Otherwise, we don't know the value. */
*************** s390_analyze_prologue (struct gdbarch *g
*** 1320,1329 ****
for (i = 0; i < S390_NUM_FPRS; i++)
pv_set_to_register (&data->fpr[i], S390_F0_REGNUM + i, 0);
! for (i = 0; i < S390_NUM_SPILL_SLOTS; i++)
! pv_set_to_unknown (&data->spill[i]);
! pv_set_to_unknown (&data->back_chain);
}
/* Start interpreting instructions, until we hit the frame's
--- 1244,1256 ----
for (i = 0; i < S390_NUM_FPRS; i++)
pv_set_to_register (&data->fpr[i], S390_F0_REGNUM + i, 0);
! for (i = 0; i < S390_NUM_GPRS; i++)
! data->gpr_slot[i] = 0;
!
! for (i = 0; i < S390_NUM_FPRS; i++)
! data->fpr_slot[i] = 0;
! data->back_chain_saved_p = 0;
}
/* Start interpreting instructions, until we hit the frame's
*************** s390_analyze_prologue (struct gdbarch *g
*** 1337,1345 ****
unsigned int b2, r1, r2, x2, r3;
int i2, d2;
! /* The values of SP, FP, and back chain before this instruction,
for detecting instructions that change them. */
! struct prologue_value pre_insn_sp, pre_insn_fp, pre_insn_back_chain;
/* If we got an error trying to read the instruction, report it. */
if (insn_len < 0)
--- 1264,1274 ----
unsigned int b2, r1, r2, x2, r3;
int i2, d2;
! /* The values of SP and FP before this instruction,
for detecting instructions that change them. */
! struct prologue_value pre_insn_sp, pre_insn_fp;
! /* Likewise for the flag whether the back chain was saved. */
! int pre_insn_back_chain_saved_p;
/* If we got an error trying to read the instruction, report it. */
if (insn_len < 0)
*************** s390_analyze_prologue (struct gdbarch *g
*** 1352,1358 ****
pre_insn_sp = data->gpr[S390_SP_REGNUM - S390_R0_REGNUM];
pre_insn_fp = data->gpr[S390_FRAME_REGNUM - S390_R0_REGNUM];
! pre_insn_back_chain = data->back_chain;
/* LHI r1, i2 --- load halfword immediate */
if (word_size == 4
--- 1281,1287 ----
pre_insn_sp = data->gpr[S390_SP_REGNUM - S390_R0_REGNUM];
pre_insn_fp = data->gpr[S390_FRAME_REGNUM - S390_R0_REGNUM];
! pre_insn_back_chain_saved_p = data->back_chain_saved_p;
/* LHI r1, i2 --- load halfword immediate */
if (word_size == 4
*************** s390_analyze_prologue (struct gdbarch *g
*** 1674,1680 ****
&& ! pv_is_register (sp, S390_SP_REGNUM, 0))
|| (! pv_is_identical (&pre_insn_fp, fp)
&& ! pv_is_register (fp, S390_FRAME_REGNUM, 0))
! || ! pv_is_identical (&pre_insn_back_chain, &data->back_chain))
result = next_pc;
}
}
--- 1603,1609 ----
&& ! pv_is_register (sp, S390_SP_REGNUM, 0))
|| (! pv_is_identical (&pre_insn_fp, fp)
&& ! pv_is_register (fp, S390_FRAME_REGNUM, 0))
! || pre_insn_back_chain_saved_p != data->back_chain_saved_p)
result = next_pc;
}
}
*************** s390_prologue_frame_unwind_cache (struct
*** 1761,1772 ****
struct s390_unwind_cache *info)
{
struct gdbarch *gdbarch = get_frame_arch (next_frame);
int word_size = gdbarch_ptr_bit (gdbarch) / 8;
struct s390_prologue_data data;
struct prologue_value *fp = &data.gpr[S390_FRAME_REGNUM - S390_R0_REGNUM];
struct prologue_value *sp = &data.gpr[S390_SP_REGNUM - S390_R0_REGNUM];
! int slot_num;
! CORE_ADDR slot_addr;
CORE_ADDR func;
CORE_ADDR result;
ULONGEST reg;
--- 1690,1702 ----
struct s390_unwind_cache *info)
{
struct gdbarch *gdbarch = get_frame_arch (next_frame);
+ struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
int word_size = gdbarch_ptr_bit (gdbarch) / 8;
struct s390_prologue_data data;
struct prologue_value *fp = &data.gpr[S390_FRAME_REGNUM - S390_R0_REGNUM];
struct prologue_value *sp = &data.gpr[S390_SP_REGNUM - S390_R0_REGNUM];
! int i;
! CORE_ADDR cfa;
CORE_ADDR func;
CORE_ADDR result;
ULONGEST reg;
*************** s390_prologue_frame_unwind_cache (struct
*** 1869,1901 ****
add back the frame size to arrive that the previous frame's
stack pointer value. */
prev_sp = frame_unwind_register_unsigned (next_frame, frame_pointer) + size;
! /* Scan the spill array; if a spill slot says it holds the
! original value of some register, then record that slot's
! address as the place that register was saved. */
!
! /* Slots for %r2 .. %r15. */
! for (slot_num = 0, slot_addr = prev_sp + 2 * data.gpr_size;
! slot_num < 14;
! slot_num++, slot_addr += data.gpr_size)
! {
! struct prologue_value *slot = &data.spill[slot_num];
!
! if (slot->kind == pv_register
! && slot->k == 0)
! info->saved_regs[slot->reg].addr = slot_addr;
! }
! /* Slots for %f0 .. %f6. */
! for (slot_num = 14, slot_addr = prev_sp + 16 * data.gpr_size;
! slot_num < S390_NUM_SPILL_SLOTS;
! slot_num++, slot_addr += data.fpr_size)
{
! struct prologue_value *slot = &data.spill[slot_num];
! if (slot->kind == pv_register
! && slot->k == 0)
! info->saved_regs[slot->reg].addr = slot_addr;
}
/* Function return will set PC to %r14. */
--- 1799,1829 ----
add back the frame size to arrive that the previous frame's
stack pointer value. */
prev_sp = frame_unwind_register_unsigned (next_frame, frame_pointer) + size;
+ cfa = prev_sp + 16*word_size + 32;
! /* Record the addresses of all register spill slots the prologue parser
! has recognized. Consider only registers defined as call-saved by the
! ABI; for call-clobbered registers the parser may have recognized
! spurious stores. */
!
! for (i = 6; i <= 15; i++)
! if (data.gpr_slot[i] != 0)
! info->saved_regs[S390_R0_REGNUM + i].addr = cfa - data.gpr_slot[i];
! switch (tdep->abi)
{
! case ABI_LINUX_S390:
! if (data.fpr_slot[4] != 0)
! info->saved_regs[S390_F4_REGNUM].addr = cfa - data.fpr_slot[4];
! if (data.fpr_slot[6] != 0)
! info->saved_regs[S390_F6_REGNUM].addr = cfa - data.fpr_slot[6];
! break;
! case ABI_LINUX_ZSERIES:
! for (i = 8; i <= 15; i++)
! if (data.fpr_slot[i] != 0)
! info->saved_regs[S390_F0_REGNUM + i].addr = cfa - data.fpr_slot[i];
! break;
}
/* Function return will set PC to %r14. */
*************** s390_push_dummy_call (struct gdbarch *gd
*** 2708,2718 ****
and the back chain pointer. */
sp -= 16*word_size + 32;
- /* Write the back chain pointer into the first word of the stack
- frame. This is needed to unwind across a dummy frame. */
- regcache_cooked_read_unsigned (regcache, S390_SP_REGNUM, &orig_sp);
- write_memory_unsigned_integer (sp, word_size, orig_sp);
-
/* Store return address. */
regcache_cooked_write_unsigned (regcache, S390_RETADDR_REGNUM, bp_addr);
--- 2636,2641 ----
*************** s390_push_dummy_call (struct gdbarch *gd
*** 2720,2728 ****
regcache_cooked_write_unsigned (regcache, S390_SP_REGNUM, sp);
/* We need to return the 'stack part' of the frame ID,
! which is actually the top of the register save area
! allocated on the original stack. */
! return orig_sp + 16*word_size + 32;
}
/* Assuming NEXT_FRAME->prev is a dummy, return the frame ID of that
--- 2643,2650 ----
regcache_cooked_write_unsigned (regcache, S390_SP_REGNUM, sp);
/* We need to return the 'stack part' of the frame ID,
! which is actually the top of the register save area. */
! return sp + 16*word_size + 32;
}
/* Assuming NEXT_FRAME->prev is a dummy, return the frame ID of that
*************** static struct frame_id
*** 2733,2742 ****
s390_unwind_dummy_id (struct gdbarch *gdbarch, struct frame_info *next_frame)
{
int word_size = gdbarch_ptr_bit (gdbarch) / 8;
! CORE_ADDR this_sp = s390_unwind_sp (gdbarch, next_frame);
! CORE_ADDR prev_sp = read_memory_unsigned_integer (this_sp, word_size);
! return frame_id_build (prev_sp + 16*word_size + 32,
frame_pc_unwind (next_frame));
}
--- 2655,2663 ----
s390_unwind_dummy_id (struct gdbarch *gdbarch, struct frame_info *next_frame)
{
int word_size = gdbarch_ptr_bit (gdbarch) / 8;
! CORE_ADDR sp = s390_unwind_sp (gdbarch, next_frame);
! return frame_id_build (sp + 16*word_size + 32,
frame_pc_unwind (next_frame));
}
--
Dr. Ulrich Weigand
weigand@informatik.uni-erlangen.de