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, Nick Clifton <nickc at cygnus dot com>, scottb at netwinder dot org
- Date: Tue, 27 Nov 2001 11:47:20 -0500
- Subject: Re: [PATCH RFA] arm-tdep.c: prologue scanner adjustments
- Organization: Red Hat Canada
- References: <1011127063801.ZM10619@ocotillo.lan>
Kevin,
We must constantly be in touch with GCC folks to get our prologue
scan in synch with the latest possible prologue code generation
patterns put out by the compiler.
If you found new cases and is now handling them, we should get
your changes in.
I can only speak for the remote ARM targets. In cases like above
we should also ask Scott (Linux/ARM). (I'm certain he will approve
though).
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 Canada Ltd. E-Mail: fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario M4P 2C9