From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 46196 invoked by alias); 11 Jun 2015 17:25:16 -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 46178 invoked by uid 89); 11 Jun 2015 17:25:16 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.9 required=5.0 tests=BAYES_00,KAM_LAZY_DOMAIN_SECURITY,RCVD_IN_DNSWL_NONE,T_RP_MATCHES_RCVD autolearn=no version=3.3.2 X-HELO: lwfs1-cam.cam.lispworks.com Received: from mail.lispworks.com (HELO lwfs1-cam.cam.lispworks.com) (46.17.166.21) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 11 Jun 2015 17:25:14 +0000 Received: from higson.cam.lispworks.com (higson.cam.lispworks.com [192.168.1.7]) by lwfs1-cam.cam.lispworks.com (8.14.5/8.14.5) with ESMTP id t5BHP6cQ052518; Thu, 11 Jun 2015 18:25:06 +0100 (BST) (envelope-from martin@lispworks.com) Received: from higson.cam.lispworks.com (localhost.localdomain [127.0.0.1]) by higson.cam.lispworks.com (8.14.4) id t5BHP5ta031297; Thu, 11 Jun 2015 18:25:05 +0100 Received: (from martin@localhost) by higson.cam.lispworks.com (8.14.4/8.14.4/Submit) id t5BHP5LR031293; Thu, 11 Jun 2015 18:25:05 +0100 Date: Thu, 11 Jun 2015 17:25:00 -0000 Message-Id: <201506111725.t5BHP5LR031293@higson.cam.lispworks.com> From: Martin Simmons To: Joel Brobecker CC: gdb-patches@sourceware.org In-reply-to: <20150609184408.GG2855@adacore.com> (message from Joel Brobecker on Tue, 9 Jun 2015 14:44:08 -0400) Subject: Re: [PATCH] Prevent internal-error when computing $pc in ARM assembly code References: <201505200949.t4K9nlqt015737@heapu.cam.lispworks.com> <20150609184408.GG2855@adacore.com> X-SW-Source: 2015-06/txt/msg00226.txt.bz2 >>>>> On Tue, 9 Jun 2015 14:44:08 -0400, Joel Brobecker said: > > 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. Thanks for looking at it. The code being debugged uses r11 (ARM_FP_REGNUM) as a global pointer rather than a frame register, so lines 1968...1984 of arm-tdep.c ("We have no symbol information.") computes bogus values for prologue_start and prologue_end. > 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? Yes, I ran make check-gdb on Raspbian GNU/Linux 7, but it produces around 1200 failures with or without my change, including some non-deterministic ones. > 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? Ah, sorry. Here is the new version. I've also included a new test, which passes for me on Raspbian GNU/Linux 7. gdb/ChangeLog: * arm-tdep.c (arm_analyze_prologue): Read memory without throwing an exception, to allow debugging of assembly code. gdb/testsuite/ChangeLog: * gdb.arch/arm-r11-non-pointer.S: New file. * gdb.arch/arm-r11-non-pointer.exp: New file. diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c index c99f2a9..fd07bca 100644 --- a/gdb/arm-tdep.c +++ b/gdb/arm-tdep.c @@ -1669,8 +1669,12 @@ 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); if (insn == 0xe1a0c00d) /* mov ip, sp */ { diff --git a/gdb/testsuite/gdb.arch/arm-r11-non-pointer.S b/gdb/testsuite/gdb.arch/arm-r11-non-pointer.S new file mode 100644 index 0000000..89884e3 --- /dev/null +++ b/gdb/testsuite/gdb.arch/arm-r11-non-pointer.S @@ -0,0 +1,45 @@ +/* Copyright 2010-2015 Free Software Foundation, Inc. + + This file is part of GDB. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + + .syntax unified + .text + .type main,%function +#if defined (__thumb__) + .code 16 + .thumb_func +#endif + + .align 2 +textdataregion: + .word dataregion + + .globl main +main: + stmfd sp!, {r3, lr} + bl .L0 + movs r0, #0 + ldmfd sp!, {r3, pc} + .size main, .-main + +.L0: ldr r11, textdataregion + mov r11, r11 + mov pc, lr + + .data + .align 2 +dataregion: + .word 64,65,66,67 diff --git a/gdb/testsuite/gdb.arch/arm-r11-non-pointer.exp b/gdb/testsuite/gdb.arch/arm-r11-non-pointer.exp new file mode 100644 index 0000000..1def957 --- /dev/null +++ b/gdb/testsuite/gdb.arch/arm-r11-non-pointer.exp @@ -0,0 +1,69 @@ +# Copyright 2010-2015 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +# This file is part of the gdb testsuite. + +# Test arm non-pointer in r11. + +if {![istarget "arm*-*-*"]} then { + verbose "Skipping arm r11 non-pointer tests." + return +} + +set testfile "arm-r11-non-pointer" +set srcfile ${testfile}.S +set binfile ${objdir}/${subdir}/${testfile} + +set additional_flags "-Wa,-g" + +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable [list debug $additional_flags]] != "" } { + untested arm-r11-non-pointer.exp + return -1 +} + +# Get things started. + +clean_restart ${testfile} + +if ![runto_main] then { + fail "Can't run to main" + return 0 +} + +gdb_test "x/i \$pc" \ + ".*bl.*0x.*" \ + "x/i pc" + +gdb_test "stepi" \ + "\\\?\\\? \\\(\\\) at .*$srcfile:.*" \ + "stepi" + +gdb_test "x/i \$pc" \ + ".*ldr.*r11,.*" \ + "x/i pc" + +gdb_test "stepi" \ + "\\\?\\\? \\\(\\\) at .*$srcfile:.*" \ + "stepi" + +gdb_test "x/i \$pc" \ + ".*mov.*r11,.*r11.*" \ + "x/i pc" + +########################################## + +# Done, run program to exit. + +gdb_continue_to_end "arm-r11-non-pointer"