[3/3,nios2] fixes for new implementation of signal handler trampolines
Commit Message
Earlier versions of the nios2 kernel used to allocate code for signal
handler trampolines on the stack, but when the port was accepted
upstream it was changed to instead put the trampoline at a fixed address
in low memory (0x1044).
Moving the code off the stack changed the layout of the stack frame, so
the first part of this fix involves updating the offset to the register
save area. This is not an exported interface from the kernel; I noticed
e.g. the existing aarch64 gdb support includes a huge block of comments
explaining the kernel's signal handler stack frame layout but ultimately
also relies on using magic numbers to access the register save area. I
used a somewhat smaller block of comments for nios2 but I think now it
is clear where the magic numbers come from and what kernel code this
corresponds to.
The second problem is that the trampoline is not writable by user
processes so GDB cannot set software breakpoints there. I've tried to
deal with that in the single-step hook by having it effectively step
over the trampoline by setting the breakpoint on its return address, but
for operations like "finish" or "advance" that use the stack unwinder to
get the location to set the breakpoint, it seems like there is nothing
to do but kfail the tests.
OK to commit?
-Sandra
Comments
On 04/23/2015 12:59 PM, Sandra Loosemore wrote:
> @@ -204,14 +251,8 @@ extern initialize_file_ftype _initialize_nios2_linux_tdep;
> void
> _initialize_nios2_linux_tdep (void)
> {
> -
> - const struct bfd_arch_info *arch_info;
> -
> - for (arch_info = bfd_lookup_arch (bfd_arch_nios2, 0);
> - arch_info != NULL;
> - arch_info = arch_info->next)
> - gdbarch_register_osabi (bfd_arch_nios2, arch_info->mach,
> - GDB_OSABI_LINUX, nios2_linux_init_abi);
> + gdbarch_register_osabi (bfd_arch_nios2, 0, GDB_OSABI_LINUX,
> + nios2_linux_init_abi);
>
> initialize_tdesc_nios2_linux ();
> }
Ooops, please ignore this patch hunk -- I generated this patch against
the wrong base after reverting a change that turned out to be
unnecessary on mainline head.
-Sandra
Sandra Loosemore <sandra@codesourcery.com> writes:
> Earlier versions of the nios2 kernel used to allocate code for signal
> handler trampolines on the stack, but when the port was accepted
> upstream it was changed to instead put the trampoline at a fixed
> address in low memory (0x1044).
>
> Moving the code off the stack changed the layout of the stack frame,
> so the first part of this fix involves updating the offset to the
> register save area. This is not an exported interface from the
> kernel; I noticed e.g. the existing aarch64 gdb support includes a
> huge block of comments explaining the kernel's signal handler stack
> frame layout but ultimately also relies on using magic numbers to
> access the register save area. I used a somewhat smaller block of
> comments for nios2 but I think now it is clear where the magic numbers
> come from and what kernel code this corresponds to.
We can make this magic number less magic by documenting how it is
calculated. We did something similar in
tic6x-linux-tdep.c:tic6x_linux_rt_sigreturn_init,
/* The base of struct sigcontext is computed by examining the definition of
struct rt_sigframe in linux kernel source arch/c6x/kernel/signal.c. */
CORE_ADDR base = (sp + TIC6X_SP_RT_SIGFRAME
/* Pointer type *pinfo and *puc in struct rt_sigframe. */
+ 4 + 4
+ TIC6X_SIGINFO_SIZE
+ 4 + 4 /* uc_flags and *uc_link in struct ucontext. */
+ TIC6X_STACK_T_SIZE);
>
> The second problem is that the trampoline is not writable by user
> processes so GDB cannot set software breakpoints there. I've tried to
> deal with that in the single-step hook by having it effectively step
> over the trampoline by setting the breakpoint on its return address,
> but for operations like "finish" or "advance" that use the stack
> unwinder to get the location to set the breakpoint, it seems like
> there is nothing to do but kfail the tests.
Could you address this in a separated patch?
On 04/28/2015 05:56 AM, Yao Qi wrote:
> Sandra Loosemore <sandra@codesourcery.com> writes:
>
>> Earlier versions of the nios2 kernel used to allocate code for signal
>> handler trampolines on the stack, but when the port was accepted
>> upstream it was changed to instead put the trampoline at a fixed
>> address in low memory (0x1044).
>>
>> Moving the code off the stack changed the layout of the stack frame,
>> so the first part of this fix involves updating the offset to the
>> register save area. This is not an exported interface from the
>> kernel; I noticed e.g. the existing aarch64 gdb support includes a
>> huge block of comments explaining the kernel's signal handler stack
>> frame layout but ultimately also relies on using magic numbers to
>> access the register save area. I used a somewhat smaller block of
>> comments for nios2 but I think now it is clear where the magic numbers
>> come from and what kernel code this corresponds to.
>
> We can make this magic number less magic by documenting how it is
> calculated. We did something similar in
> tic6x-linux-tdep.c:tic6x_linux_rt_sigreturn_init,
>
> /* The base of struct sigcontext is computed by examining the definition of
> struct rt_sigframe in linux kernel source arch/c6x/kernel/signal.c. */
> CORE_ADDR base = (sp + TIC6X_SP_RT_SIGFRAME
> /* Pointer type *pinfo and *puc in struct rt_sigframe. */
> + 4 + 4
> + TIC6X_SIGINFO_SIZE
> + 4 + 4 /* uc_flags and *uc_link in struct ucontext. */
> + TIC6X_STACK_T_SIZE);
Well, ahem, the magic number was actually calculated by inspection of
the stack from the debugger. :-) I got lost trying to calculate the
sizes of the data structures (struct siginfo, etc) from the kernel code
by hand, and what purpose would it serve to have more magic numbers that
are harder to compute than the current one?
>> The second problem is that the trampoline is not writable by user
>> processes so GDB cannot set software breakpoints there. I've tried to
>> deal with that in the single-step hook by having it effectively step
>> over the trampoline by setting the breakpoint on its return address,
>> but for operations like "finish" or "advance" that use the stack
>> unwinder to get the location to set the breakpoint, it seems like
>> there is nothing to do but kfail the tests.
>
> Could you address this in a separated patch?
Yes, I can split the patch.
-Sandra
@@ -1570,7 +1570,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);
@@ -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 */
@@ -29,6 +29,7 @@
#include "linux-tdep.h"
#include "glibc-tdep.h"
#include "nios2-tdep.h"
+#include "breakpoint.h"
#include "features/nios2-linux.c"
@@ -114,7 +115,24 @@ 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, 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++)
@@ -166,6 +185,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
@@ -191,6 +236,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. */
@@ -204,14 +251,8 @@ extern initialize_file_ftype _initialize_nios2_linux_tdep;
void
_initialize_nios2_linux_tdep (void)
{
-
- const struct bfd_arch_info *arch_info;
-
- for (arch_info = bfd_lookup_arch (bfd_arch_nios2, 0);
- arch_info != NULL;
- arch_info = arch_info->next)
- gdbarch_register_osabi (bfd_arch_nios2, arch_info->mach,
- GDB_OSABI_LINUX, nios2_linux_init_abi);
+ gdbarch_register_osabi (bfd_arch_nios2, 0, GDB_OSABI_LINUX,
+ nios2_linux_init_abi);
initialize_tdesc_nios2_linux ();
}
@@ -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} $" {
@@ -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} $" {