[linuxsh-dev] More on Signal/Restart and pt_regs

[Jeremy S. <js...@mv...>]

NIIBE Yutaka wrote:

> Jeremy Siegel wrote:
>  > However: REG_SYSCALL (and its successors) were not updated; I think this
>  > implies that using them as arguments to sys_ptrace() (e.g. 4*REG_FPUL) would
>  > yield unexpected results.  And from what I see in some of the GDB and
>  > GLIBC versions we're working with, there are defines for these values in
>  > other files which would need to be updated independently.
>
> Err...  Yup.
>
>  > Unless someone's already submitted such updates, I suggest we just put back a
>  > field at the end of pt_regs, even if it's empty; only its presence is
>  > required so ptrace() users like gdb have a view of "user" space consistent
>  > with the kernel.  [Actually, I'd like to fill it in with info like the old
>  > syscall_nr, i.e. system call and argcount; then "get_wchan()" might be able
>  > to return something more useful, and strace could consistency-check the
>  > argument count if desired.]
>
> How about following:
>         Put back the field for 2.4 kernel with empty value.
>
>         2.5 has same issues for clobbered "r0",
>          porting the fix, remove the field for userspace too.
>
> That is, no userspace change for 2.4, but for 2.5.
>
> Please go ahead for 2.4 changes.  I'll forward-port it to 2.5.

Ok -- I'll put the field back just as a placeholder so glibc/gdb/strace etc. can be
consistent.

Ran into something else w/strace.  Since r0 (ret-code) is saved on the stack, and
r8/r9 set up at syscall_ret() and syscall_ret_trace(), the new sys_sigreturn()
forces "not from syscall" in r8 and returns directly to ret_from_syscall().  This
means that when a signal to a PTRACEd process calls sys_sigreturn() after the
signal, you get a syscall_trace() call on entry to sys_sigreturn(), but not on
exit.  [Same with sys_rt_sigreturn(), of course.]  Turns out this confuses strace
(in my version, at least) since only certain calls (e.g. exit(), fork()) are
expected to be unbalanced.

Copying the MIPS solution seemed to work, i.e. adding:

    if (current->ptrace & PT_TRACESYS)
        syscall_trace();

before return of sys_sigreturn() and sys_rt_sigreturn() seemed to do the trick; but
I thought it might be preferable to simply move the r8/r9 setting from
syscall_ret() and syscall_ret_trace() to right before the system call.  Since those
regs will be saved and restored by the sys_xxx() function prologue and epilogue,
they'll still be valid on return to syscall_ret*, and sys_sigreturn doesn't have to
modify them or skip any code.  (Of course, it then has to return the correct value,
as before, since r0 will be saved to the stack.)

The attached patch adds the empty field, and changes the sys_sig*return() setup as
described.  Any objections to commiting this?  Preference for the C code above
instead, or any other issues?  (Admittedly this has been only minimally tested,
strace w/signals and a select() restart.)

I also have an unrelated question: in the x86 code there's a cli at
ret_from_syscall(), followed by an sti() at signal_return() (before calling
do_signal); in SH we have the effective CLI, but we don't have the STI, so
do_signal() is called with interrupts disabled.  I doubt this is a big deal, but
I'm wondering if there's any particular reason for the difference?

Thanks!
  --Jeremy