Message ID | CAFKAgTcgMqVpopxTw8+8N005OSOoNM33L2a8DLJvn7Tpa-BGvQ@mail.gmail.com |
---|---|
State | New |
Headers | show |
// Test source and desired /real output: #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! ... */
ping? is this ok? Or it is not trivial at all, and better do a normal patch? And write it better? I have no inconvenience on doing so, but I didn't want to duplicate patches in the list. El 10/02/2012 10:57, "Alex Barcelo" <abarcelo@ac.upc.edu> escribió: > // Test source and desired /real output: > > #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! > ... > */ >
On 15.02.2012, at 07:55, Alex Barcelo wrote:
> ping? is this ok? Or it is not trivial at all, and better do a normal patch? And write it better?
The patch is fine, but it's not trivial. It's a ppc patch, hence it should've gotten CC'ed to qemu-ppc :). Also, whatever mailer you used to send the patch line wrapped it:
agraf@lychee:/home/agraf/release/qemu> git pw am 140538
git coERROR: patch seems to be corrupt (line wrapped?)
#40: FILE: linux-user/signal.c:4114:
target_sigaction *ka,
total: 1 errors, 0 warnings, 9 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
I'll fix it up and apply it to ppc-next.
Thanks!
Alex
On Wed, Feb 15, 2012 at 09:35, Alexander Graf <agraf@suse.de> wrote: > On 15.02.2012, at 07:55, Alex Barcelo wrote: >> ping? is this ok? Or it is not trivial at all, and better do a normal patch? And write it better? > > The patch is fine, but it's not trivial. It's a ppc patch, hence it should've gotten CC'ed > to qemu-ppc :). Also, whatever mailer you used to send the patch line wrapped it: > ... > > I'll fix it up and apply it to ppc-next. Sorry for the inconvenience, and thanks a lot!
diff --git a/linux-user/signal.c b/linux-user/signal.c index 79a39dc..26e0530 100644 --- a/linux-user/signal.c +++ b/linux-user/signal.c @@ -4115,7 +4115,7 @@ static target_ulong get_sigframe(struct target_sigaction *ka, oldsp = env->gpr[1]; if ((ka->sa_flags & TARGET_SA_ONSTACK) && - (sas_ss_flags(oldsp))) { + (sas_ss_flags(oldsp)) == 0) { oldsp = (target_sigaltstack_used.ss_sp + target_sigaltstack_used.ss_size);
This is v2 of the patch "sas_ss_flags bug for powerpc", which had a horrible name and no description. All architectures work the same way, and all check for sas_ss_flags == 0. The powerpc lines are wrong, and do the check the other way round (it's a qemu internal check, which is done wrong only for this architecture, it's more a typo than a bug). It's NOT ppc specific, it's POSIX standard (sigaltstack) and qemu internal. I have a test source that I will send in a follow-up (it's longer than I would have wished, I'm sure that a better test case can be written if needed) Signed-off-by: Alex Barcelo <abarcelo@ac.upc.edu> --- linux-user/signal.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) } -- 1.7.5.4