|
From: Bart V. A. <bva...@so...> - 2021-02-20 22:57:25
|
https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=719d23b7a9c3b02411dee7d5d4d3744938bb768c commit 719d23b7a9c3b02411dee7d5d4d3744938bb768c Author: Bart Van Assche <bva...@ac...> Date: Sat Feb 20 14:48:58 2021 -0800 drd/tests/swapcontext: Improve portability Diff: --- drd/tests/swapcontext.c | 28 +++++----------------------- 1 file changed, 5 insertions(+), 23 deletions(-) diff --git a/drd/tests/swapcontext.c b/drd/tests/swapcontext.c index f3944d1ba6..e692559b06 100644 --- a/drd/tests/swapcontext.c +++ b/drd/tests/swapcontext.c @@ -16,7 +16,6 @@ typedef struct thread_local { ucontext_t uc[3]; size_t nrsw; - int tfd; } thread_local_t; static void f(void *data, int n) @@ -26,15 +25,11 @@ static void f(void *data, int n) thread_local_t *tlocal = data; while (1) { - memset(&pfd, 0, sizeof(pfd)); - pfd.fd = tlocal->tfd; - pfd.events = POLLIN; - - if (poll(&pfd, 1, 0) == 1) { - if (++tlocal->nrsw == NR_SWITCHES) - return; - swapcontext(&tlocal->uc[n], &tlocal->uc[3 - n]); - } + struct timespec delay = { .tv_nsec = 1000 }; + nanosleep(&delay, NULL); + if (++tlocal->nrsw == NR_SWITCHES) + return; + swapcontext(&tlocal->uc[n], &tlocal->uc[3 - n]); } } @@ -62,19 +57,6 @@ void *worker(void *data) __valgrind_register_current_stack(); - tlocal->tfd = timerfd_create(CLOCK_REALTIME, 0); - if (tlocal->tfd < 0) - abort(); - - it.it_interval.tv_sec = 0; - it.it_interval.tv_nsec = 1000; - - it.it_value.tv_sec = time(NULL); - it.it_value.tv_nsec = 1000; - - if (timerfd_settime(tlocal->tfd, TFD_TIMER_ABSTIME, &it, NULL) < 0) - abort(); - if (getcontext(&(tlocal->uc[1])) < 0) abort(); if (getcontext(&(tlocal->uc[2])) < 0) |
|
From: Paul F. <pj...@wa...> - 2021-02-22 10:36:21
|
On 2/20/21 11:57 PM, Bart Van Assche wrote: > https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=719d23b7a9c3b02411dee7d5d4d3744938bb768c > > commit 719d23b7a9c3b02411dee7d5d4d3744938bb768c > Author: Bart Van Assche <bva...@ac...> > Date: Sat Feb 20 14:48:58 2021 -0800 > > drd/tests/swapcontext: Improve portability Hi This testcase is now closer to building, but still not quite there. The problem is now with 'pthread_getattr_np', which, as the manpage says CONFORMING TO This function is a nonstandard GNU extension; hence the suffix "_np" (nonportable) in the name. This function exists on Solaris 11.4 but not on the other platforms that I have tested (macOS 10.7.5, Solaris 11.3, FreeBSD 12.2 and FreeBSD 14) If I use pthread_attr_init() instead, then the testcase seems to run OK, but yet again there is a problem. On FreeBSD it seems as though the pthread_kill() with SIGINT is causing the application to return a SIGINT status, which stops tests/vg_regtest because of # if $keepunfiltered, copies $1 to $1.unfiltered.out # renames $0 tp $1 sub filtered_rename($$) { if ($keepunfiltered == 1) { mysystem("cp $_[1] $_[1].unfiltered.out"); } rename ($_[0], $_[1]); } So finally with the diff below, and an update to the expected, it should pass. However, I'm not sure that using pthread_attr_init rather than pthread_getattr_np changes the behaviour. I suspect not, since in this case the global default stack size should be the same as the individual stack sizes. Changing the stacks and sizes gets done after the call to __valgrind_register_current_stack A+ Paul paulf> git diff diff --git a/drd/tests/swapcontext.c b/drd/tests/swapcontext.c index e692559b0..79bab3221 100644 --- a/drd/tests/swapcontext.c +++ b/drd/tests/swapcontext.c @@ -8,7 +8,6 @@ #include <stdlib.h> #include <string.h> #include <sys/time.h> -#include <sys/timerfd.h> #include <ucontext.h> #include <unistd.h> #include "valgrind.h" @@ -21,7 +20,6 @@ typedef struct thread_local { static void f(void *data, int n) { enum { NR_SWITCHES = 200000 }; - struct pollfd pfd; thread_local_t *tlocal = data; while (1) { @@ -39,13 +37,16 @@ void __valgrind_register_current_stack(void) size_t stacksize; void *stack; - if (pthread_getattr_np(pthread_self(), &attr) != 0) + if (pthread_attr_init(&attr) != 0) abort(); if (pthread_attr_getstack(&attr, &stack, &stacksize) != 0) abort(); VALGRIND_STACK_REGISTER(stack, stack + stacksize); + + if (pthread_attr_destroy(&attr) != 0) + abort(); } #define STACKSIZE 8192 @@ -53,7 +54,6 @@ void __valgrind_register_current_stack(void) void *worker(void *data) { thread_local_t *tlocal = data; - struct itimerspec it; __valgrind_register_current_stack(); @@ -97,7 +97,7 @@ int main(int argc, char *argv[]) // Wait until the threads have been created. sleep(1); for (i = 0; i < NR; i++) - pthread_kill(thread[i], SIGINT); + pthread_kill(thread[i], SIGALRM); for (i = 0; i < NR; i++) pthread_join(thread[i], NULL); |
|
From: Bart V. A. <bva...@ac...> - 2021-02-23 20:44:27
|
On 2/22/21 2:35 AM, Paul Floyd wrote:
> This testcase is now closer to building, but still not quite there. The
> problem is now with 'pthread_getattr_np', which, as the manpage says
>
>
> CONFORMING TO
>
> This function is a nonstandard GNU extension; hence the suffix
> "_np" (nonportable) in the name.
>
>
> This function exists on Solaris 11.4 but not on the other platforms that
> I have tested (macOS 10.7.5, Solaris 11.3, FreeBSD 12.2 and FreeBSD 14)
>
> If I use pthread_attr_init() instead, then the testcase seems to run OK,
> but yet again there is a problem. On FreeBSD it seems as though the
> pthread_kill() with SIGINT is causing the application to return a SIGINT
> status, which stops tests/vg_regtest because of
>
> # if $keepunfiltered, copies $1 to $1.unfiltered.out
> # renames $0 tp $1
> sub filtered_rename($$)
> {
> if ($keepunfiltered == 1) {
> mysystem("cp $_[1] $_[1].unfiltered.out");
> }
> rename ($_[0], $_[1]);
> }
>
> So finally with the diff below, and an update to the expected, it should
> pass.
>
> However, I'm not sure that using pthread_attr_init rather than
> pthread_getattr_np changes the behaviour. I suspect not, since in this
> case the global default stack size should be the same as the individual
> stack sizes. Changing the stacks and sizes gets done after the call to
> __valgrind_register_current_stack
Hi Paul,
Thanks for the feedback. Please take a look at commit caf05d5ca941
("drd/tests/swapcontext: Improve the portability of this test further").
Bart.
|
|
From: Paul F. <pj...@wa...> - 2021-02-23 21:15:45
|
> On 23 Feb 2021, at 21:44, Bart Van Assche <bva...@ac...> wrote:
>
>
> Hi Paul,
>
> Thanks for the feedback. Please take a look at commit caf05d5ca941
> ("drd/tests/swapcontext: Improve the portability of this test further").
>
> Bart.
Hi Bart
I’ll take a look tomorrow.
A+
Paul
|
|
From: Paul F. <pj...@wa...> - 2021-02-24 12:21:54
|
On 23/02/2021 21:44, Bart Van Assche wrote:
>
> Hi Paul,
>
> Thanks for the feedback. Please take a look at commit caf05d5ca941
> ("drd/tests/swapcontext: Improve the portability of this test further").
>
> Bart.
Hi Bart
I made a couple more small changes. My status is now
Fedora 33 compiles and passes but I get intermittent failures as the
stack at the moment of the signal is sometimes that of the nanosleep call.
FreeBSD 12.2 compiles and passes with a little bit more filter (leading
underscore before swapcontext)
Solaris 11.3 compiles, minor diff
OS X still needs one more change (grr) then it compiles but the test
asserts. I wouldn't worry about that too much, most of the other DRD
tests also fail on OS X.
The Solaris diff is
--- swapcontext.stderr.exp 2021-02-24 10:21:51.613366346 +0100
+++ swapcontext.stderr.out 2021-02-24 11:12:44.815315110 +0100
@@ -1,7 +1,6 @@
Process terminating with default action of signal 14 (SIGALRM)
- at 0x........: swapcontext (in /...libc...)
- by 0x........: f (swapcontext.c:?)
+ at 0x........: f (swapcontext.c:?)
A+
Paul
|