This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: [PATCH RFA] arm-tdep.c: prologue scanner adjustments
- From: Fernando Nasser <fnasser at redhat dot com>
- To: Kevin Buettner <kevinb at redhat dot com>
- Cc: gdb-patches at sources dot redhat dot com
- Date: Wed, 19 Dec 2001 13:21:29 -0500
- Subject: Re: [PATCH RFA] arm-tdep.c: prologue scanner adjustments
- Organization: Red Hat , Inc. - Toronto
- References: <1011127063801.ZM10619@ocotillo.lan>
This patch actually requires
http://sources.redhat.com/ml/gdb-patches/2001-12/msg00349.html
After that one is applied it goes nice (otherwise it uncovers
the problem fixed by the other patch in a nasty way that prevents
the testsuite to run).
It is approved to be applied after that one or as a combined patch
(I tested Kevin's combined patch already).
Thanks for the fixes Kevin. Once more.
Fernando
Kevin Buettner wrote:
>
> I've come across a case where the current ARM prologue scanner is
> deficient. Here's the prologue in question...
>
> 0x400ef260 <__sigsuspend>: stmdb sp!, {r4, r5, r10, lr}
> 0x400ef264 <__sigsuspend+4>: mov r4, r0
> 0x400ef268 <__sigsuspend+8>:
> ldr r10, [pc, #144] ; 0x400ef300 <__sigsuspend+160>
> 0x400ef26c <__sigsuspend+12>:
> ldr r3, [pc, #144] ; 0x400ef304 <__sigsuspend+164>
> 0x400ef270 <__sigsuspend+16>: add r10, pc, r10
> 0x400ef274 <__sigsuspend+20>: ldr r5, [r10, r3]
> 0x400ef278 <__sigsuspend+24>: ldr r2, [r5]
> 0x400ef27c <__sigsuspend+28>: cmp r2, #0 ; 0x0
>
> It turns out that the ARM prologue scanner has two problems with
> the above prologue.
>
> First, it does not start with "mov ip, sp". I've read the current
> ABI documents and, while this may have been mandatory at one time,
> I can't find any language which requires that this be the first
> instruction in the prologue. I've revised the prologue scanner to
> make it optional.
>
> Second, it appears to me that the "no frame pointer" case has never
> been tested. The code which sets the frame offset will incorrectly
> compute negative value which leads to some pretty weird behavior when
> attempting to do a backtrace.
>
> I'm attaching two versions of the patch. The first will be of interest
> to those of you examining this patch either for correctness or to just
> see the salient changes. It uses the -w switch which causes the whitespace
> caused by the (right) shifting of many lines to be ignored. The second
> version of the patch should be used by anyone wanting to actually apply
> the patch to the sources.
>
> In the testing I've done, this patch fixes a number of FAILs in
> linux-dp.exp. I haven't seen any regressions.
>
> Okay to commit?
>
> * arm-tdep.c (arm_scan_prologue): Don't require "mov ip, sp"
> to be the first instruction in the prologue. Also, revise
> the way the frame offset is computed for frameless functions.
>
> Index: arm-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/arm-tdep.c,v
> retrieving revision 1.17
> diff -u -w -p -r1.17 arm-tdep.c
> --- arm-tdep.c 2001/11/14 08:18:32 1.17
> +++ arm-tdep.c 2001/11/27 06:18:25
> @@ -797,15 +797,19 @@ arm_scan_prologue (struct frame_info *fi
> traceback.
>
> In the APCS, the prologue should start with "mov ip, sp" so
> - if we don't see this as the first insn, we will stop. */
> + if we don't see this as the first insn, we will stop. [Note:
> + This doesn't seem to be true any longer, so it's now an optional
> + part of the prologue. - Kevin Buettner, 2001-11-20] */
>
> sp_offset = fp_offset = 0;
>
> if (read_memory_unsigned_integer (prologue_start, 4)
> == 0xe1a0c00d) /* mov ip, sp */
> - {
> - for (current_pc = prologue_start + 4; current_pc < prologue_end;
> - current_pc += 4)
> + current_pc = prologue_start + 4;
> + else
> + current_pc = prologue_start;
> +
> + for (; current_pc < prologue_end; current_pc += 4)
> {
> unsigned int insn = read_memory_unsigned_integer (current_pc, 4);
>
> @@ -882,13 +886,15 @@ arm_scan_prologue (struct frame_info *fi
> so we just skip what we don't recognize. */
> continue;
> }
> - }
>
> /* The frame size is just the negative of the offset (from the original SP)
> of the last thing thing we pushed on the stack. The frame offset is
> [new FP] - [new SP]. */
> fi->framesize = -sp_offset;
> + if (fi->framereg == FP_REGNUM)
> fi->frameoffset = fp_offset - sp_offset;
> + else
> + fi->frameoffset = 0;
>
> save_prologue_cache (fi);
> }
>
> ---- Second version which shows changes due to shifted lines... ----
>
> Index: arm-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/arm-tdep.c,v
> retrieving revision 1.17
> diff -u -p -r1.17 arm-tdep.c
> --- arm-tdep.c 2001/11/14 08:18:32 1.17
> +++ arm-tdep.c 2001/11/27 06:16:07
> @@ -797,98 +797,104 @@ arm_scan_prologue (struct frame_info *fi
> traceback.
>
> In the APCS, the prologue should start with "mov ip, sp" so
> - if we don't see this as the first insn, we will stop. */
> + if we don't see this as the first insn, we will stop. [Note:
> + This doesn't seem to be true any longer, so it's now an optional
> + part of the prologue. - Kevin Buettner, 2001-11-20] */
>
> sp_offset = fp_offset = 0;
>
> if (read_memory_unsigned_integer (prologue_start, 4)
> == 0xe1a0c00d) /* mov ip, sp */
> + current_pc = prologue_start + 4;
> + else
> + current_pc = prologue_start;
> +
> + for (; current_pc < prologue_end; current_pc += 4)
> {
> - for (current_pc = prologue_start + 4; current_pc < prologue_end;
> - current_pc += 4)
> + unsigned int insn = read_memory_unsigned_integer (current_pc, 4);
> +
> + if ((insn & 0xffff0000) == 0xe92d0000)
> + /* stmfd sp!, {..., fp, ip, lr, pc}
> + or
> + stmfd sp!, {a1, a2, a3, a4} */
> {
> - unsigned int insn = read_memory_unsigned_integer (current_pc, 4);
> + int mask = insn & 0xffff;
>
> - if ((insn & 0xffff0000) == 0xe92d0000)
> - /* stmfd sp!, {..., fp, ip, lr, pc}
> - or
> - stmfd sp!, {a1, a2, a3, a4} */
> - {
> - int mask = insn & 0xffff;
> + /* Calculate offsets of saved registers. */
> + for (regno = PC_REGNUM; regno >= 0; regno--)
> + if (mask & (1 << regno))
> + {
> + sp_offset -= 4;
> + fi->fsr.regs[regno] = sp_offset;
> + }
> + }
> + else if ((insn & 0xfffff000) == 0xe24cb000) /* sub fp, ip #n */
> + {
> + unsigned imm = insn & 0xff; /* immediate value */
> + unsigned rot = (insn & 0xf00) >> 7; /* rotate amount */
> + imm = (imm >> rot) | (imm << (32 - rot));
> + fp_offset = -imm;
> + fi->framereg = FP_REGNUM;
> + }
> + else if ((insn & 0xfffff000) == 0xe24dd000) /* sub sp, sp #n */
> + {
> + unsigned imm = insn & 0xff; /* immediate value */
> + unsigned rot = (insn & 0xf00) >> 7; /* rotate amount */
> + imm = (imm >> rot) | (imm << (32 - rot));
> + sp_offset -= imm;
> + }
> + else if ((insn & 0xffff7fff) == 0xed6d0103) /* stfe f?, [sp, -#c]! */
> + {
> + sp_offset -= 12;
> + regno = F0_REGNUM + ((insn >> 12) & 0x07);
> + fi->fsr.regs[regno] = sp_offset;
> + }
> + else if ((insn & 0xffbf0fff) == 0xec2d0200) /* sfmfd f0, 4, [sp!] */
> + {
> + int n_saved_fp_regs;
> + unsigned int fp_start_reg, fp_bound_reg;
>
> - /* Calculate offsets of saved registers. */
> - for (regno = PC_REGNUM; regno >= 0; regno--)
> - if (mask & (1 << regno))
> - {
> - sp_offset -= 4;
> - fi->fsr.regs[regno] = sp_offset;
> - }
> - }
> - else if ((insn & 0xfffff000) == 0xe24cb000) /* sub fp, ip #n */
> + if ((insn & 0x800) == 0x800) /* N0 is set */
> {
> - unsigned imm = insn & 0xff; /* immediate value */
> - unsigned rot = (insn & 0xf00) >> 7; /* rotate amount */
> - imm = (imm >> rot) | (imm << (32 - rot));
> - fp_offset = -imm;
> - fi->framereg = FP_REGNUM;
> + if ((insn & 0x40000) == 0x40000) /* N1 is set */
> + n_saved_fp_regs = 3;
> + else
> + n_saved_fp_regs = 1;
> }
> - else if ((insn & 0xfffff000) == 0xe24dd000) /* sub sp, sp #n */
> + else
> {
> - unsigned imm = insn & 0xff; /* immediate value */
> - unsigned rot = (insn & 0xf00) >> 7; /* rotate amount */
> - imm = (imm >> rot) | (imm << (32 - rot));
> - sp_offset -= imm;
> + if ((insn & 0x40000) == 0x40000) /* N1 is set */
> + n_saved_fp_regs = 2;
> + else
> + n_saved_fp_regs = 4;
> }
> - else if ((insn & 0xffff7fff) == 0xed6d0103) /* stfe f?, [sp, -#c]! */
> +
> + fp_start_reg = F0_REGNUM + ((insn >> 12) & 0x7);
> + fp_bound_reg = fp_start_reg + n_saved_fp_regs;
> + for (; fp_start_reg < fp_bound_reg; fp_start_reg++)
> {
> sp_offset -= 12;
> - regno = F0_REGNUM + ((insn >> 12) & 0x07);
> - fi->fsr.regs[regno] = sp_offset;
> + fi->fsr.regs[fp_start_reg++] = sp_offset;
> }
> - else if ((insn & 0xffbf0fff) == 0xec2d0200) /* sfmfd f0, 4, [sp!] */
> - {
> - int n_saved_fp_regs;
> - unsigned int fp_start_reg, fp_bound_reg;
> -
> - if ((insn & 0x800) == 0x800) /* N0 is set */
> - {
> - if ((insn & 0x40000) == 0x40000) /* N1 is set */
> - n_saved_fp_regs = 3;
> - else
> - n_saved_fp_regs = 1;
> - }
> - else
> - {
> - if ((insn & 0x40000) == 0x40000) /* N1 is set */
> - n_saved_fp_regs = 2;
> - else
> - n_saved_fp_regs = 4;
> - }
> -
> - fp_start_reg = F0_REGNUM + ((insn >> 12) & 0x7);
> - fp_bound_reg = fp_start_reg + n_saved_fp_regs;
> - for (; fp_start_reg < fp_bound_reg; fp_start_reg++)
> - {
> - sp_offset -= 12;
> - fi->fsr.regs[fp_start_reg++] = sp_offset;
> - }
> - }
> - else if ((insn & 0xf0000000) != 0xe0000000)
> - break; /* Condition not true, exit early */
> - else if ((insn & 0xfe200000) == 0xe8200000) /* ldm? */
> - break; /* Don't scan past a block load */
> - else
> - /* The optimizer might shove anything into the prologue,
> - so we just skip what we don't recognize. */
> - continue;
> }
> + else if ((insn & 0xf0000000) != 0xe0000000)
> + break; /* Condition not true, exit early */
> + else if ((insn & 0xfe200000) == 0xe8200000) /* ldm? */
> + break; /* Don't scan past a block load */
> + else
> + /* The optimizer might shove anything into the prologue,
> + so we just skip what we don't recognize. */
> + continue;
> }
>
> /* The frame size is just the negative of the offset (from the original SP)
> of the last thing thing we pushed on the stack. The frame offset is
> [new FP] - [new SP]. */
> fi->framesize = -sp_offset;
> - fi->frameoffset = fp_offset - sp_offset;
> + if (fi->framereg == FP_REGNUM)
> + fi->frameoffset = fp_offset - sp_offset;
> + else
> + fi->frameoffset = 0;
>
> save_prologue_cache (fi);
> }
--
Fernando Nasser
Red Hat - Toronto E-Mail: fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario M4P 2C9