gdb/riscv: Write 4-byte nop to dummy code region before inferior calls

When making inferior function calls GDB sets up a dummy code region on
the stack, and places a breakpoint within that region.  If the random
stack contents appear to be a compressed instruction then GDB  will
place a compressed breakpoint, which can cause problems if the target
doesn't support compressed instructions.

This commit prevents this issue by writing a 4-byte nop instruction to
the dummy region at the time the region is allocated.  With this nop
instruction in place, when we come to insert the breakpoint then an
uncompressed breakpoint will be used.

This is similar to other targets, for example mips.

gdb/ChangeLog:

	* riscv-tdep.c (riscv_push_dummy_code): Write a 4-byte nop
	instruction to the dummy code region.

gdb/testsuite/ChangeLog:

	* gdb.arch/riscv-bp-infcall.c: New file.
	* gdb.arch/riscv-bp-infcall.exp: New file.

5 files changed, 129 insertions, 1 deletions

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index c5a102f..fb08d71 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2019-07-19  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* riscv-tdep.c (riscv_push_dummy_code): Write a 4-byte nop
+	instruction to the dummy code region.
+
 2019-07-19  Tom Tromey  <tromey@adacore.com>
 
 	* contrib/ari/gdb_ari.sh: Mention C++11, not ISO C 90.
diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index 7f3a1f6..e4a66f1 100644
--- a/gdb/riscv-tdep.c
+++ b/gdb/riscv-tdep.c
@@ -1621,11 +1621,44 @@ riscv_push_dummy_code (struct gdbarch *gdbarch, CORE_ADDR sp,
 		       struct type *value_type, CORE_ADDR *real_pc,
 		       CORE_ADDR *bp_addr, struct regcache *regcache)
 {
+  /* A nop instruction is 'add x0, x0, 0'.  */
+  static const gdb_byte nop_insn[] = { 0x13, 0x00, 0x00, 0x00 };
+
   /* Allocate space for a breakpoint, and keep the stack correctly
-     aligned.  */
+     aligned.  The space allocated here must be at least big enough to
+     accommodate the NOP_INSN defined above.  */
   sp -= 16;
   *bp_addr = sp;
   *real_pc = funaddr;
+
+  /* When we insert a breakpoint we select whether to use a compressed
+     breakpoint or not based on the existing contents of the memory.
+
+     If the breakpoint is being placed onto the stack as part of setting up
+     for an inferior call from GDB, then the existing stack contents may
+     randomly appear to be a compressed instruction, causing GDB to insert
+     a compressed breakpoint.  If this happens on a target that does not
+     support compressed instructions then this could cause problems.
+
+     To prevent this issue we write an uncompressed nop onto the stack at
+     the location where the breakpoint will be inserted.  In this way we
+     ensure that we always use an uncompressed breakpoint, which should
+     work on all targets.
+
+     We call TARGET_WRITE_MEMORY here so that if the write fails we don't
+     throw an exception.  Instead we ignore the error and move on.  The
+     assumption is that either GDB will error later when actually trying to
+     insert a software breakpoint, or GDB will use hardware breakpoints and
+     there will be no need to write to memory later.  */
+  int status = target_write_memory (*bp_addr, nop_insn, sizeof (nop_insn));
+
+  if (riscv_debug_breakpoints || riscv_debug_infcall)
+    fprintf_unfiltered (gdb_stdlog,
+			"Writing %lld-byte nop instruction to %s: %s\n",
+			((unsigned long long) sizeof (nop_insn)),
+			paddress (gdbarch, *bp_addr),
+			(status == 0 ? "success" : "failed"));
+
   return sp;
 }
 
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 4d7c4a7..5313470 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2019-07-19  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* gdb.arch/riscv-bp-infcall.c: New file.
+	* gdb.arch/riscv-bp-infcall.exp: New file.
+
 2019-07-17  Andrew Burgess  <andrew.burgess@embecosm.com>
 
 	PR breakpoints/24541
diff --git a/gdb/testsuite/gdb.arch/riscv-bp-infcall.c b/gdb/testsuite/gdb.arch/riscv-bp-infcall.c
new file mode 100644
index 0000000..9bd40af
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/riscv-bp-infcall.c
@@ -0,0 +1,29 @@
+/* This file is part of GDB, the GNU debugger.
+
+   Copyright 2019 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/>.  */
+
+void
+dummy_call ()
+{
+  asm ("" ::: "memory");
+}
+
+int
+main ()
+{
+  dummy_call ();
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.arch/riscv-bp-infcall.exp b/gdb/testsuite/gdb.arch/riscv-bp-infcall.exp
new file mode 100644
index 0000000..478032a
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/riscv-bp-infcall.exp
@@ -0,0 +1,56 @@
+# Copyright 2019 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/>.
+
+# Test GDB for RISC-V always uses an uncompressed breakpoint when
+# setting up for an inferior call.
+
+if {![istarget "riscv*-*-*"]} {
+    verbose "Skipping ${gdb_test_file_name}."
+    return
+}
+
+standard_testfile
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
+    return -1
+}
+
+if ![runto_main] then {
+   fail "can't run to main"
+   return 0
+}
+
+# Figure out where the breakpoint will be placed taking account for
+# stack alignment, and allocation of the dummy code area.
+set bp_addr [get_valueof "/x" "\$sp" 0]
+set bp_addr [format 0x%x [expr ($bp_addr & ~0xf) - 0x20]]
+
+# Fill the region we know will be used as the scratch area with the
+# compressed nop instruction.  If GDB fails to overwrite this with an
+# uncompressed nop then a compressed breakpoint will be used in the
+# following inferior call.
+for {set i 0} {$i < 16} {incr i 2} {
+    gdb_test_no_output "set *((unsigned short *) (${bp_addr} + $i))=0x1" \
+	"place compressed nop in scratch area at offset $i"
+}
+
+# Make an inferior call.  GDB should write an uncompressed nop into
+# the scratch area and so force the use of an uncompressed breakpoint,
+# however, if this mechanism fails and GDB uses a compressed
+# breakpoint, and the target doesn't support compressed instructions,
+# then we would expect weird things to happen here.
+gdb_test_no_output "set debug riscv breakpoints 1"
+gdb_test "call dummy_call ()" \
+    ".*Using EBREAK for breakpoint at $bp_addr \\(instruction length 4\\).*"