Re: [patch 3/3, nios2] fixes for new implementation of signal handler trampolines

[Sandra Loosemore <sandra@codesourcery.com>]

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