From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9886 invoked by alias); 18 Mar 2004 16:15:41 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 9872 invoked from network); 18 Mar 2004 16:15:39 -0000 Received: from unknown (HELO mx1.redhat.com) (66.187.233.31) by sources.redhat.com with SMTP; 18 Mar 2004 16:15:39 -0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.12.10/8.12.10) with ESMTP id i2IGFd4b030140 for ; Thu, 18 Mar 2004 11:15:39 -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 i2IGFdj19997 for ; Thu, 18 Mar 2004 11:15:39 -0500 Received: from localhost.localdomain (vpn50-70.rdu.redhat.com [172.16.50.70]) by pobox.corp.redhat.com (8.12.8/8.12.8) with ESMTP id i2IGFcK0003254 for ; Thu, 18 Mar 2004 11:15:38 -0500 Received: from saguaro (saguaro.lan [192.168.64.2]) by localhost.localdomain (8.12.10/8.12.10) with SMTP id i2IGFXcG026366 for ; Thu, 18 Mar 2004 09:15:33 -0700 Date: Thu, 18 Mar 2004 16:15:00 -0000 From: Kevin Buettner To: gdb-patches@sources.redhat.com Subject: Re: [rfa/ppc] prologue parser tweaks Message-ID: <20040318091532.63ae9d21@saguaro> In-Reply-To: <40577FA6.5080506@gnu.org> References: <40577FA6.5080506@gnu.org> Organization: Red Hat Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-SW-Source: 2004-03.o/txt/msg00424.txt Message-ID: <20040318161500.98ctJQGUW79hQaqC0522xjfNtPBVcE_sUVDTn6xVm0I@z> On Tue, 16 Mar 2004 17:28:54 -0500 Andrew Cagney wrote: > * rs6000-tdep.c: Add field "func_start". If this part goes in, it should be: * rs6000-tdep.c (struct rs6000_framedata): Add field "func_start". > (skip_prologue): New variable num_skip_syscall_insn, use to skip > over first half of a GNU/Linux syscall and update "func_start". > Record only the first LR save, use to skip over PIC code. > > Index: rs6000-tdep.c > =================================================================== > RCS file: /cvs/src/src/gdb/rs6000-tdep.c,v > retrieving revision 1.183 > diff -u -r1.183 rs6000-tdep.c > --- rs6000-tdep.c 2 Mar 2004 02:20:25 -0000 1.183 > +++ rs6000-tdep.c 16 Mar 2004 22:08:24 -0000 > @@ -65,6 +65,7 @@ > > struct rs6000_framedata > { > + CORE_ADDR func_start; /* true function start */ Why do you need to add this field? Do you have some other patches which depend on it? > int offset; /* total size of frame --- the distance > by which we decrement sp to allocate > the frame */ > @@ -502,6 +503,7 @@ > int minimal_toc_loaded = 0; > int prev_insn_was_prologue_insn = 1; > int num_skip_non_prologue_insns = 0; > + int num_skip_syscall_insn = 0; Could you add a brief comment indicating that this is a state variable for the PPC64 GNU/Linux system call tests? I don't want someone to inadvertently start using it for some other purpose. Hmm, even better might be to name it something like ``ppc64_linux_syscall_state''. With a name like that, it's unlikely that it'll inadvertently get used for some other purpose. > const struct bfd_arch_info *arch_info = gdbarch_bfd_arch_info (current_gdbarch); > struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch); > > @@ -521,6 +523,7 @@ > lim_pc = refine_prologue_limit (pc, lim_pc); > > memset (fdata, 0, sizeof (struct rs6000_framedata)); > + fdata->func_start = pc; > fdata->saved_gpr = -1; > fdata->saved_fpr = -1; > fdata->saved_vr = -1; > @@ -548,6 +551,70 @@ [...] > + if (((op & 0xffff0000) == 0x38000000 /* li r0,N */ > + && pc == fdata->func_start + 0) It seems to me that substituting ``orig_pc'' in place of ``fdata->func_start'' will work, wont't it? E.g.: && pc == orig_pc + 0) > + || (op == 0x44000002 /* sc */ > + && pc == fdata->func_start + 4 && pc == orig_pc + 4 ?? > + && num_skip_syscall_insn == 1) > + || (op == 0x4ca30020 /* bnslr+ */ > + && pc == fdata->func_start + 8 && pc == orig_pc + 8 ?? > + && num_skip_syscall_insn == 2)) > + { > + num_skip_syscall_insn++; > + continue; > + } > + else if ((op & 0xfc000003) == 0x48000000 /* b __syscall_error */ > + && pc == fdata->func_start + 12 && pc == orig_pc + 12 > + && num_skip_syscall_insn == 3) > + { > + num_skip_syscall_insn++; > + fdata->func_start = pc; What's this about? Where else does fdata->func_start get used? > + continue; > + } I'd like to see the above test moved down a ways in the sequence of tests. Hmm, maybe it could go right before the AltiVec tests? > + > + if ((op & 0xfc1fffff) == 0x7c0802a6) > + { /* mflr Rx */ > + /* Since shared library / PIC code, which needs to get its > + address at runtime, can appear to save more than one link > + register vis: > + > + *INDENT-OFF* > + stwu r1,-304(r1) > + mflr r3 > + bl 0xff570d0 (blrl) > + stw r30,296(r1) > + mflr r30 > + stw r31,300(r1) > + stw r3,308(r1); > + ... > + *INDENT-ON* > + > + remember just the first one, but skip over additional > + ones. */ > + if (lr_reg < 0) > + lr_reg = (op & 0x03e00000); > + continue; > + } > > if ((op & 0xfc1fffff) == 0x7c0802a6) > { /* mflr Rx */ The above is okay so long as you remove the old "mflr Rx" test.