From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 91373 invoked by alias); 9 Jun 2015 18:44:12 -0000 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 Received: (qmail 91362 invoked by uid 89); 9 Jun 2015 18:44:11 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.2 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY autolearn=no version=3.3.2 X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Tue, 09 Jun 2015 18:44:11 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 2D58528B58; Tue, 9 Jun 2015 14:44:09 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id EbZ4DZZHuX2C; Tue, 9 Jun 2015 14:44:09 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 141E328B55; Tue, 9 Jun 2015 14:44:09 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 9B0BA4067C; Tue, 9 Jun 2015 14:44:08 -0400 (EDT) Date: Tue, 09 Jun 2015 18:44:00 -0000 From: Joel Brobecker To: Martin Simmons Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] Prevent internal-error when computing $pc in ARM assembly code Message-ID: <20150609184408.GG2855@adacore.com> References: <201505200949.t4K9nlqt015737@heapu.cam.lispworks.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201505200949.t4K9nlqt015737@heapu.cam.lispworks.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-SW-Source: 2015-06/txt/msg00150.txt.bz2 On Wed, May 20, 2015 at 10:49:47AM +0100, Martin Simmons wrote: > This patch prevents a gdb internal-error when computing $pc for ARM assembly > code that doesn't use the normal stack frame convention. > > It might also help with gdb/12223. > > I'm not subscribed to the list so please include me on any replies. > > > gdb/ChangeLog: > > * arm-tdep.c (arm_analyze_prologue): Read memory without throwing an > exception, to allow debugging of assembly code. > > diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c > index 8181f25..d47b4af 100644 > --- a/gdb/arm-tdep.c > +++ b/gdb/arm-tdep.c > @@ -1669,8 +1669,11 @@ arm_analyze_prologue (struct gdbarch *gdbarch, > current_pc < prologue_end; > current_pc += 4) > { > - unsigned int insn > - = read_memory_unsigned_integer (current_pc, 4, byte_order_for_code); > + unsigned int insn; > + gdb_byte buf[4]; > + if (target_read_memory (current_pc, buf, 4)) > + break; > + insn = extract_unsigned_integer (buf, 4, byte_order_for_code); It would be good to have a few more details about what causes us to be in a situation where we'd be failing to read memory at an address. Perhaps just showing the disassembled code and a quick explanation of what happens might be sufficient. Also, for our piece of mind, we normally ask that the change be validated by running the testsuite. Did you do that? If yes, on which platform? Lastly - a small nitpick, due to GDB's Coding style, which requires that an empty line be inserted between local variable declarations and the first statement after that. So, would you mind adding an empty line after "buf"'s declaration? Other than that, I think the patch looks reasonable. I'm just a little unsure about how you can be trying to read an instruction at an address that's unreadable. Looking at the code, and in particular, looking at the callers, I think I might have some idea, but I'd prefer if you showed us the issue that you actually hit. Thank you, -- Joel