Re: [LTP] [PATCH] syscalls: fix some failure on arch X86_64

[liubo <liu...@cn...>]

Hi, Garrett,
My revised patch has been "Acked/Tested-By: Rishikesh K
Rajak<ris...@li...>"

url: http://marc.info/?l=ltp-list&m=126683033423523&w=2

Thanks,
Liubo

On 02/23/2010 02:05 AM, Garrett Cooper wrote:
> On Mon, Feb 22, 2010 at 1:08 AM, liubo <liu...@cn...> wrote:
>   
>> On 02/22/2010 03:56 PM, Garrett Cooper wrote:
>>
>> By and large this diff looks really good. My comments...
>>
>> On Sun, Feb 21, 2010 at 9:21 PM, liubo <liu...@cn...> wrote:
>>
>>
>> - rt_sigaction01
>> On arch x86_64, if we directly get to call syscall
>> rt_sigaction, there will be "segment fault".
>> 1) One reason is that we must supply the flag of
>> "SA_RESTORER" and the correct pointer to the fuction
>> "restorer", according to the kernel code.
>> 2) The other reason is that default syscall rt_sigaction
>> use kernel "sigaction" structure, which is different
>> with normal "sigaction" structure.
>>
>> So,
>> 1) We manage to find the address of the function
>> "restorer" by using glibc function "sigaction",
>> which might be something tricky. Then we add these
>> arguments to make test run correctly.
>> 2) We also use kernel "sigaction" structure to fit
>> realtime syscall __NR_rt_sigaction.
>>
>> - rt_sigprocmask01
>> First, there exsits the same problem as rt_sigaction01.
>> Second, this testcase uses a unchanged signal number 33,
>> which may diff among different archs and lead to error
>> "unknown signal".
>>
>> So, we use a macro TEST_SIG which refers to SIGRTMIN
>> to replace 33.
>>
>>
>> The problem is that SIGRTMIN is actually signal 32, not signal 33, and
>> here's a problematic comment to ponder over (from bits/signum.h):
>>
>> /* These are the hard limits of the kernel.  These values should not be
>>    used directly at user level.  */
>> #define __SIGRTMIN      32
>> #define __SIGRTMAX      (_NSIG - 1)
>>
>> I think the proper resolution would be to make the value SIGRTMIN+1.
>>
>>
>>
>> Hi, Garrett,
>>
>> In fact, I'm confused about whether #33 signal is particularly needed here,
>> in some of my boxes, SIGRTMIN actually refers #34 signal, that is,
>> #33 is unreachable.
>>
>> IMO, any realtime reachable signal can be used to pass the case here, and
>> I pick SIGRTMIN by define a macro: #define TEST_SIG SIGRTMIN.
>>
>> Of course, the value "SIGRTMIN+1" also pass the case, so I'll pick your
>> value.
>>
>> If you are free, looking forward to your explaination ...
>>     
>
> Several signals between the non-RT and non-reserved RT signals are
> used by [the now defunct] linuxthreads and nptl. See the following
> page which refers to a very old school copy of LTP :]...:
>
> http://www.redhat.com/archives/phil-list/2003-April/msg00049.html
>
>   

ok , got it.
>> - rt_sigsuspend01
>> There exists the same problem as rt_sigaction01.
>>
>> This patch fixed these failure.
>>
>> Signed-off-by: Liu Bo <liu...@cn...>
>>
>> ---
>>  include/rt_signal.h                                |   60
>> ++++++++++++++++++
>>  .../kernel/syscalls/rt_sigaction/rt_sigaction01.c  |   64
>> +++++++++++--------
>>  .../syscalls/rt_sigprocmask/rt_sigprocmask01.c     |   32 +++++++---
>>  .../syscalls/rt_sigsuspend/rt_sigsuspend01.c       |   13 ++++-
>>  4 files changed, 132 insertions(+), 37 deletions(-)
>>  create mode 100644 include/rt_signal.h
>>
>> diff --git a/include/rt_signal.h b/include/rt_signal.h
>> new file mode 100644
>> index 0000000..ea1e7e3
>> --- /dev/null
>> +++ b/include/rt_signal.h
>> @@ -0,0 +1,60 @@
>> +/******************************************************************************/
>> +/*
>>    */
>> +/* Copyright (c) 2009 FUJITSU LIMITED
>>   */
>> +/*
>>    */
>> +/* This program is free software;  you can redistribute it and/or modify
>>    */
>> +/* it under the terms of the GNU General Public License as published by
>>   */
>> +/* the Free Software Foundation; either version 2 of the License, or
>>    */
>> +/* (at your option) any later version.
>>    */
>> +/*
>>    */
>> +/* This program is distributed in the hope that it will be useful,
>>    */
>> +/* but WITHOUT ANY WARRANTY;  without even the implied warranty of
>>    */
>> +/* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
>>    */
>> +/* the GNU General Public License for more details.
>>   */
>> +/*
>>    */
>> +/* You should have received a copy of the GNU General Public License
>>    */
>> +/* along with this program;  if not, write to the Free Software
>>   */
>> +/* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>>    */
>> +/*
>>    */
>> +/* Author: Liu Bo <liu...@cn...>
>>    */
>> +/*
>>    */
>> +/******************************************************************************/
>> +
>> +#ifndef __LTP_SIGNAL_H
>> +#define __LTP_SIGNAL_H
>> +
>> +/* We do not globally define the SA_RESTORER flag so do it here.  */
>> +#define HAVE_SA_RESTORER
>> +#define SA_RESTORER 0x04000000
>> +
>> +struct kernel_sigaction {
>> +       __sighandler_t k_sa_handler;
>> +       unsigned long sa_flags;
>> +       void (*sa_restorer) (void);
>> +       sigset_t sa_mask;
>> +};
>> +
>> +void (*restore_rt) (void);
>> +
>> +void
>> +handler_h(void)
>> +{
>> +       return;
>> +}
>> +
>> +/* initial restore_rt for x86_64 */
>> +void
>> +sig_initial(int sig)
>> +{
>> +       struct sigaction act, oact;
>> +
>> +       act.sa_handler = (void *)handler_h;
>> +       sigemptyset(&act.sa_mask);
>> +       sigaddset(&act.sa_mask, sig);
>> +       /* copy act.sa_restorer to kernel */
>> +       sigaction(sig, &act, &oact);
>> +       /* copy oact.sa_restorer from kernel */
>> +       sigaction(sig, &act, &oact);
>> +       restore_rt = oact.sa_restorer;
>> +}
>> +#endif
>> diff --git a/testcases/kernel/syscalls/rt_sigaction/rt_sigaction01.c
>> b/testcases/kernel/syscalls/rt_sigaction/rt_sigaction01.c
>> index ffc5fa2..d30f204 100644
>> --- a/testcases/kernel/syscalls/rt_sigaction/rt_sigaction01.c
>> +++ b/testcases/kernel/syscalls/rt_sigaction/rt_sigaction01.c
>> @@ -55,6 +55,10 @@
>>  #include "usctest.h"
>>  #include "linux_syscall_numbers.h"
>>
>> +#ifdef __x86_64__
>> +#include "rt_signal.h"
>> +#endif
>> +
>>  /*
>>  * For all but __mips__:
>>  *
>> @@ -101,13 +105,14 @@ int  TST_TOTAL = 1;                   /* total number
>> of tests in this file.   *
>>  /*              On success - Exits calling tst_exit(). With '0' return
>> code.  */
>>  /*
>>    */
>>  /******************************************************************************/
>> -extern void cleanup() {
>> -        /* Remove tmp dir and all files in it */
>> -        TEST_CLEANUP;
>> -        tst_rmdir();
>> +extern void cleanup()
>> +{
>> +       /* Remove tmp dir and all files in it */
>> +       TEST_CLEANUP;
>> +       tst_rmdir();
>>
>> -        /* Exit with appropriate return code. */
>> -        tst_exit();
>> +       /* Exit with appropriate return code. */
>> +       tst_exit();
>>  }
>>
>>  /* Local  Functions */
>> @@ -128,11 +133,12 @@ extern void cleanup() {
>>  /*              On success - returns 0.
>>   */
>>  /*
>>    */
>>  /******************************************************************************/
>> -void setup() {
>> -        /* Capture signals if any */
>> -        /* Create temporary directories */
>> -        TEST_PAUSE;
>> -        tst_tmpdir();
>> +void setup()
>> +{
>> +       /* Capture signals if any */
>> +       /* Create temporary directories */
>> +       TEST_PAUSE;
>> +       tst_tmpdir();
>>  }
>>
>>  int test_flags[] = {SA_RESETHAND|SA_SIGINFO, SA_RESETHAND,
>> SA_RESETHAND|SA_SIGINFO, SA_RESETHAND|SA_SIGINFO, SA_NOMASK};
>> @@ -144,22 +150,23 @@ handler(int sig)
>>         tst_resm(TINFO,"Signal Handler Called with signal number %d\n",sig);
>>         return;
>>  }
>> -
>>  int
>> -set_handler(int sig, int sig_to_mask, int mask_flags)
>> +set_handler(int sig, int mask_flags)
>>  {
>> -        struct sigaction sa, oldaction;
>> -
>> -                sa.sa_sigaction = (void *)handler;
>> -                sa.sa_flags = mask_flags;
>> -                sigemptyset(&sa.sa_mask);
>> -                sigaddset(&sa.sa_mask, sig_to_mask);
>> -                TEST(syscall(__NR_rt_sigaction,sig, &sa,
>> &oldaction,SIGSETSIZE));
>> -        if (TEST_RETURN == 0) {
>> -                return 0;
>> -        } else {
>> -                        return TEST_RETURN;
>> -        }
>> +#ifdef __x86_64__
>> +       struct kernel_sigaction sa, oldaction;
>> +       mask_flags |= SA_RESTORER;
>> +       sa.sa_restorer = restore_rt;
>> +       sa.k_sa_handler = (void *)handler;
>> +#else
>> +       struct sigaction sa, oldaction;
>> +       sa.sa_handler = (void *)handler;
>> +#endif
>> +       sa.sa_flags = mask_flags;
>> +       sigemptyset(&sa.sa_mask);
>> +       sigaddset(&sa.sa_mask, sig);
>> +       TEST(syscall(__NR_rt_sigaction,sig, &sa, &oldaction,SIGSETSIZE));
>> +       return TEST_RETURN;
>>  }
>>
>>
>> @@ -182,8 +189,11 @@ int main(int ac, char **av) {
>>                 for (testno = 0; testno < TST_TOTAL; ++testno) {
>>
>>                        for (signal = SIGRTMIN; signal <= (SIGRTMAX );
>> signal++){//signal for 34 to 65
>> -                               for(flag=0; flag<5;flag++) {
>> -                                        TEST(set_handler(signal, 0,
>> test_flags[flag]));
>> +#ifdef __x86_64__
>> +                               sig_initial(signal);
>> +#endif
>> +                               for(flag=0; flag<5;flag++) {
>> +                                        TEST(set_handler(signal,
>> test_flags[flag]));
>>
>>
>> I know this isn't your code change, but since you're here.. a better
>> way to do this would be:
>>
>> sizeof (test_flags) / sizeof (test_flags[0])
>>
>> as this is a fixed array buffer.
>>
>>
>> Sure, this can increase codes's portability.
>>
>> I'm generating a new version patch.
>>     
>
> The less we have to modify code in the future, the better... Cheers!
>
>   
>>                                                 if (TEST_RETURN == 0) {
>>                                                tst_resm(TINFO,"signal: %d ",
>> signal);
>>                                                tst_resm(TPASS, "rt_sigaction
>> call succeeded: result = %ld ",TEST_RETURN );
>> diff --git a/testcases/kernel/syscalls/rt_sigprocmask/rt_sigprocmask01.c
>> b/testcases/kernel/syscalls/rt_sigprocmask/rt_sigprocmask01.c
>> index 6398a28..8bcdc78 100644
>> --- a/testcases/kernel/syscalls/rt_sigprocmask/rt_sigprocmask01.c
>> +++ b/testcases/kernel/syscalls/rt_sigprocmask/rt_sigprocmask01.c
>> @@ -62,6 +62,12 @@
>>  #include "usctest.h"
>>  #include "linux_syscall_numbers.h"
>>
>> +#ifdef __x86_64__
>> +#include "rt_signal.h"
>> +#endif
>> +
>> +#define TEST_SIG SIGRTMIN
>> +
>>  /* Extern Global Variables */
>>  extern int Tst_count;           /* counter for tst_xxx routines.         */
>>  extern char *TESTDIR;           /* temporary dir created by tst_tmpdir() */
>> @@ -131,7 +137,16 @@ void sig_handler(int sig)
>>  }
>>
>>  int main(int ac, char **av) {
>> -       struct sigaction act, oact;
>> +#ifdef __x86_64__
>> +       struct kernel_sigaction act, oact;
>> +       sig_initial(TEST_SIG);
>> +       act.sa_flags |= SA_RESTORER;
>> +        act.sa_restorer = restore_rt;
>> +        act.k_sa_handler = sig_handler;
>> +#else
>> +        struct sigaction act, oact;
>> +        act.sa_handler = sig_handler;
>> +#endif
>>         sigset_t set, oset;
>>         int lc;                 /* loop counter */
>>         char *msg;              /* message returned from parse_opts */
>> @@ -154,22 +169,21 @@ int main(int ac, char **av) {
>>                                cleanup();
>>                                tst_exit();
>>                        }
>> -                       TEST(sigaddset(&set, 33));
>> +                       TEST(sigaddset(&set, TEST_SIG));
>>                        if(TEST_RETURN == -1){
>>                                tst_resm(TFAIL,"Call to sigaddset() Failed,
>> errno=%d : %s",TEST_ERRNO, strerror(TEST_ERRNO));
>>                                cleanup();
>>                                tst_exit();
>>                        }
>> -
>> +
>>                        /* call rt_sigaction() */
>> -                       act.sa_handler = sig_handler;
>> -                        TEST(syscall(__NR_rt_sigaction, 33, &act, &oact,
>> 8));
>> +                        TEST(syscall(__NR_rt_sigaction, TEST_SIG, &act,
>> &oact, 8));
>>                        if(TEST_RETURN != 0){
>>                                tst_resm(TFAIL,"Call to rt_sigaction()
>> Failed, errno=%d : %s",TEST_ERRNO, strerror(TEST_ERRNO));
>>                                cleanup();
>>                                tst_exit();
>>                        }
>> -                       /* call rt_sigprocmask() to block signal#33 */
>> +                       /* call rt_sigprocmask() to block signal#SIGRTMIN */
>>                         TEST(syscall(__NR_rt_sigprocmask, SIG_BLOCK, &set,
>> &oset, 8));
>>                        if(TEST_RETURN == -1){
>>                                tst_resm(TFAIL,"Call to rt_sigprocmask()****
>> Failed, errno=%d : %s",TEST_ERRNO, strerror(TEST_ERRNO));
>> @@ -178,7 +192,7 @@ int main(int ac, char **av) {
>>                        }
>>
>>                        else {
>> -                               TEST(kill(getpid(), 33));
>> +                               TEST(kill(getpid(), TEST_SIG));
>>                                if(TEST_RETURN != 0){
>>                                        tst_resm(TFAIL,"Call to kill()
>> Failed, errno=%d : %s",TEST_ERRNO, strerror(TEST_ERRNO));
>>                                        cleanup();
>> @@ -198,13 +212,13 @@ int main(int ac, char **av) {
>>                                                cleanup();
>>                                                tst_exit();
>>                                        }
>> -                                       TEST(sigismember(&oset, 33));
>> +                                       TEST(sigismember(&oset, TEST_SIG));
>>                                        if(TEST_RETURN == 0 ){
>>                                                tst_resm(TFAIL,"call
>> sigismember() Failed, errno=%d : %s",TEST_ERRNO, strerror(TEST_ERRNO));
>>                                                cleanup();
>>                                                tst_exit();
>>                                        }
>> -                                       /* call rt_sigprocmask() to unblock
>> signal#33 */
>> +                                       /* call rt_sigprocmask() to unblock
>> signal#SIGRTMIN */
>>                                        TEST(syscall(__NR_rt_sigprocmask,
>> SIG_UNBLOCK, &set, &oset, 8));
>>                                        if(TEST_RETURN == -1){
>>                                                tst_resm(TFAIL,"Call to
>> rt_sigprocmask() Failed, errno=%d : %s",TEST_ERRNO, strerror(TEST_ERRNO));
>> diff --git a/testcases/kernel/syscalls/rt_sigsuspend/rt_sigsuspend01.c
>> b/testcases/kernel/syscalls/rt_sigsuspend/rt_sigsuspend01.c
>> index 416a7c9..84f2967 100644
>> --- a/testcases/kernel/syscalls/rt_sigsuspend/rt_sigsuspend01.c
>> +++ b/testcases/kernel/syscalls/rt_sigsuspend/rt_sigsuspend01.c
>> @@ -47,6 +47,10 @@
>>  #include "usctest.h"
>>  #include "linux_syscall_numbers.h"
>>
>> +#ifdef __x86_64__
>> +#include "rt_signal.h"
>> +#endif
>> +
>>  /* Extern Global Variables */
>>  extern int Tst_count;           /* counter for tst_xxx routines.         */
>>  extern char *TESTDIR;           /* temporary dir created by tst_tmpdir() */
>> @@ -139,9 +143,16 @@ int main(int ac, char **av) {
>>                                cleanup();
>>                                tst_exit();
>>                        }
>> +#ifdef __x86_64__
>> +                       struct kernel_sigaction act, oact;
>> +                       sig_initial(SIGALRM);
>> +                       act.sa_flags |= SA_RESTORER;
>> +                       act.sa_restorer = restore_rt;
>> +                       act.k_sa_handler = sig_handler;
>> +#else
>>                        struct sigaction act, oact;
>>                        act.sa_handler = sig_handler;
>> -
>> +#endif
>>                        TEST(syscall(__NR_rt_sigaction, SIGALRM, &act, &oact,
>> 8));
>>                        if(TEST_RETURN == -1){
>>                                tst_resm(TFAIL,"rt_sigaction() Failed,
>> errno=%d : %s",TEST_ERRNO, strerror(TEST_ERRNO));
>> --
>> 1.6.2.2
>>
>>
>>
>>
>>     
>
>
>