From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26974 invoked by alias); 28 Feb 2006 22:33:02 -0000 Received: (qmail 26962 invoked by uid 22791); 28 Feb 2006 22:33:01 -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; Tue, 28 Feb 2006 22:32:59 +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 k1SMWvi5014718 for ; Tue, 28 Feb 2006 17:32:57 -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 k1SMWv122583 for ; Tue, 28 Feb 2006 17:32:57 -0500 Received: from localhost.localdomain (vpn50-44.rdu.redhat.com [172.16.50.44]) by pobox.corp.redhat.com (8.12.8/8.12.8) with ESMTP id k1SMWufp011461 for ; Tue, 28 Feb 2006 17:32:56 -0500 Received: from ironwood.lan (ironwood.lan [192.168.64.8]) by localhost.localdomain (8.12.11/8.12.10) with ESMTP id k1SMYohT004388 for ; Tue, 28 Feb 2006 15:34:50 -0700 Date: Tue, 28 Feb 2006 22:52:00 -0000 From: Kevin Buettner To: gdb-patches@sources.redhat.com Subject: Re: [RFA] mn10300 prologue analyzer fixes Message-ID: <20060228153254.6096c4ef@ironwood.lan> In-Reply-To: <44049B32.8070503@redhat.com> References: <20051207153245.49b0342c@ironwood.lan> <44049B32.8070503@redhat.com> 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: 2006-02/txt/msg00513.txt.bz2 On Tue, 28 Feb 2006 10:49:22 -0800 Michael Snyder wrote: > Looks good, except that the indentation seems staggered in a > couple of block comments. Might just be a tabs artifact... Yes, it was indeed a tabs artifact. I found two lines which were using spaces instead of tabs. I fixed those lines to match the whitespace convention used in the rest of the comment and now things look better. I've just committed the patch below: 2006-02-28 Kevin Buettner * 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.135 retrieving revision 1.136 diff -u -p -r1.135 -r1.136 --- mn10300-tdep.c 17 Dec 2005 22:34:01 -0000 1.135 +++ mn10300-tdep.c 28 Feb 2006 22:28:21 -0000 1.136 @@ -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;