From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30719 invoked by alias); 24 Feb 2013 12:55:35 -0000 Received: (qmail 30709 invoked by uid 22791); 24 Feb 2013 12:55:34 -0000 X-SWARE-Spam-Status: No, hits=-4.5 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 24 Feb 2013 12:55:29 +0000 Received: from svr-orw-fem-01.mgc.mentorg.com ([147.34.98.93]) by relay1.mentorg.com with esmtp id 1U9b72-0000Cy-7B from Maciej_Rozycki@mentor.com ; Sun, 24 Feb 2013 04:55:28 -0800 Received: from SVR-IES-FEM-01.mgc.mentorg.com ([137.202.0.104]) by svr-orw-fem-01.mgc.mentorg.com over TLS secured channel with Microsoft SMTPSVC(6.0.3790.4675); Sun, 24 Feb 2013 04:55:27 -0800 Received: from [172.30.64.88] (137.202.0.76) by SVR-IES-FEM-01.mgc.mentorg.com (137.202.0.104) with Microsoft SMTP Server id 14.1.289.1; Sun, 24 Feb 2013 12:55:26 +0000 Date: Sun, 24 Feb 2013 12:55:00 -0000 From: "Maciej W. Rozycki" To: Pierre Muller CC: Subject: [committed] Fix MIPS frame prologue scan problem In-Reply-To: Message-ID: 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> User-Agent: Alpine 1.10 (DEB 962 2008-03-14) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" 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/msg00609.txt.bz2 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 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)