signal handling problems (32 bit kernel)

Hi,
Following on from my problems with SEGV last week, I've come up with
a few signal handling issues:

=============================================================

parisc:signal.c

        /* Possibly bogus. */
#warning XXX FIXME probably bogus -PB
        /* I think this is bogus -- it'll cause the first instn of the
         * signal handler to be executed twice!  Better might be to
         * set iaoq[0] to one of the NOPs in the trampoline. -PB
         */
        regs->iaoq[0] = (unsigned long) haddr | 3;
        regs->iaoq[1] = (unsigned long) haddr | 3;

This causes real problems if the signal handler is a dynamic object which has not yet been resolved; in that case the signal handler points at the b,l instr in the following at the end of the .plt:

    2700:       0e 80 10 96     ldw  0(sr0,r20),r22
    2704:       ea c0 c0 00     bv r0(r22)
    2708:       0e 88 10 95     ldw  4(sr0,r20),r21
    270c:       ea 9f 1f dd     b,l 2700 <__DTOR_END__+0x54>,r20
    2710:       d6 80 1c 1e     depwi 0,31,2,r20

typically that results in a misalligned access on the first ldw as r20 is screwed.

I've fixed that by setting iaoq[1] = iaoq[0]+4. Is that OK, or should we use the NOP approach in the comment above?

=============================================================

If a program gets a SEGV, fixes the cause of it in its signal handler, and returns, then the faulting instruction is not rerun. I think that's wrong (differs from m68k anyway).

=============================================================

Set the following program running:

#include <stdio.h>
#include <string.h>
#include <unistd.h>
#include <signal.h>

int v[8];

void (* fp)(int);

void sig_handler(int sig)
{
}

void poke(int i)

{
        v[0] = i; v[1] = i; v[2] = i; v[3] = i;
        v[4] = i; v[5] = i; v[6] = i; v[7] = i;
}

int main()

{
        int j, i = 0;
        struct sigaction act;

        memset(&act, 0, sizeof(act));
        act.sa_handler = sig_handler;
        sigaction(SIGUSR1, &act, NULL);

        fp = sig_handler;
        fp(0);
        printf("I am %d\n", getpid());
        while (1) {
                poke (++i);
                j = v[7];
                if (j != i)
                        printf("Wah!\n");
        }
}

and then in another terminal do 'kill -USR1 <pid>'. The program either goes 'Wah!', gives a SEGV, or works. That seems to be because %r1 is corrupted while processing the signal. The signal handler ends with a syscall (rt_sigreturn_wrapper), and %r1, at least, is not saved and restored over the syscall. %r31 also appears to get corrupted, as it is used in the final branch of the syscall return.

Richard

Definitely bogus, as with quite a lot of iaoq manipulation in signal.c

Sounds like the right thing to do. I reckon everywhere ioaq is fudged from/to r31 should have this sort of mapping. ie. it should be

	regs->gr[31] = regs->iaoq[0];

in sys_rt_sigreturn, and

	err |= __put_user(regs->gr[31], &sc->sc_iaoq[0]);
	err |= __put_user(regs->gr[31] + 4, &sc->sc_iaoq[1]);

in setup_sigcontext, and so on. I'm guessing the origial author of this code didn't know which of iaoq[0] and ioaq[1] was ioaq_front. :)

I don't really understand what is going on here, but it seems wrong to me that setup_rt_frame should be touching regs->iaoq at all when in_syscall.

Hmm, and in that case why all the other gr[31] to/from ioaq[] fudgery?

Alan

Linuxcare.  Support for the Revolution.

As another example, if a process gets a signal as it is about to execute the instr in the delay slot of a branch, it forgets that it was supposed to be branching on return from the signal handler. Try compiling the following and sending it a SIGUSR1:

#include <stdio.h>
#include <string.h>
#include <unistd.h>
#include <signal.h>

void sig_handler(int sig)
{
}

int main()

{
        struct sigaction act;
        int i = 1;

        memset(&act, 0, sizeof(act));
        act.sa_handler = sig_handler;
        sigaction(SIGUSR1, &act, NULL);

        printf("I am %d\n", getpid());
        while (i++)
                ;
        printf("Escaped, i=%d!\n", i);
        return 0;
}

Oh, you have to run it with "LD_BIND_NOW=1 <progname>" to avoid one of the other problems.

Time to try and work out what signal.c is really trying to do, I guess.

Richard

Maybe you should start by considering all the possible states a task can be in when a signal is delivered, in regards to saved registers and stack layout. It would be a _very_ good idea to formalize this once you've sorted it out by splitting up struct pt_regs appropriately. ie. as other architectures do, into struct pt_regs and struct switch_stack. Actually, parisc could go one further and have three structures, one corresponding to registers saved on syscall entry (new pt_regs), one corresponding to macro callee_save (switch_stack), and one corresponding more or less to macro save_specials. Quite a bit of work, but IMO well worth doing.

Alan Modra

Linuxcare.  Support for the Revolution.

Problem is that whenever a signal handler is invoked, it is followed by a sys_rt_sigreturn syscall (invoked via the trampoline code), before returning to original usercode. I can imagine that this might work if the process in question was blocked on a syscall, but not if it was suspended due to an interrupt. In either case the path back to the original user code is the syscall return path, and that obviously cannot put things back as they were if the process was interrupted.

I think the answer is for syscall_do_signal to save enough in the pt_regs such that sys_rt_sigreturn_wrapper can return to userland via an RFI (like intr_restore, but remembering to trace the syscall exit if necessary) regardless of the process state when the signal occurred.

Richard