From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10061 invoked by alias); 7 Dec 2005 22:32:57 -0000 Received: (qmail 10050 invoked by uid 22791); 7 Dec 2005 22:32:56 -0000 X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (66.187.233.31) by sourceware.org (qpsmtpd/0.31) with ESMTP; Wed, 07 Dec 2005 22:32:54 +0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.12.11/8.12.11) with ESMTP id jB7MWrwt019440 for ; Wed, 7 Dec 2005 17:32:53 -0500 Received: from pobox.corp.redhat.com (pobox.corp.redhat.com [172.16.52.156]) by int-mx1.corp.redhat.com (8.11.6/8.11.6) with ESMTP id jB7MWrV12418; Wed, 7 Dec 2005 17:32:53 -0500 Received: from localhost.localdomain (vpn50-83.rdu.redhat.com [172.16.50.83]) by pobox.corp.redhat.com (8.12.8/8.12.8) with ESMTP id jB7MWqOp003862; Wed, 7 Dec 2005 17:32:52 -0500 Received: from ironwood.lan (ironwood.lan [192.168.64.8]) by localhost.localdomain (8.12.11/8.12.10) with ESMTP id jB7MWlRN005853; Wed, 7 Dec 2005 15:32:47 -0700 Date: Thu, 08 Dec 2005 09:44:00 -0000 From: Kevin Buettner To: gdb-patches@sources.redhat.com Cc: Michael Snyder Subject: [RFA] mn10300 prologue analyzer fixes Message-ID: <20051207153245.49b0342c@ironwood.lan> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2005-12/txt/msg00140.txt.bz2 This patch actually contains two distinct fixes, but one of them is a one-liner... The bulk of the patch is concerned with the pattern match that culminates in finding a series of fmov instructions. As explained in the comments, the code attempted to defer the increment of `addr' until it was certain that the pattern match succeeded. Unfortunately, it fails to do the right thing in at least two places. The first is just below that comment which used to say "We're committed now." The second is in the `for' loop which processes the fmov instructions. The initialization portion of this loop advances `addr' prior to a successful read of an fmov instruction. If / when we do either of these increments, but later fail to find an fmov instruction, the remainder of the prologue scan will continue with `addr' advanced too far. The one-liner portion of the patch concerns the adjustment of `stack_extra_size' in the last hunk of the patch. As I recall, the size being extracted is a negative quantity, which necessitates changing the sign. This bug was discovered when attempting to backtrace through a frame-pointer-less function. Okay? * mn10300-tdep.c (mn10300_analyze_prologue): Implement backtrack out of pattern match by saving relevant state. Fix stack size adjustment bug. Index: mn10300-tdep.c =================================================================== RCS file: /cvs/src/src/gdb/mn10300-tdep.c,v retrieving revision 1.134 diff -u -p -r1.134 mn10300-tdep.c --- mn10300-tdep.c 8 Sep 2005 22:48:56 -0000 1.134 +++ mn10300-tdep.c 7 Dec 2005 22:07:09 -0000 @@ -620,6 +620,17 @@ mn10300_analyze_prologue (struct frame_i [mov sp,a3] [mov sp,a3] [add -SIZE2,sp] [add -SIZE2,sp] */ + /* Remember the address at which we started in the event that we + don't ultimately find an fmov instruction. Once we're certain + that we matched one of the above patterns, we'll set + ``restore_addr'' to the appropriate value. Note: At one time + in the past, this code attempted to not adjust ``addr'' until + there was a fair degree of certainty that the pattern would be + matched. However, that code did not wait until an fmov instruction + was actually encountered. As a consequence, ``addr'' would + sometimes be advanced even when no fmov instructions were found. */ + CORE_ADDR restore_addr = addr; + /* First, look for add -SIZE,sp (i.e. add imm8,sp (0xf8feXX) or add imm16,sp (0xfafeXXXX) or add imm32,sp (0xfcfeXXXXXXXX)) */ @@ -651,10 +662,10 @@ mn10300_analyze_prologue (struct frame_i This is a one byte instruction: mov sp,aN = 0011 11XX where XX is the register number. - Skip this instruction by incrementing addr. (We're - committed now.) The "fmov" instructions will have the - form "fmov fs#,(aN+)" in this case, but that will not - necessitate a change in the "fmov" parsing logic below. */ + Skip this instruction by incrementing addr. The "fmov" + instructions will have the form "fmov fs#,(aN+)" in this + case, but that will not necessitate a change in the + "fmov" parsing logic below. */ addr++; @@ -698,6 +709,14 @@ mn10300_analyze_prologue (struct frame_i if (buf[0] != 0xf9 && buf[0] != 0xfb) break; + /* An fmov instruction has just been seen. We can + now really commit to the pattern match. Set the + address to restore at the end of this speculative + bit of code to the actually address that we've + been incrementing (or not) throughout the + speculation. */ + restore_addr = addr; + /* Get the floating point register number from the 2nd and 3rd bytes of the "fmov" instruction: Machine Code: 0000 00X0 YYYY 0000 => @@ -719,6 +738,7 @@ mn10300_analyze_prologue (struct frame_i { /* No "fmov" was found. Reread the two bytes at the original "addr" to reset the state. */ + addr = restore_addr; if (!safe_frame_unwind_memory (fi, addr, buf, 2)) goto finish_prologue; } @@ -727,8 +747,16 @@ mn10300_analyze_prologue (struct frame_i instruction. Handle this below. */ } /* else no "add -SIZE,sp" was found indicating no floating point - registers are saved in this prologue. Do not increment addr. Pretend - this bit of code never happened. */ + registers are saved in this prologue. */ + + /* In the pattern match code contained within this block, `restore_addr' + is set to the starting address at the very beginning and then + iteratively to the next address to start scanning at once the + pattern match has succeeded. Thus `restore_addr' will contain + the address to rewind to if the pattern match failed. If the + match succeeded, `restore_addr' and `addr' will already have the + same value. */ + addr = restore_addr; } /* Now see if we set up a frame pointer via "mov sp,a3" */ @@ -777,7 +805,7 @@ mn10300_analyze_prologue (struct frame_i goto finish_prologue; /* Note the size of the stack. */ - stack_extra_size += extract_signed_integer (buf, imm_size); + stack_extra_size -= extract_signed_integer (buf, imm_size); /* We just consumed 2 + imm_size bytes. */ addr += 2 + imm_size;