Re: [PATCH] Prevent internal-error when computing $pc in ARM assembly code

[Martin Simmons <martin@lispworks.com>]

>>>>> On Fri, 12 Jun 2015 09:21:13 -0400, Joel Brobecker said:
> 
> Looks good. A few little things in your new testcase...
> 
> > +++ b/gdb/testsuite/gdb.arch/arm-r11-non-pointer.S
> > @@ -0,0 +1,45 @@
> > +/* Copyright 2010-2015 Free Software Foundation, Inc.
> 
> Did you really mean for the date range to be 2010-2015?
> If yes, then great. If not, can you adjust it?

I started with the code from another testcase, but it has pretty much all been
rewriten now so I've adjusted it to be 2015.

> > +set testfile "arm-r11-non-pointer"
> > +set srcfile ${testfile}.S
> > +set binfile ${objdir}/${subdir}/${testfile}
> 
> Can you use "standard_testfile .S" instead of the 3 commands above?
> 
> > +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}
> 
> Can you use prepare_for_testing instead of gdb_compile and
> clean_restart?
> 
> > +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"
> > +
> 
> We require that all test "labels" be unique. Would you mind ammending
> them to make them so. Eg: replace the first "x/i pc" by "x/i pc at
> such and such instruction", etc. Same for the "stepi" ones.

OK, all done in the new version below.

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..40729fb
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/arm-r11-non-pointer.S
@@ -0,0 +1,45 @@
+/* Copyright 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 <http://www.gnu.org/licenses/>.  */
+
+	.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..d633a78
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/arm-r11-non-pointer.exp
@@ -0,0 +1,60 @@
+# Copyright 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 <http://www.gnu.org/licenses/>.
+
+# 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
+}
+
+standard_testfile .S
+
+if { [prepare_for_testing ${testfile}.exp ${testfile} $srcfile [list debug "additional_flags=-Wa,-g"]] } {
+    return -1
+}
+
+if ![runto_main] then {
+    fail "Can't run to main"
+    return 0
+}
+
+gdb_test "x/i \$pc" \
+    ".*bl.*0x.*" \
+    "x/i pc at bl"
+
+gdb_test "stepi" \
+    "\\\?\\\? \\\(\\\) at .*$srcfile:.*" \
+    "stepi from bl to ldr"
+
+gdb_test "x/i \$pc" \
+    ".*ldr.*r11,.*" \
+    "x/i pc at ldr"
+
+gdb_test "stepi" \
+    "\\\?\\\? \\\(\\\) at .*$srcfile:.*" \
+    "stepi from ldr to mov"
+
+gdb_test "x/i \$pc" \
+    ".*mov.*r11,.*r11.*" \
+    "x/i pc at mov"
+
+##########################################
+
+# Done, run program to exit.
+
+gdb_continue_to_end "arm-r11-non-pointer"