From: liubo <liubo2009@cn.fujitsu.com>
To: Garrett Cooper <yanegomi@gmail.com>
Cc: ltp-list <ltp-list@lists.sourceforge.net>
Subject: Re: [LTP] [PATCH] syscalls: fix some failure on arch X86_64
Date: Mon, 22 Feb 2010 17:08:55 +0800 [thread overview]
Message-ID: <4B8249A7.3040401@cn.fujitsu.com> (raw)
In-Reply-To: <364299f41002212356g2a4ceb09o3d271e31b388fe51@mail.gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 16388 bytes --]
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 <liubo2009@cn.fujitsu.com> 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 ...
>> - rt_sigsuspend01
>> There exists the same problem as rt_sigaction01.
>>
>> This patch fixed these failure.
>>
>> Signed-off-by: Liu Bo <liubo2009@cn.fujitsu.com>
>>
>> ---
>> 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 <liubo2009@cn.fujitsu.com> */
>> +/* */
>> +/******************************************************************************/
>> +
>> +#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.
Thanks,
Liubo
>
>> 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
>>
>
>
>
[-- Attachment #1.2: Type: text/html, Size: 26366 bytes --]
[-- Attachment #2: Type: text/plain, Size: 345 bytes --]
------------------------------------------------------------------------------
Download Intel® Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
[-- Attachment #3: Type: text/plain, Size: 155 bytes --]
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list
next prev parent reply other threads:[~2010-02-22 9:06 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-02-22 5:21 [LTP] [PATCH] syscalls: fix some failure on arch X86_64 liubo
2010-02-22 7:56 ` Garrett Cooper
2010-02-22 9:08 ` liubo [this message]
2010-02-22 18:05 ` Garrett Cooper
2010-02-23 0:59 ` liubo
2010-02-25 7:26 ` liubo
2010-02-25 10:00 ` Garrett Cooper
2010-02-26 0:35 ` liubo
2010-02-27 4:12 ` Garrett Cooper
-- strict thread matches above, loose matches on Subject: below --
2010-02-22 9:20 liubo
2010-02-22 14:45 ` Rishikesh K Rajak
2009-12-09 7:34 liubo
2009-12-09 12:14 ` Subrata Modak
2009-12-18 16:03 ` Subrata Modak
2009-12-22 2:51 ` Garrett Cooper
2009-12-22 13:12 ` liubo
2009-11-10 9:38 liubo
2009-11-11 1:28 ` liubo
2009-11-11 4:14 ` Garrett Cooper
2009-11-11 4:30 ` Wei Yongjun
2009-11-11 5:22 ` liubo
2009-11-11 4:33 ` Mike Frysinger
2009-11-11 5:03 ` Wei Yongjun
2009-11-16 8:13 ` Subrata Modak
2009-11-16 8:53 ` liubo
2009-11-26 11:11 ` Garrett Cooper
2009-11-27 5:33 ` liubo
2009-11-27 6:49 ` Garrett Cooper
2009-11-27 8:50 ` Garrett Cooper
2009-11-27 10:07 ` liubo
2009-11-27 22:18 ` Garrett Cooper
2009-11-29 1:22 ` Wei Yongjun
2009-12-01 0:00 ` Garrett Cooper
2009-12-09 7:29 ` liubo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4B8249A7.3040401@cn.fujitsu.com \
--to=liubo2009@cn.fujitsu.com \
--cc=ltp-list@lists.sourceforge.net \
--cc=yanegomi@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.