[patch v2 0/5, nios2] unbreak nios2-linux-gnu GDB

[patch v2 0/5, nios2] unbreak nios2-linux-gnu GDB

[Sandra Loosemore]

This is a revised version of the patch series originally posted here:

https://sourceware.org/ml/gdb-patches/2015-04/msg00904.html

There are now 5 parts (splitting the original part 3 into 2 patches and 
adding a new part 5):

(1) revert to using "trap 31" for breakpoints
(2) use PTRACE_GETREGSET/SETREGSET in gdbserver
(3) fix register save offset for signal handler trampolines
(4) work around issues with unwritable signal handler trampoline code
(5) add NEWS entry for ABI change

-Sandra

[patch v2 1/5, nios2] revert to using "trap 31" for breakpoints

[Sandra Loosemore]

[-- Attachment #1: Type: text/plain, Size: 199 bytes --]

This is a revised version of the patch previously posted at

https://sourceware.org/ml/gdb-patches/2015-04/msg00905.html

I've made the requested change to the comment text.  OK to commit?

-Sandra


[-- Attachment #2: gdb-breakpoint.log --]
[-- Type: text/x-log, Size: 180 bytes --]

2015-04-29  Sandra Loosemore  <sandra@codesourcery.com>

	gdb/
	* nios2-tdep.c (nios2_breakpoint_from_pc): Revert to using
	"trap 31" as the breakpoint instruction on all targets.

[-- Attachment #3: gdb-breakpoint.patch --]
[-- Type: text/x-patch, Size: 1722 bytes --]

diff --git a/gdb/nios2-tdep.c b/gdb/nios2-tdep.c
index 08f2034..988b9fc 100644
--- a/gdb/nios2-tdep.c
+++ b/gdb/nios2-tdep.c
@@ -1189,7 +1189,15 @@ nios2_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR start_pc)
   return nios2_analyze_prologue (gdbarch, start_pc, start_pc, &cache, NULL);
 }
 
-/* Implement the breakpoint_from_pc gdbarch hook.  */
+/* Implement the breakpoint_from_pc gdbarch hook.
+
+   The Nios II ABI for Linux says: "Userspace programs should not use
+   the break instruction and userspace debuggers should not insert
+   one." and "Userspace breakpoints are accomplished using the trap
+   instruction with immediate operand 31 (all ones)."
+
+   So, we use "trap 31" consistently as the breakpoint on bare-metal
+   as well as Linux targets.  */
 
 static const gdb_byte*
 nios2_breakpoint_from_pc (struct gdbarch *gdbarch, CORE_ADDR *bp_addr,
@@ -1198,11 +1206,11 @@ nios2_breakpoint_from_pc (struct gdbarch *gdbarch, CORE_ADDR *bp_addr,
   enum bfd_endian byte_order_for_code = gdbarch_byte_order_for_code (gdbarch);
   unsigned long mach = gdbarch_bfd_arch_info (gdbarch)->mach;
 
-  /* R1 break encoding:
-     ((0x1e << 17) | (0x34 << 11) | (0x1f << 6) | (0x3a << 0))
-     0x003da7fa */
-  static const gdb_byte r1_breakpoint_le[] = {0xfa, 0xa7, 0x3d, 0x0};
-  static const gdb_byte r1_breakpoint_be[] = {0x0, 0x3d, 0xa7, 0xfa};
+  /* R1 trap encoding:
+     ((0x1d << 17) | (0x2d << 11) | (0x1f << 6) | (0x3a << 0))
+     0x003b6ffa */
+  static const gdb_byte r1_breakpoint_le[] = {0xfa, 0x6f, 0x3b, 0x0};
+  static const gdb_byte r1_breakpoint_be[] = {0x0, 0x3b, 0x6f, 0xfa};
   *bp_size = NIOS2_OPCODE_SIZE;
   if (byte_order_for_code == BFD_ENDIAN_BIG)
     return r1_breakpoint_be;

Re: [patch v2 1/5, nios2] revert to using "trap 31" for breakpoints

[Yao Qi]

Sandra Loosemore <sandra@codesourcery.com> writes:

> This is a revised version of the patch previously posted at
>
> https://sourceware.org/ml/gdb-patches/2015-04/msg00905.html
>
> I've made the requested change to the comment text.  OK to commit?

Yes, it is OK.

-- 
Yao (齐尧)

[patch v2 2/5, nios2] use PTRACE_GETREGSET/SETREGSET in gdbserver

[Sandra Loosemore]

[-- Attachment #1: Type: text/plain, Size: 205 bytes --]

This patch is unchanged since the original version posted here:

https://sourceware.org/ml/gdb-patches/2015-04/msg00906.html

but I made the requested change to the ChangeLog.  OK to commit now?

-Sandra


[-- Attachment #2: gdb-regset.log --]
[-- Type: text/x-log, Size: 284 bytes --]

2015-04-29  Sandra Loosemore  <sandra@codesourcery.com>

	gdb/gdbserver/
	* linux-nios2-low.c: Include elf/common.h.  Adjust comments.
	Remove HAVE_PTRACE_GETREGS conditionals.
	(nios2_regsets): Use PTRACE_GETREGSET and PTRACE_SETREGSET
	instead of PTRACE_GETREGS and PTRACE_SETREGS.

[-- Attachment #3: gdb-regset.patch --]
[-- Type: text/x-patch, Size: 1474 bytes --]

diff --git a/gdb/gdbserver/linux-nios2-low.c b/gdb/gdbserver/linux-nios2-low.c
index e2fbb89..7bd3c97 100644
--- a/gdb/gdbserver/linux-nios2-low.c
+++ b/gdb/gdbserver/linux-nios2-low.c
@@ -21,6 +21,7 @@
 
 #include "server.h"
 #include "linux-low.h"
+#include "elf/common.h"
 #include <sys/ptrace.h>
 #include <endian.h>
 #include "gdb_proc_service.h"
@@ -32,7 +33,7 @@
 
 /* The following definition must agree with the number of registers
    defined in "struct user_regs" in GLIBC
-   (ports/sysdeps/unix/sysv/linux/nios2/sys/user.h), and also with
+   (sysdeps/unix/sysv/linux/nios2/sys/user.h), and also with
    NIOS2_NUM_REGS in GDB proper.  */
 
 #define nios2_num_regs 49
@@ -163,8 +164,6 @@ ps_get_thread_area (const struct ps_prochandle *ph,
   return PS_OK;
 }
 
-#ifdef HAVE_PTRACE_GETREGS
-
 /* Helper functions to collect/supply a single register REGNO.  */
 
 static void
@@ -205,14 +204,12 @@ nios2_store_gregset (struct regcache *regcache, const void *buf)
   for (i = 0; i < nios2_num_regs; i++)
     nios2_supply_register (regcache, i, regset + i);
 }
-#endif /* HAVE_PTRACE_GETREGS */
 
 static struct regset_info nios2_regsets[] =
 {
-#ifdef HAVE_PTRACE_GETREGS
-  { PTRACE_GETREGS, PTRACE_SETREGS, 0, nios2_num_regs * 4, GENERAL_REGS,
+  { PTRACE_GETREGSET, PTRACE_SETREGSET, NT_PRSTATUS,
+    nios2_num_regs * 4, GENERAL_REGS,
     nios2_fill_gregset, nios2_store_gregset },
-#endif /* HAVE_PTRACE_GETREGS */
   { 0, 0, 0, -1, -1, NULL, NULL }
 };
 

Re: [patch v2 2/5, nios2] use PTRACE_GETREGSET/SETREGSET in gdbserver

[Yao Qi]

Sandra Loosemore <sandra@codesourcery.com> writes:

> This patch is unchanged since the original version posted here:
>
> https://sourceware.org/ml/gdb-patches/2015-04/msg00906.html
>
> but I made the requested change to the ChangeLog.  OK to commit now?

Yes, it is OK.

-- 
Yao (齐尧)

[patch v2 3/5, nios2] fix register save offset for signal handler trampolines

[Sandra Loosemore]

[-- Attachment #1: Type: text/plain, Size: 297 bytes --]

It was requested that I split the patch previously posted here:

https://sourceware.org/ml/gdb-patches/2015-04/msg00907.html

into two parts.  This is the first part.  I also expanded the comments 
to indicate how the register save offset magic number was calculated.

OK to commit now?

-Sandra


[-- Attachment #2: gdb-sigtramp-offset.log --]
[-- Type: text/x-log, Size: 244 bytes --]

2015-04-29  Sandra Loosemore  <sandra@codesourcery.com>

	gdb/
	* nios2-linux-tdep.c (NIOS2_SIGRETURN_TRAMP_ADDR): Define.
	(NIOS2_SIGRETURN_REGSAVE_OFFSET): Define.
	(nios2_linux_rt_sigreturn_init): Adjust base address of
	register save area.

[-- Attachment #3: gdb-sigtramp-offset.patch --]
[-- Type: text/x-patch, Size: 1743 bytes --]

diff --git a/gdb/nios2-linux-tdep.c b/gdb/nios2-linux-tdep.c
index b829569..0a837b0 100644
--- a/gdb/nios2-linux-tdep.c
+++ b/gdb/nios2-linux-tdep.c
@@ -114,7 +114,25 @@ nios2_iterate_over_regset_sections (struct gdbarch *gdbarch,
 }
 
 /* Initialize a trad-frame cache corresponding to the tramp-frame.
-   FUNC is the address of the instruction TRAMP[0] in memory.  */
+   FUNC is the address of the instruction TRAMP[0] in memory.
+
+   This ABI is not documented.  It corresponds to rt_setup_ucontext in
+   the kernel arch/nios2/kernel/signal.c file.
+
+   The key points are:
+   - The kernel creates a trampoline at the hard-wired address 0x1044.
+   - The stack pointer points to an object of type struct rt_sigframe.
+     The definition of this structure is not exported from the kernel.
+     The register save area is located at offset 152 bytes (as determined
+     by inspection of the stack contents in the debugger), and the
+     registers are saved as r1-r23, ra, fp, gp, ea, sp.
+
+   This interface was implemented with kernel version 3.19 (the first
+   official mainline kernel).  Older unofficial kernel versions used
+   incompatible conventions; we do not support those here.  */
+
+#define NIOS2_SIGRETURN_TRAMP_ADDR 0x1044
+#define NIOS2_SIGRETURN_REGSAVE_OFFSET 152
 
 static void
 nios2_linux_rt_sigreturn_init (const struct tramp_frame *self,
@@ -122,7 +140,8 @@ nios2_linux_rt_sigreturn_init (const struct tramp_frame *self,
 			       struct trad_frame_cache *this_cache,
 			       CORE_ADDR func)
 {
-  CORE_ADDR base = func + 41 * 4;
+  CORE_ADDR sp = get_frame_register_unsigned (next_frame, NIOS2_SP_REGNUM);
+  CORE_ADDR base = sp + NIOS2_SIGRETURN_REGSAVE_OFFSET;
   int i;
 
   for (i = 0; i < 23; i++)

Re: [patch v2 3/5, nios2] fix register save offset for signal handler trampolines

[Yao Qi]

Sandra Loosemore <sandra@codesourcery.com> writes:

> It was requested that I split the patch previously posted here:
>
> https://sourceware.org/ml/gdb-patches/2015-04/msg00907.html
>
> into two parts.  This is the first part.  I also expanded the comments
> to indicate how the register save offset magic number was calculated.

Hi Sandra,
The reason I suggest to describe how this magic number is calculated in
this mail https://sourceware.org/ml/gdb-patches/2015-04/msg01038.html is
to make the code more maintainable.  Supposing some one changes the
offset in linux kernel in the future, we'll quickly know how to update
the magic number.  However, I don't insist on this...

>
> OK to commit now?

Yes, it is OK.

-- 
Yao (齐尧)

[patch v2 4/5, nios2] work around issues with unwritable signal handler trampoline code

[Sandra Loosemore]

[-- Attachment #1: Type: text/plain, Size: 367 bytes --]

It was requested that I split the patch previously posted here:

https://sourceware.org/ml/gdb-patches/2015-04/msg00907.html

into two parts.  This is the second part.  There are no changes from v1 
other than separating it from the part of the original patch that dealt 
with adjusting the register save area offset for the trampoline.  OK to 
commit now?

-Sandra


[-- Attachment #2: gdb-sigtramp-breakpoint.log --]
[-- Type: text/x-log, Size: 430 bytes --]

2015-04-29  Sandra Loosemore  <sandra@codesourcery.com>

	gdb/
	* nios2-tdep.c (nios2_get_next_pc): Make non-static.
	* nios2-tdep.h (nios2_get_next_pc): Declare as extern.
	* nios2-linux-tdep.c: Include breakpoint.h.
	(nios2_linux_software_single_step): New target hook function.
	(nios2_linux_init_abi): Install it.

	gdb/testsuite/
	* gdb.base/sigaltstack.exp: Add kfails for nios2-*-linux*.
	* gdb.base/sigstep.exp: Likewise.

[-- Attachment #3: gdb-sigtramp-breakpoint.patch --]
[-- Type: text/x-patch, Size: 4477 bytes --]

diff --git a/gdb/nios2-linux-tdep.c b/gdb/nios2-linux-tdep.c
index 0a837b0..c73d8af 100644
--- a/gdb/nios2-linux-tdep.c
+++ b/gdb/nios2-linux-tdep.c
@@ -29,6 +29,7 @@
 #include "linux-tdep.h"
 #include "glibc-tdep.h"
 #include "nios2-tdep.h"
+#include "breakpoint.h"
 
 #include "features/nios2-linux.c"
 
@@ -185,6 +186,32 @@ nios2_linux_syscall_next_pc (struct frame_info *frame)
   return pc + NIOS2_OPCODE_SIZE;
 }
 
+/* Override the bare-metal software_single_step gdbarch method.
+   If the PC where we'd ordinarily want to set the breakpoint is
+   the signal trampoline at 0x1044, the kernel will not let us set a
+   breakpoint at that location.  So, treat it as if it were a function call
+   and set the breakpoint on its return address instead.  */
+
+static int
+nios2_linux_software_single_step (struct frame_info *frame)
+{
+  struct gdbarch *gdbarch = get_frame_arch (frame);
+  struct address_space *aspace;
+  CORE_ADDR next_pc = nios2_get_next_pc (frame, get_frame_pc (frame));
+
+  if (next_pc == NIOS2_SIGRETURN_TRAMP_ADDR)
+    {
+      frame = get_prev_frame (frame);
+      next_pc = frame_unwind_caller_pc (frame);
+    }
+
+  aspace = get_frame_address_space (frame);
+  insert_single_step_breakpoint (gdbarch, aspace, next_pc);
+
+  return 1;
+}
+
+
 /* Hook function for gdbarch_register_osabi.  */
 
 static void
@@ -210,6 +237,8 @@ nios2_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   tramp_frame_prepend_unwinder (gdbarch,
                                 &nios2_linux_rt_sigreturn_tramp_frame);
 
+  /* Single stepping.  */
+  set_gdbarch_software_single_step (gdbarch, nios2_linux_software_single_step);
   tdep->syscall_next_pc = nios2_linux_syscall_next_pc;
 
   /* Index of target address word in glibc jmp_buf.  */
diff --git a/gdb/nios2-tdep.c b/gdb/nios2-tdep.c
index 988b9fc..4d9c415 100644
--- a/gdb/nios2-tdep.c
+++ b/gdb/nios2-tdep.c
@@ -1576,7 +1576,7 @@ static const struct frame_unwind nios2_stub_frame_unwind =
 /* Determine where to set a single step breakpoint while considering
    branch prediction.  */
 
-static CORE_ADDR
+CORE_ADDR
 nios2_get_next_pc (struct frame_info *frame, CORE_ADDR pc)
 {
   struct gdbarch *gdbarch = get_frame_arch (frame);
diff --git a/gdb/nios2-tdep.h b/gdb/nios2-tdep.h
index af36c41..46eb3e2 100644
--- a/gdb/nios2-tdep.h
+++ b/gdb/nios2-tdep.h
@@ -77,4 +77,6 @@ struct gdbarch_tdep
 extern struct target_desc *tdesc_nios2_linux;
 extern struct target_desc *tdesc_nios2;
 
+extern CORE_ADDR nios2_get_next_pc (struct frame_info *, CORE_ADDR);
+
 #endif /* NIOS2_TDEP_H */
diff --git a/gdb/testsuite/gdb.base/sigaltstack.exp b/gdb/testsuite/gdb.base/sigaltstack.exp
index b65ea48..59a6c17 100644
--- a/gdb/testsuite/gdb.base/sigaltstack.exp
+++ b/gdb/testsuite/gdb.base/sigaltstack.exp
@@ -76,6 +76,7 @@ proc finish_test { pattern msg } {
 	    # don't gracefully fall back to single-stepping.
 	    setup_kfail gdb/8841 "i?86-*-linux*"
 	    setup_kfail gdb/8841 "*-*-openbsd*"
+	    setup_kfail gdb/8841 "nios2-*-linux*"
 	    fail "$msg (could not set breakpoint)"
 	}
 	-re "$pattern.*${gdb_prompt} $" {
diff --git a/gdb/testsuite/gdb.base/sigstep.exp b/gdb/testsuite/gdb.base/sigstep.exp
index 3c9454c..800fa08 100644
--- a/gdb/testsuite/gdb.base/sigstep.exp
+++ b/gdb/testsuite/gdb.base/sigstep.exp
@@ -161,6 +161,14 @@ set in_handler_map {
 		fail "$test (spurious SIGTRAP)"
 		return
 	    }
+	    -re "Cannot insert breakpoint 0.*${gdb_prompt} $" {
+		# Some platforms use a special read-only page for signal
+		# trampolines.  We can't set a breakpoint there, and we
+		# don't gracefully fall back to single-stepping.
+		setup_kfail gdb/8841 "nios2-*-linux*"
+		fail "$test (could not set breakpoint)"
+		return
+	    }
 	    -re "other handler location.*$gdb_prompt $" {
 		pass $test
 	    }
@@ -203,6 +211,7 @@ proc advancei { cmd } {
 		# don't gracefully fall back to single-stepping.
 		setup_kfail gdb/8841 "i?86-*-linux*"
 		setup_kfail gdb/8841 "*-*-openbsd*"
+		setup_kfail gdb/8841 "nios2-*-linux*"
 		fail "$test (could not set breakpoint)"
 		return
 	    }
@@ -225,6 +234,10 @@ proc advancei { cmd } {
 		pass "$test"
 	    }
 	    -re "main .*${gdb_prompt} $" {
+	    # Some targets cannot set single-step breakpoints on a
+	    # signal trampoline and step over the trampoline
+	    # instead of through it.
+	    setup_kfail gdb/8841 "nios2-*-linux*"
 		fail "$test (in main)"
 	    }
 	    -re "$inferior_exited_re normally.*${gdb_prompt} $" {

Re: [patch v2 4/5, nios2] work around issues with unwritable signal handler trampoline code

[Yao Qi]

Sandra Loosemore <sandra@codesourcery.com> writes:

> It was requested that I split the patch previously posted here:
>
> https://sourceware.org/ml/gdb-patches/2015-04/msg00907.html
>
> into two parts.  This is the second part.  There are no changes from
> v1 other than separating it from the part of the original patch that
> dealt with adjusting the register save area offset for the trampoline.

Hi Sandra,
I am inclined to fix this problem in a target-independent way.  There
was a discussion here https://sourceware.org/ml/gdb/2004-04/msg00089.html
and two approaches were discussed:

 1. insert the momentary breakpoint right to the place where signal was
 raised, to cross the signal handler trampoline,
 2. keep single stepping until it goes out the function or goes the
 right location.

These two are target-independent, and I don't have special preference on
them.

> OK to commit now?

Sorry, I am afraid this patch can't go in, but your first three patches
can go in independently.

-- 
Yao (齐尧)

[patch v2 5/5, nios2] add NEWS entry for ABI change

[Sandra Loosemore]

[-- Attachment #1: Type: text/plain, Size: 136 bytes --]

This patch is a new addition to the series since v1.  It adds a NEWS 
entry about the incompatible ABI change.  OK to commit?

-Sandra


[-- Attachment #2: gdb-news.log --]
[-- Type: text/x-log, Size: 118 bytes --]

2015-04-29  Sandra Loosemore  <sandra@codesourcery.com>

	gdb/
	* NEWS: Add bullet for Nios II GNU/Linux ABI changes.

[-- Attachment #3: gdb-news.patch --]
[-- Type: text/x-patch, Size: 470 bytes --]

diff --git a/gdb/NEWS b/gdb/NEWS
index d463b52..b371e10 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -46,6 +46,10 @@
 
 * GDB now supports the vector ABI on S/390 GNU/Linux targets.
 
+* On Nios II GNU/Linux targets, GDB now supports the generic syscall ABI
+  implemented beginning with kernel version 3.19 and glibc version 2.21.
+  Older versions are incompatible and no longer supported.
+
 * Python Scripting
 
   ** gdb.Objfile objects have a new attribute "username",