This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[committed] Fix MIPS frame prologue scan problem
- From: "Maciej W. Rozycki" <macro at codesourcery dot com>
- To: Pierre Muller <pierre dot muller at ics-cnrs dot unistra dot fr>
- Cc: <gdb-patches at sourceware dot org>
- Date: Sun, 24 Feb 2013 12:55:17 +0000
- Subject: [committed] Fix MIPS frame prologue scan problem
- References: <00a501cd495e$db6adea0$92409be0$@muller@ics-cnrs.unistra.fr> <alpine.DEB.1.10.1206212329420.23962@tp.orcam.me.uk> <002a01cd5043$a0e9f310$e2bdd930$@muller@ics-cnrs.unistra.fr> <005901cdcbef$c4a28560$4de79020$@muller@ics-cnrs.unistra.fr> <alpine.DEB.1.10.1211261641120.11298@tp.orcam.me.uk>
On Mon, 26 Nov 2012, Maciej W. Rozycki wrote:
> > > But in mips32_scan_prologue,
> > > the first
> > > ADDI $s8,$sp,LocalSize
> > > instruction,
> > > interpreted it in mips32_scan_prologue function
> > > but ended up with a wrong position of my
> > > non-ABI standard frame pointer
> > > because it changed frame pointer register from sp to s8 register,
> > > but kept frame_offset value as set by the
> > > SUBI $sp, $sp, LocalSize
> > > instruction
> > > analyzed before.
> > >
> > > Thus GDB wrongly ends up with a
> > > frame pointer located a
> > > value of $s8 register (as from ADDI instruction analysis)
> > > + LocalSize (from SUBI instruction)
> > >
> > > This means that of
> > > $sp is say at address addr
> > > $s8 is at addr +LocalSize
> > > and the virtual frame pointer
> > > $fp at $s8 + LocalSize = addr + 2 * LocalSize
> > >
> > >
> > > This means that it would be better to remove
> > > analysis of the ADDI $s8, $sp, LocalSize
> > > than to leave the current behavior.
> > >
> > > I think that we should either use my proposed patch,
> > > or completely remove the analysis of this ADDI $s8, $sp, LocalSize...
>
> Given the ambiguities noted above and following the principle of being
> liberal as to what to accept I agree your proposal makes sense. I am
> fairly sure though that in the presence of a hard frame pointer ($fp) it
> is that value we should rely on as it's there for a reason.
Thanks for your patience. I have now looked through it in depth and your
observation is actually a regression introduced sometime between 4.16 and
4.17, probably when the function was rewritten when MIPS16 support was
added sometime in 1997 and the original heuristic_proc_desc code forked
into mips32_heuristic_proc_desc and mips16_heuristic_proc_desc, which is
how the respective functions were called back then.
Before that regression frame_offset was unconditionally forced to zero
whenever $fp was used to hold the virtual frame pointer -- the only use
case supported for $fp back then. Original code read:
if (has_frame_reg) {
PROC_FRAME_REG(&temp_proc_desc) = 30;
PROC_FRAME_OFFSET(&temp_proc_desc) = 0;
}
else {
PROC_FRAME_REG(&temp_proc_desc) = SP_REGNUM;
PROC_FRAME_OFFSET(&temp_proc_desc) = frame_size;
}
After the problematic change the PROC_FRAME_OFFSET(&temp_proc_desc) = 0
assignment was lost.
I have therefore applied the change below; no change is needed for MIPS16
or microMIPS analysers as they use different code -- with frame_adjust
holding the extra frame adjustment in the corresponding case (it may make
sense to make all the three pieces more uniform, but that's another
matter).
This is not a code path that is ever used with modern GCC or it would
have likely been spotted much earlier than 15 years after the regression
and it is therefore not really covered by the test suite. I have run it
regardless, for the MIPS/Linux target and a couple multilibs, just for the
peace of mind.
Again, thanks for your patience and apologies to take so long.
Maciej
2013-02-24 Maciej W. Rozycki <macro@codesourcery.com>
gdb/
* mips-tdep.c (mips32_scan_prologue): Reset frame_offset to zero
if $fp is used as the virtual frame pointer.
Index: gdb-fsf-trunk-quilt/gdb/mips-tdep.c
===================================================================
--- gdb-fsf-trunk-quilt.orig/gdb/mips-tdep.c 2013-02-24 00:29:58.000000000 +0000
+++ gdb-fsf-trunk-quilt/gdb/mips-tdep.c 2013-02-24 01:34:10.834052078 +0000
@@ -3316,6 +3316,7 @@ mips32_scan_prologue (struct gdbarch *gd
frame_reg = 30;
frame_addr = get_frame_register_signed
(this_frame, gdbarch_num_regs (gdbarch) + 30);
+ frame_offset = 0;
alloca_adjust = (unsigned) (frame_addr - (sp + low_word));
if (alloca_adjust > 0)