Re: [Qemu-trivial] [Qemu-devel] [TRIVIAL] sas_ss_flags bug for powerpc

[Alex Barcelo]

On Thu, Feb 9, 2012 at 19:43, Andreas Färber <address@hidden> wrote:
> Am 09.02.2012 19:30, schrieb Alex Barcelo:
>> Signed-off-by: Alex Barcelo <address@hidden>
>
> This patch needs a better description than "bug",

sorry, something like "Incorrect zero comparison in sas_ss_flags"
would have been better. I used my internal git name for the patch
without realizing.

> and you forgot to cc the linux-user maintainer.
new here, I read Contribute/TrivialPatches and think that it wasn't
needed. Sorry about that.

> The patch should describe what it touches
> (linux-user), what it does, what for and make clear why that is correct.
> Is there a particular test case that's broken without the patch?[1]
>
> I can't speak for Stefan, but to me it is totally unclear from looking
> at the patch what sas_ss_flags() does here so this is likely not really
> a trivial one.

Well, is really trivial when compared to the other architectures,
because all do a zero check and this one does it the other way round.
I'm really new here, and I still don't get the workflow and the way to
do things. Will try my best!

Again, sorry for that.

[1] I did a trying-to-be-easy test case, which didn't work before the
patch and worked after the patch. The unsigned int a value should be
independent between the different stacks, but without this patch no
stack change is done so all the functions use the same stack and the
same a variable.

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

void handler(int sig)
{
    unsigned int a;
    // to prevent uninitialized stack, normally a = 0
    if ( a>10 ) a = 0;
    a = a + 1;
    printf ("new value: %d\n" , a );
    if (a > 7) _exit(a);
    return;
}

int main()
{
    int ret;
    char * stackA = malloc(SIGSTKSZ);
    char * stackB = malloc(SIGSTKSZ);
    stack_t ssA = {
        .ss_size = SIGSTKSZ,
        .ss_sp = stackA,
    };
    stack_t ssB = {
        .ss_size = SIGSTKSZ,
        .ss_sp = stackB,
    };
    struct sigaction sa = {
        .sa_handler = handler,
        .sa_flags = SA_ONSTACK
    };

    // no error checking, only debug output
    ret = sigfillset(&sa.sa_mask);
    printf ( "Sigfillset: %d\n" , ret );
    ret = sigaction(SIGUSR1, &sa, 0);
    printf ( "Sigaction: %d\n" , ret );

    while (1) {
        printf ("On stack A -- " );
        ret = sigaltstack(&ssA, 0);
        printf ( "sigaltstack return: %d -- " , ret );
        kill(0, SIGUSR1);
        sleep(1);
        printf ("                                    -- " );
        kill(0, SIGUSR1);
        sleep(1);

        printf ("On stack B -- " );
        ret = sigaltstack(&ssB, 0);
        printf ( "sigaltstack return: %d -- " , ret );
        kill(0, SIGUSR1);
        sleep(1);
    }
}

/* Desired output:
Sigfillset: 0
Sigaction: 0
On stack A -- sigaltstack return: 0 -- new value: 1
                                    -- new value: 2
On stack B -- sigaltstack return: 0 -- new value: 1
On stack A -- sigaltstack return: 0 -- new value: 3
                                    -- new value: 4
On stack B -- sigaltstack return: 0 -- new value: 2
On stack A -- sigaltstack return: 0 -- new value: 5
                                    -- new value: 6
On stack B -- sigaltstack return: 0 -- new value: 3
On stack A -- sigaltstack return: 0 -- new value: 7
                                    -- new value: 8

Output for ppc without patch:
Sigfillset: 0
Sigaction: 0
On stack A -- sigaltstack return: 0 -- new value: 1
                                    -- new value: 2
On stack B -- sigaltstack return: 0 -- new value: 3 // WRONG!!
On stack A -- sigaltstack return: 0 -- new value: 4
                                    -- new value: 5 // WRONG AGAIN!
...
*/