[PATCH RFA] Fix some ARM call dummy problems
Fernando Nasser
fnasser@redhat.com
Mon Dec 17 11:18:00 GMT 2001
Yes, this patch solves the problem.
Please check it in.
Thanks a lot for the fix Kevin.
Fernando
Kevin Buettner wrote:
>
> The patch below fixes several problems with call dummies on the ARM.
> In the testing that I've been doing, it fixes the following failures:
>
> FAIL: gdb.base/break.exp: backtrace while in called function
> FAIL: gdb.base/callfuncs.exp: continue after stop in call dummy preserves register contents
> FAIL: gdb.base/callfuncs.exp: finish after stop in call dummy preserves register contents
> FAIL: gdb.base/callfuncs.exp: back at main after return from call dummy breakpoint
>
> It also adds the following PASS. (This test was suppressed when the "back at
> main..." test failed.)
>
> PASS: gdb.base/callfuncs.exp: return after stop in call dummy preserves register contents
>
> The reason for the first and fourth failures listed above were because
> GDB for ARM was unable to backtrace through a call dummy. The second
> and third failures were due to the fact that not all of the general
> purpose registers were being saved. Thus, when the inferior function
> call returned, the values of some of the registers would be changed.
>
> It was tempting to replace the existing ARM call dummy machinery with
> generic call dummies, but I resisted doing this for these reasons:
>
> 1) The changes needed would've been more extensive. Since it's
> taking a *very* long time to get feedback on ARM patches these
> days, the chances are very good that I will have forgotten
> the details regarding such a patch if there are issues when
> the patch is finally reviewed.
> 2) It is unclear to me how Thumb support should be arranged with
> generic call dummies. By instead fixing the current mechanism,
> I felt there was less danger of breaking the existing Thumb
> support.
> 3) There is already some #if 0'd code for supporting generic call
> dummies. It's not clear to me why this code is not enabled,
> but one possibility is that there were problems with doing it.
> Or, it might simply be uncompleted work. Regardless, I decided
> it was best to allow the ARM maintainer to complete this work.
>
> I should also note that in the course of simplifying arm_pop_frame(),
> I ended up deleting some code which is just plain wrong. Code that
> looks like this:
>
> sp -= sizeof (CORE_ADDR);
>
> is almost never right. The CORE_ADDR size in question is the size on
> the host, not the target. Prior to realizing that the special case
> containing this code could be deleted, I was rewriting these statements
> along the following lines:
>
> sp -= REGISTER_RAW_SIZE (regnum);
>
> But, after a time, I realized that the special purpose code for
> popping a call dummy was superfluous since the code for regular frames
> will do the job quite nicely so long as the frame info has been
> initialized correctly. It just so happened that all of the erroneous
> ``sizeof (CORE_ADDR)'' was contained within this special case.
>
> Okay to commit?
>
> * arm-tdep.c (arm_init_extra_frame_info): Add special case for
> call dummies.
> (arm_frame_saved_pc): Likewise.
> (arm_push_dummy_frame): Make sure all of the GPRs are saved.
> (arm_pop_frame): Eliminate special case for call dummies. It
> is no longer needed now that the frame info is being properly
> initialized.
>
> Index: arm-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/arm-tdep.c,v
> retrieving revision 1.19
> diff -u -p -r1.19 arm-tdep.c
> --- arm-tdep.c 2001/12/05 08:36:01 1.19
> +++ arm-tdep.c 2001/12/13 17:39:26
> @@ -1009,6 +1009,7 @@ void
> arm_init_extra_frame_info (int fromleaf, struct frame_info *fi)
> {
> int reg;
> + CORE_ADDR sp;
>
> if (fi->next)
> fi->pc = FRAME_SAVED_PC (fi->next);
> @@ -1028,6 +1029,13 @@ arm_init_extra_frame_info (int fromleaf,
> else
> #endif
>
> + /* Compute stack pointer for this frame. We use this value for both the
> + sigtramp and call dummy cases. */
> + if (!fi->next)
> + sp = read_sp();
> + else
> + sp = fi->next->frame - fi->next->frameoffset + fi->next->framesize;
> +
> /* Determine whether or not we're in a sigtramp frame.
> Unfortunately, it isn't sufficient to test
> fi->signal_handler_caller because this value is sometimes set
> @@ -1038,28 +1046,45 @@ arm_init_extra_frame_info (int fromleaf,
> Note: If an ARM 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 IN_SIGTRAMP. */
> + to IN_SIGTRAMP. */
>
> if (SIGCONTEXT_REGISTER_ADDRESS_P ()
> && (fi->signal_handler_caller || IN_SIGTRAMP (fi->pc, 0)))
> {
> - CORE_ADDR sp;
> -
> - if (!fi->next)
> - sp = read_sp();
> - else
> - sp = fi->next->frame - fi->next->frameoffset + fi->next->framesize;
> -
> for (reg = 0; reg < NUM_REGS; reg++)
> fi->fsr.regs[reg] = SIGCONTEXT_REGISTER_ADDRESS (sp, fi->pc, reg);
>
> /* FIXME: What about thumb mode? */
> fi->framereg = SP_REGNUM;
> - fi->frame = read_memory_integer (fi->fsr.regs[fi->framereg], 4);
> + fi->frame = read_memory_integer (fi->fsr.regs[fi->framereg],
> + REGISTER_RAW_SIZE (fi->framereg));
> fi->framesize = 0;
> fi->frameoffset = 0;
>
> }
> + else if (PC_IN_CALL_DUMMY (fi->pc, sp, fi->frame))
> + {
> + CORE_ADDR rp;
> + CORE_ADDR callers_sp;
> +
> + /* Set rp point at the high end of the saved registers. */
> + rp = fi->frame - REGISTER_SIZE;
> +
> + /* Fill in addresses of saved registers. */
> + fi->fsr.regs[PS_REGNUM] = rp;
> + rp -= REGISTER_RAW_SIZE (PS_REGNUM);
> + for (reg = PC_REGNUM; reg >= 0; reg--)
> + {
> + fi->fsr.regs[reg] = rp;
> + rp -= REGISTER_RAW_SIZE (reg);
> + }
> +
> + callers_sp = read_memory_integer (fi->fsr.regs[SP_REGNUM],
> + REGISTER_RAW_SIZE (SP_REGNUM));
> + fi->framereg = FP_REGNUM;
> + fi->framesize = callers_sp - sp;
> + fi->frameoffset = fi->frame - sp;
> + }
> else
> {
> arm_scan_prologue (fi);
> @@ -1105,7 +1130,12 @@ arm_frame_saved_pc (struct frame_info *f
> return generic_read_register_dummy (fi->pc, fi->frame, PC_REGNUM);
> else
> #endif
> + if (PC_IN_CALL_DUMMY (fi->pc, fi->frame - fi->frameoffset, fi->frame))
> {
> + return read_memory_integer (fi->fsr.regs[PC_REGNUM], REGISTER_RAW_SIZE (PC_REGNUM));
> + }
> + else
> + {
> CORE_ADDR pc = arm_find_callers_reg (fi, LR_REGNUM);
> return IS_THUMB_ADDR (pc) ? UNMAKE_THUMB_ADDR (pc) : pc;
> }
> @@ -1151,13 +1181,15 @@ arm_push_dummy_frame (void)
> instruction stores the PC, it stores the address of the stm
> instruction itself plus 12. */
> fp = sp = push_word (sp, prologue_start + 12);
> - sp = push_word (sp, read_register (PC_REGNUM)); /* FIXME: was PS_REGNUM */
> - sp = push_word (sp, old_sp);
> - sp = push_word (sp, read_register (FP_REGNUM));
>
> - for (regnum = 10; regnum >= 0; regnum--)
> + /* Push the processor status. */
> + sp = push_word (sp, read_register (PS_REGNUM));
> +
> + /* Push all 16 registers starting with r15. */
> + for (regnum = PC_REGNUM; regnum >= 0; regnum--)
> sp = push_word (sp, read_register (regnum));
>
> + /* Update fp (for both Thumb and ARM) and sp. */
> write_register (FP_REGNUM, fp);
> write_register (THUMB_FP_REGNUM, fp);
> write_register (SP_REGNUM, sp);
> @@ -1372,45 +1404,25 @@ arm_push_arguments (int nargs, struct va
> return sp;
> }
>
> +/* Pop the current frame. So long as the frame info has been initialized
> + properly (see arm_init_extra_frame_info), this code works for dummy frames
> + as well as regular frames. I.e, there's no need to have a special case
> + for dummy frames. */
> void
> arm_pop_frame (void)
> {
> int regnum;
> struct frame_info *frame = get_current_frame ();
> -
> - if (!PC_IN_CALL_DUMMY(frame->pc, frame->frame, read_fp()))
> - {
> - CORE_ADDR old_SP;
> + CORE_ADDR old_SP = frame->frame - frame->frameoffset + frame->framesize;
>
> - old_SP = read_register (frame->framereg);
> - for (regnum = 0; regnum < NUM_REGS; regnum++)
> - if (frame->fsr.regs[regnum] != 0)
> - write_register (regnum,
> - read_memory_integer (frame->fsr.regs[regnum], 4));
> + for (regnum = 0; regnum < NUM_REGS; regnum++)
> + if (frame->fsr.regs[regnum] != 0)
> + write_register (regnum,
> + read_memory_integer (frame->fsr.regs[regnum],
> + REGISTER_RAW_SIZE (regnum)));
>
> - write_register (PC_REGNUM, FRAME_SAVED_PC (frame));
> - write_register (SP_REGNUM, old_SP);
> - }
> - else
> - {
> - CORE_ADDR sp;
> -
> - sp = read_register (FP_REGNUM);
> - sp -= sizeof(CORE_ADDR); /* we don't care about this first word */
> -
> - write_register (PC_REGNUM, read_memory_integer (sp, 4));
> - sp -= sizeof(CORE_ADDR);
> - write_register (SP_REGNUM, read_memory_integer (sp, 4));
> - sp -= sizeof(CORE_ADDR);
> - write_register (FP_REGNUM, read_memory_integer (sp, 4));
> - sp -= sizeof(CORE_ADDR);
> -
> - for (regnum = 10; regnum >= 0; regnum--)
> - {
> - write_register (regnum, read_memory_integer (sp, 4));
> - sp -= sizeof(CORE_ADDR);
> - }
> - }
> + write_register (PC_REGNUM, FRAME_SAVED_PC (frame));
> + write_register (SP_REGNUM, old_SP);
>
> flush_cached_frames ();
> }
--
Fernando Nasser
Red Hat - Toronto E-Mail: fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario M4P 2C9
More information about the Gdb-patches
mailing list