[RFA/ARM] Framificate the ARM port [3/3]
Andrew Cagney
ac131313@redhat.com
Tue Jul 1 15:32:00 GMT 2003
So much code gets deleted, Ya!
Here are some ``GDB speak'' comments - help clarify use of `prev',
`this', and `next'. Some of them (e.g., s/our/THIS/ are probably not
GNU PC).
> ===================================================================
> --- gdb.orig/arm-tdep.c 2003-06-30 18:28:56.000000000 -0400
> +++ gdb/arm-tdep.c 2003-06-30 18:28:57.000000000 -0400
> @@ -34,6 +34,9 @@
> #include "value.h"
> #include "arch-utils.h"
> #include "osabi.h"
> +#include "frame-unwind.h"
> +#include "frame-base.h"
> +#include "trad-frame.h"
>
> #include "arm-tdep.h"
> #include "gdb/sim-arm.h"
> @@ -155,21 +158,31 @@ static void convert_from_extended (const
> static void convert_to_extended (const struct floatformat *, void *,
> const void *);
>
> -/* Define other aspects of the stack frame. We keep the offsets of
> - all saved registers, 'cause we need 'em a lot! We also keep the
> - current size of the stack frame, and the offset of the frame
> - pointer from the stack pointer (for frameless functions, and when
> - we're still in the prologue of a function with a frame). */
> -
> -#define arm_get_cache(fi) ((struct arm_prologue_cache *) get_frame_extra_info (fi))
> -
> struct arm_prologue_cache
> {
> - CORE_ADDR unwound_sp, unwound_pc;
> + /* The stack pointer at the time this frame was created; i.e. our caller's
> + stack pointer when we were called. We use this to identify the frame. */
> + CORE_ADDR unwound_sp;
I'd call it ``prev_sp'', and ``It is used used to identify THIS frame''.
> + /* The current PC in this frame; i.e. our callee's resume address, if we are
> + not the innermost frame. This is not constant throughout the lifetime
> + of this frame, so we don't use it to identify the frame, just to find
> + the function. */
> + CORE_ADDR unwound_pc;
Hmm, ``unwound_sp'' belongs to the PREV frame but ``unwound_pc'' belongs
to THIS frame :-(
Suggest instead using a function local ``this_pc'' variable and not even
cacheing the THIS frame's PC value. frame_pc_unwind(next_frame) is
effecient (it already caches the PC) and in the past bad kama has
resulted from redundant caching of pc values (cf deprecated update frame
pc hack).
> + /* The frame base for this frame is just unwound_sp + frame offset - frame
> + size. FRAMESIZE is the size of the stack frame, and FRAMEOFFSET
> + if the initial offset from the stack pointer (our stack pointer, not
> + UNWOUND_SP) to the frame base. */
> +
> int framesize;
> int frameoffset;
Um, which SP? THIS or PREV?
> + /* The register used to hold the frame pointer for this frame. */
> int framereg;
> - CORE_ADDR saved_regs[1];
> +
> + /* Saved register offsets. */
> + struct trad_frame_saved_reg *saved_regs;
> };
>
> /* Addresses for calling Thumb functions have the bit 0 set.
> + /* Check that we're not going round in circles with the same frame
> + ID (but avoid applying the test to sentinel frames which do go
> + round in circles). */
This shouldn't be needed. Hmm, ah, I never enabled (from frame.c):
/* Check that this and the next frame are different. If they are
not, there is most likely a stack cycle. As with the inner-than
test, avoid the inner-most and sentinel frames. */
/* FIXME: cagney/2003-03-17: Can't yet enable this this check. The
frame_id_eq() method doesn't yet use function addresses when
comparing frame IDs. */
if (0
&& this_frame->level > 0
&& frame_id_eq (get_frame_id (this_frame),
get_frame_id (this_frame->next)))
error ("This frame identical to next frame (corrupt stack?)");
> + if (frame_relative_level (next_frame) >= 0
> + && get_frame_type (next_frame) == NORMAL_FRAME
> + && frame_id_eq (get_frame_id (next_frame), id))
> + return;
> + if (*this_cache == NULL)
> + *this_cache = arm_make_prologue_cache (next_frame);
> + cache = *this_cache;
> +
> + /* If we are asked to unwind the PC, then we need to return the LR instead.
> + The saved value of PC points into this frame's prologue, not the
> + next frame's resume location. */
> + if (prev_regnum == ARM_PC_REGNUM)
> + prev_regnum = ARM_LR_REGNUM;
See above, eliminating unwound_pc would get rid of the confusion.
> + /* If someone asks for the stack pointer, then they want unwound_sp,
> + which was our stack pointer at the time of the call. SP is not
> + generally saved to the stack. */
(I can't stand the GNU style of using first and second person in code,
it's way too ambigious :-() I'd just state:
``The PREV frame's stack pointer was previously computed and saved in
cache->prev_sp. The SP is not generally saved to the stack.''
> + if (prev_regnum == ARM_SP_REGNUM)
> + {
> + *lvalp = not_lval;
> + if (valuep)
> + store_unsigned_integer (valuep, 4, cache->unwound_sp);
> + return;
> + }
>
> +static const struct frame_unwind *
> +arm_sigtramp_unwind_p (CORE_ADDR pc)
> +{
> + /* Note: If an ARM PC_IN_SIGTRAMP method ever needs to compare
> + against the name of the function, the code below will have to be
> + changed to first fetch the name of the function and then pass
> + this name to PC_IN_SIGTRAMP. */
>
> - memcpy (get_frame_extra_info (fi), cache, (sizeof (struct arm_prologue_cache)
> - + ((NUM_REGS + NUM_PSEUDO_REGS - 1)
> - * sizeof (CORE_ADDR))));
> - memcpy (get_frame_saved_regs (fi), cache->saved_regs,
> - (NUM_REGS + NUM_PSEUDO_REGS - 1) * sizeof (CORE_ADDR));
> -}
> + if (SIGCONTEXT_REGISTER_ADDRESS_P () && PC_IN_SIGTRAMP (pc, (char *) 0))
> + return &arm_sigtramp_unwind;
Can PC_IN_SIGTRAMP be eliminated here?
> +/* Assuming NEXT_FRAME->prev is a dummy, return the frame ID of that
> + dummy frame. The frame ID's base needs to match the TOS value
> + saved by save_dummy_frame_tos(), and the PC match the dummy frame's
> + breakpoint. */
... tos() and returned by arm_push_dummy_call.
> +/* Given THIS_FRAME, find the frame's resume PC (which will be part of the
> + frame ID for THIS_FRAME's caller's frame). */
Given THIS_FRAME, find the PREV frame's resume PC (which will be used to
find the PREV frame's function and that in turn used to construct the
PREV frame's ID).
> static CORE_ADDR
> -arm_read_fp (void)
> +arm_unwind_pc (struct gdbarch *gdbarch, struct frame_info *this_frame)
> {
> - if (read_register (ARM_PS_REGNUM) & 0x20) /* Bit 5 is Thumb state bit */
> - return read_register (THUMB_FP_REGNUM); /* R7 if Thumb */
> - else
> - return read_register (ARM_FP_REGNUM); /* R11 if ARM */
> + CORE_ADDR pc;
> + pc = frame_unwind_register_unsigned (this_frame, ARM_PC_REGNUM);
> + return IS_THUMB_ADDR (pc) ? UNMAKE_THUMB_ADDR (pc) : pc;
> }
Nice,
Andrew
More information about the Gdb-patches
mailing list