From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31804 invoked by alias); 24 Feb 2013 21:52:17 -0000 Received: (qmail 31796 invoked by uid 22791); 24 Feb 2013 21:52:16 -0000 X-SWARE-Spam-Status: No, hits=-1.6 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED,MSGID_MULTIPLE_AT X-Spam-Check-By: sourceware.org Received: from mailhost.u-strasbg.fr (HELO mailhost.u-strasbg.fr) (130.79.200.158) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 24 Feb 2013 21:52:09 +0000 Received: from md16.u-strasbg.fr (md16.u-strasbg.fr [130.79.200.206]) by mailhost.u-strasbg.fr (8.14.3/jtpda-5.5pre1) with ESMTP id r1OLq09s037361 ; Sun, 24 Feb 2013 22:52:00 +0100 (CET) (envelope-from pierre.muller@ics-cnrs.unistra.fr) Received: from mailserver.u-strasbg.fr (ms12.u-strasbg.fr [130.79.204.112]) by md16.u-strasbg.fr (8.14.3/jtpda-5.5pre1) with ESMTP id r1OLpx7T011194 ; Sun, 24 Feb 2013 22:52:00 +0100 (envelope-from pierre.muller@ics-cnrs.unistra.fr) Received: from E6510Muller (lec67-4-82-230-53-140.fbx.proxad.net [82.230.53.140]) (user=mullerp mech=LOGIN) by mailserver.u-strasbg.fr (8.14.3/jtpda-5.5pre1) with ESMTP id r1OLpvOF000892 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=NO) ; Sun, 24 Feb 2013 22:51:58 +0100 (envelope-from pierre.muller@ics-cnrs.unistra.fr) From: "Pierre Muller" To: "'Maciej W. Rozycki'" Cc: References: <00a501cd495e$db6adea0$92409be0$@muller@ics-cnrs.unistra.fr> <002a01cd5043$a0e9f310$e2bdd930$@muller@ics-cnrs.unistra.fr> <005901cdcbef$c4a28560$4de79020$@muller@ics-cnrs.unistra.fr> In-Reply-To: Subject: RE: [committed] Fix MIPS frame prologue scan problem Date: Sun, 24 Feb 2013 21:52:00 -0000 Message-ID: <000001ce12d9$2b3aa570$81aff050$@muller@ics-cnrs.unistra.fr> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2013-02/txt/msg00610.txt.bz2 Thank you so much for fixing this issue! I can confirm that after your patch, I can use=20 unmodified GDB CVS tree to generate a GDB executable that is able to correctly handle the prologue generated by the free pascal compiler which uses Dump of assembler code for function TOPTION__INTERPRET_OPTION: 0x005ef624 <+0>: addiu sp,sp,-1928 0x005ef628 <+4>: sw ra,212(sp) 0x005ef62c <+8>: sw s8,208(sp) 0x005ef630 <+12>: addiu s8,sp,1928 The value of $fp is equal to $s8 after your patch, which makes it undistinguishable from the patch I proposed... I did not find any gdb_7_6-branch yet, which probably means that this will be in 7.6 release, no? Let me end by thanking you again=20=20 Pierre Muller as Free Pascal core developer > -----Message d'origine----- > De=A0: gdb-patches-owner@sourceware.org [mailto:gdb-patches- > owner@sourceware.org] De la part de Maciej W. Rozycki > Envoy=E9=A0: dimanche 24 f=E9vrier 2013 13:55 > =C0=A0: Pierre Muller > Cc=A0: gdb-patches@sourceware.org > Objet=A0: [committed] Fix MIPS frame prologue scan problem >=20 > On Mon, 26 Nov 2012, Maciej W. Rozycki wrote: >=20 > > > > 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 =3D 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. >=20 > 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. >=20 > 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: >=20 > if (has_frame_reg) { > PROC_FRAME_REG(&temp_proc_desc) =3D 30; > PROC_FRAME_OFFSET(&temp_proc_desc) =3D 0; > } > else { > PROC_FRAME_REG(&temp_proc_desc) =3D SP_REGNUM; > PROC_FRAME_OFFSET(&temp_proc_desc) =3D frame_size; > } >=20 > After the problematic change the PROC_FRAME_OFFSET(&temp_proc_desc) =3D 0 > assignment was lost. >=20 > 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). >=20 > 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. >=20 > Again, thanks for your patience and apologies to take so long. >=20 > Maciej >=20 > 2013-02-24 Maciej W. Rozycki >=20 > gdb/ > * mips-tdep.c (mips32_scan_prologue): Reset frame_offset to zero > if $fp is used as the virtual frame pointer. >=20 > Index: gdb-fsf-trunk-quilt/gdb/mips-tdep.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- 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 =3D 30; > frame_addr =3D get_frame_register_signed > (this_frame, gdbarch_num_regs (gdbarch) + 30); > + frame_offset =3D 0; >=20 > alloca_adjust =3D (unsigned) (frame_addr - (sp + low_word)); > if (alloca_adjust > 0)