Message ID | 1410476585-18046-1-git-send-email-emachado@linux.vnet.ibm.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 27660 invoked by alias); 11 Sep 2014 23:03:21 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 27651 invoked by uid 89); 11 Sep 2014 23:03:20 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.0 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: e24smtp03.br.ibm.com Received: from e24smtp03.br.ibm.com (HELO e24smtp03.br.ibm.com) (32.104.18.24) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Thu, 11 Sep 2014 23:03:19 +0000 Received: from /spool/local by e24smtp03.br.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for <gdb-patches@sourceware.org> from <emachado@linux.vnet.ibm.com>; Thu, 11 Sep 2014 20:03:16 -0300 Received: from d24dlp02.br.ibm.com (9.18.248.206) by e24smtp03.br.ibm.com (10.172.0.139) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Thu, 11 Sep 2014 20:03:13 -0300 Received: from d24relay01.br.ibm.com (d24relay01.br.ibm.com [9.8.31.16]) by d24dlp02.br.ibm.com (Postfix) with ESMTP id 2B2F01DC0027 for <gdb-patches@sourceware.org>; Thu, 11 Sep 2014 19:03:12 -0400 (EDT) Received: from d24av05.br.ibm.com (d24av05.br.ibm.com [9.18.232.44]) by d24relay01.br.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id s8BN3aMx4915290 for <gdb-patches@sourceware.org>; Thu, 11 Sep 2014 20:03:36 -0300 Received: from d24av05.br.ibm.com (localhost [127.0.0.1]) by d24av05.br.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id s8BN3BsV031281 for <gdb-patches@sourceware.org>; Thu, 11 Sep 2014 19:03:11 -0400 Received: from grandaddy.ibm.com ([9.18.199.8]) by d24av05.br.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id s8BN3BIp031267; Thu, 11 Sep 2014 19:03:11 -0400 From: Edjunior Barbosa Machado <emachado@linux.vnet.ibm.com> To: gdb-patches@sourceware.org Cc: Ulrich Weigand <uweigand@de.ibm.com> Subject: =?UTF-8?q?=5BPATCH=5D=20=5BPR=20tdep/17379=5D=20Fix=20internal-error=20when=20stack=20pointer=20is=20invalid?= Date: Thu, 11 Sep 2014 20:03:05 -0300 Message-Id: <1410476585-18046-1-git-send-email-emachado@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 14091123-0009-0000-0000-000001BFDD8E X-IsSubscribed: yes |
Commit Message
Edjunior Barbosa Machado
Sept. 11, 2014, 11:03 p.m. UTC
Hi, this patch intends to fix PR tdep/17379: https://sourceware.org/bugzilla/show_bug.cgi?id=17379 The problem is that rs6000_frame_cache attempts to read the stack backchain via read_memory_unsigned_integer, which throws an exception if the stack pointer is invalid. With this path, it calls safe_read_memory_integer instead, which doesn't throw an exception and allows for safe handling of that situation. Regression tested on ppc64{,le}. Ok? Thanks and regards, -- Edjunior gdb/ 2014-09-11 Edjunior Barbosa Machado <emachado@linux.vnet.ibm.com> Ulrich Weigand <uweigand@de.ibm.com> PR tdep/17379 * rs6000-tdep.c (rs6000_frame_cache): Use safe_read_memory_integer instead of read_memory_unsigned_integer. --- gdb/rs6000-tdep.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
Comments
On Thursday, September 11 2014, Edjunior Barbosa Machado wrote: > The problem is that rs6000_frame_cache attempts to read the stack backchain via > read_memory_unsigned_integer, which throws an exception if the stack pointer is > invalid. With this path, it calls safe_read_memory_integer instead, which > doesn't throw an exception and allows for safe handling of that situation. > Regression tested on ppc64{,le}. Ok? Heya! Thanks for the patch. Not having reviewed the code deeply to understand if it's the best approach, I would just like to point that a testcase for this would be awesome. As it turns out, you actually already have a testcase almost written in the bug description :-). Again, thanks for addressing those issues! Cheers,
Hi guys, See https://sourceware.org/bugzilla/show_bug.cgi?id=17384 . When safe_read_memory_integer call fails, GDB prints a surprising/confusing error message, more so in case the unwinder is triggered for some reason other than the "bt" command, like with "step"/"next". I take you're now seeing the same errors with this patch. IMO, printing the error is not something a low-level helper function like safe_read_memory_integer should be doing, as GDB uses it when probing with heuristics because it can't sure its guesses make sense (whether there's a frame at all, etc.) safe_frame_unwind_memory, which is used in rs6000_in_function_epilogue_p doesn't print the error either. Thoughts? Thanks, Pedro Alves On 09/12/2014 12:03 AM, Edjunior Barbosa Machado wrote: > Hi, > > this patch intends to fix PR tdep/17379: > https://sourceware.org/bugzilla/show_bug.cgi?id=17379 > > The problem is that rs6000_frame_cache attempts to read the stack backchain via > read_memory_unsigned_integer, which throws an exception if the stack pointer is > invalid. With this path, it calls safe_read_memory_integer instead, which > doesn't throw an exception and allows for safe handling of that situation. > Regression tested on ppc64{,le}. Ok? > > Thanks and regards, > -- > Edjunior > > gdb/ > 2014-09-11 Edjunior Barbosa Machado <emachado@linux.vnet.ibm.com> > Ulrich Weigand <uweigand@de.ibm.com> > > PR tdep/17379 > * rs6000-tdep.c (rs6000_frame_cache): Use safe_read_memory_integer > instead of read_memory_unsigned_integer. > > --- > gdb/rs6000-tdep.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c > index 730afe7..dabf448 100644 > --- a/gdb/rs6000-tdep.c > +++ b/gdb/rs6000-tdep.c > @@ -3190,9 +3190,14 @@ rs6000_frame_cache (struct frame_info *this_frame, void **this_cache) > } > > if (!fdata.frameless) > - /* Frameless really means stackless. */ > - cache->base > - = read_memory_unsigned_integer (cache->base, wordsize, byte_order); > + { > + /* Frameless really means stackless. */ > + LONGEST backchain; > + > + if (safe_read_memory_integer (cache->base, wordsize, > + byte_order, &backchain)) > + cache->base = (CORE_ADDR) backchain; > + } > > trad_frame_set_value (cache->saved_regs, > gdbarch_sp_regnum (gdbarch), cache->base);
diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c index 730afe7..dabf448 100644 --- a/gdb/rs6000-tdep.c +++ b/gdb/rs6000-tdep.c @@ -3190,9 +3190,14 @@ rs6000_frame_cache (struct frame_info *this_frame, void **this_cache) } if (!fdata.frameless) - /* Frameless really means stackless. */ - cache->base - = read_memory_unsigned_integer (cache->base, wordsize, byte_order); + { + /* Frameless really means stackless. */ + LONGEST backchain; + + if (safe_read_memory_integer (cache->base, wordsize, + byte_order, &backchain)) + cache->base = (CORE_ADDR) backchain; + } trad_frame_set_value (cache->saved_regs, gdbarch_sp_regnum (gdbarch), cache->base);