From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25522 invoked by alias); 7 Mar 2007 21:42:16 -0000 Received: (qmail 25514 invoked by uid 22791); 7 Mar 2007 21:42:15 -0000 X-Spam-Check-By: sourceware.org Received: from nile.gnat.com (HELO nile.gnat.com) (205.232.38.5) by sourceware.org (qpsmtpd/0.31) with ESMTP; Wed, 07 Mar 2007 21:42:09 +0000 Received: from localhost (localhost [127.0.0.1]) by filtered-nile.gnat.com (Postfix) with ESMTP id CB05548CBF0 for ; Wed, 7 Mar 2007 16:42:06 -0500 (EST) Received: from nile.gnat.com ([127.0.0.1]) by localhost (nile.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id 18011-01 for ; Wed, 7 Mar 2007 16:42:06 -0500 (EST) Received: from takamaka.act-europe.fr (unknown [70.71.0.212]) by nile.gnat.com (Postfix) with ESMTP id 314DF48CBD1 for ; Wed, 7 Mar 2007 16:42:05 -0500 (EST) Received: by takamaka.act-europe.fr (Postfix, from userid 1000) id 3A633E7B38; Wed, 7 Mar 2007 13:42:28 -0800 (PST) Date: Wed, 07 Mar 2007 21:42:00 -0000 From: Joel Brobecker To: gdb-patches@sourceware.org Subject: Re: [RFA/mips(commit?)] Unwinding from noreturn function Message-ID: <20070307214228.GB18974@adacore.com> References: <20070307041643.GJ25742@adacore.com> <20070307122032.GB18998@caradoc.them.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="ikeVEW9yuYc//A+q" Content-Disposition: inline In-Reply-To: <20070307122032.GB18998@caradoc.them.org> User-Agent: Mutt/1.4.2.2i 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: 2007-03/txt/msg00079.txt.bz2 --ikeVEW9yuYc//A+q Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-length: 2032 > I Am Dumb. Check CVS history, but I think I changed that just a > couple of weeks ago; I audited all the sniffers looking for what ought > to use the unwound PC and what ought to use the unwound block address. > Here, I'm pretty sure I made the wrong choice. Yep, I remembered. However, I also thought that your choice made sense. I really think it does, but given: > I would recommend you revert my changes to this function and > mips_insn32_frame_sniffer instead. And: > > It seems to me that the above check is only an optimization, > > and I've spotted at least one instance where I cannot see an > > obvious guaranty that the address has not been decremented > > by one of the _in_block functions... So the decision I made > > was to remove that check. > > No, it's not just an optimization. Especially with limited debug > info, it's important. ^^^^^^^^^^^^^^ I decided to revert your changes for now. > > 2. One minor: There was a confusion in the unwinder between > > the return address and the address of the instruction calling us. > > So I replaced frame_pc_unwind calls by their associated > > frame_unwind_address_in_block. > > This half looks right. Thanks. So here is what I ended up checkin in: 2007-03-07 Joel Brobecker * mips-tdep.c (mips_insn16_frame_cache, mips_insn32_frame_sniffer): Revert the previous change that had some unexpected side-effects on mips32. (mips_insn16_frame_cache, mips_insn32_frame_cache): Use the proper function to get the address of the calling instruction. Re-tested on mips-irix, just to be sure. Same results as before (meaning about 500 less FAILs). I'm also sad to report that I have been told to put off work on mips-irix for a while. That was a personal initiative on my side, but I'm lacking time at work, so this was the first task that got cut. I hope someone else can find the time to bring it back to life... Thanks again, Daniel. -- Joel --ikeVEW9yuYc//A+q Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="mips-tdep.c.diff" Content-length: 1716 Index: mips-tdep.c =================================================================== RCS file: /cvs/src/src/gdb/mips-tdep.c,v retrieving revision 1.404 diff -u -p -r1.404 mips-tdep.c --- mips-tdep.c 27 Feb 2007 20:17:19 -0000 1.404 +++ mips-tdep.c 7 Mar 2007 21:26:32 -0000 @@ -1640,7 +1640,8 @@ mips_insn16_frame_cache (struct frame_in /* Analyze the function prologue. */ { - const CORE_ADDR pc = frame_pc_unwind (next_frame); + const CORE_ADDR pc = + frame_unwind_address_in_block (next_frame, NORMAL_FRAME); CORE_ADDR start_addr; find_pc_partial_function (pc, NULL, &start_addr, NULL); @@ -1693,7 +1694,7 @@ static const struct frame_unwind mips_in static const struct frame_unwind * mips_insn16_frame_sniffer (struct frame_info *next_frame) { - CORE_ADDR pc = frame_unwind_address_in_block (next_frame, NORMAL_FRAME); + CORE_ADDR pc = frame_pc_unwind (next_frame); if (mips_pc_is_mips16 (pc)) return &mips_insn16_frame_unwind; return NULL; @@ -1961,7 +1962,8 @@ mips_insn32_frame_cache (struct frame_in /* Analyze the function prologue. */ { - const CORE_ADDR pc = frame_pc_unwind (next_frame); + const CORE_ADDR pc = + frame_unwind_address_in_block (next_frame, NORMAL_FRAME); CORE_ADDR start_addr; find_pc_partial_function (pc, NULL, &start_addr, NULL); @@ -2014,7 +2016,7 @@ static const struct frame_unwind mips_in static const struct frame_unwind * mips_insn32_frame_sniffer (struct frame_info *next_frame) { - CORE_ADDR pc = frame_unwind_address_in_block (next_frame, NORMAL_FRAME); + CORE_ADDR pc = frame_pc_unwind (next_frame); if (! mips_pc_is_mips16 (pc)) return &mips_insn32_frame_unwind; return NULL; --ikeVEW9yuYc//A+q--