[3/8] nptl: Fix testcases for new pthread cancellation, mechanism
Commit Message
With upcoming fix for BZ#12683, pthread cancellation does not act for:
1. If syscall is blocked but with some side effects already having taken
place (e.g. a partial read or write)
2. After the syscall has returned.
It is because program need to act on such cases (for instance, to avoid
leak of allocated resources our handling partial read/write).
This patches fixes the NPTL testcase that assumes the old behavior and
also remove the tst-cancel-wrappers.sh test (which checks for symbols
that does not exist anymore).
For testcases that are not tested in blocked case (pread64 for instance)
tst-cancel4/5 tests for its cancellation point as 'early exit'. Although
this is not ideal, 'tst-cancel-wrappers.sh' also does not offer more
coverage. Ideally and as pointed out in testcases itself, the best option
would be to create a way to actually block some cancellable entrypoint.
However, for some syscalls calls this is very trick (pread64 for instance
does not work on pipe-type descriptors).
Checked on x86_64, ppc32, and ppc64.
--
* nptl/Makefile [test-special]: Remove tst-cancel-wrappers rule.
[generated]: Likewise.
* nptl/tst-cancel-wrappers.sh: Remove file.
* nptl/tst-cancel2.c (tf): Add pthread_cancel checks for partial
read/write due cancel signal.
* nptl/tst-cancel20.c (sh_body): Likewise.
(tf_body): Likewise.
* nptl/tst-cancel21.c (sh_body): Likewise.
(tf_body): Likewise.
* nptl/tst-cancel4.c (tf_write): Likewise.
(tf_send): Likewise.
---
Comments
On Wed, 8 Oct 2014, Adhemerval Zanella wrote:
> For testcases that are not tested in blocked case (pread64 for instance)
> tst-cancel4/5 tests for its cancellation point as 'early exit'. Although
> this is not ideal, 'tst-cancel-wrappers.sh' also does not offer more
> coverage. Ideally and as pointed out in testcases itself, the best option
> would be to create a way to actually block some cancellable entrypoint.
> However, for some syscalls calls this is very trick (pread64 for instance
> does not work on pipe-type descriptors).
I still don't understand exactly what the effects are on test coverage.
Could you provide a list of all the functions tested in
tst-cancel-wrappers.sh, with, for each function, a statement either of the
other testcase that tests cancellation of that function, or of why it is
not possible to test cancellation of that function in a useful way?
On 08-10-2014 20:12, Joseph S. Myers wrote:
> On Wed, 8 Oct 2014, Adhemerval Zanella wrote:
>
>> For testcases that are not tested in blocked case (pread64 for instance)
>> tst-cancel4/5 tests for its cancellation point as 'early exit'. Although
>> this is not ideal, 'tst-cancel-wrappers.sh' also does not offer more
>> coverage. Ideally and as pointed out in testcases itself, the best option
>> would be to create a way to actually block some cancellable entrypoint.
>> However, for some syscalls calls this is very trick (pread64 for instance
>> does not work on pipe-type descriptors).
> I still don't understand exactly what the effects are on test coverage.
>
> Could you provide a list of all the functions tested in
> tst-cancel-wrappers.sh, with, for each function, a statement either of the
> other testcase that tests cancellation of that function, or of why it is
> not possible to test cancellation of that function in a useful way?
>
I understand your hesitation about remove tst-cancel-wrappers.sh testcase,
however currently it does not offer any more coverage than current other tests.
Below is a chart I compiled with the function tst-cancel-wrappers.sh is intended
to check:
syscall | testcase | blocked case | early exit | Notes
----------------|----------------|--------------|------------|------
accept | tst-cancel4.c | yes | yes |
close | tst-cancel4.c | no | yes |
connect | tst-cancel4.c | no | yes |
creat | tst-cancel4.c | no | yes |
fcntl | tst-cancel16.c | no | yes | *
fdatasync | tst-cancel4.c | no | yes |
fsync | tst-cancel4.c | no | yes |
msgrcv | tst-cancel4.c | yes | yes |
msgsnd | tst-cancel4.c | no | yes |
msync | tst-cancel4.c | no | yes |
nanosleep | tst-cancel4.c | yes | yes |
open/open64 | tst-cancel4.c | no | yes |
pause | tst-cancel4.c | yes | yes |
poll | tst-cancel4.c | yes | yes |
pread/pread64 | tst-cancel4.c | no | yes |
pselect | tst-cancel4.c | yes | yes |
pwrite/pwrite64 | tst-cancel4.c | no | yes |
read | tst-cancel4.c | yes | yes |
readv | tst-cancel4.c | yes | yes |
recv | tst-cancel4.c | yes | yes |
recvfrom | tst-cancel4.c | yes | yes |
recvmsg | tst-cancel4.c | yes | yes |
select | tst-cancel4.c | yes | yes |
send | tst-cancel4.c | yes | yes |
sendmsg | tst-cancel4.c | no | yes |
sendto | tst-cancel4.c | no | yes |
sigpause | tst-cancel4.c | yes | yes |
sigsuspend | tst-cancel4.c | yes | yes |
sigwait | tst-cancel4.c | yes | yes |
sigwaitinfo | tst-cancel4.c | yes | yes |
tcdrain | tst-cancel4.c | no | yes |
wait | tst-cancel4.c | yes | yes |
waitid | tst-cancel4.c | yes | yes |
waitpid | tst-cancel4.c | yes | yes |
write | tst-cancel4.c | yes | yes |
writev | tst-cancel4.c | yes | yes |
* tested through lockf (F_LOCK) which calls fctnl (F_SETLK)
The 'blocked case' case refers to a cancellation signal send while the syscall
is blocked and the test checks cancellation is done properly. As the 'tst-cancel4.c'
states, some syscalls are hard to put in a blocked case (pread/close for instance)
to exercise all code patch. If you have suggestion of how to add blocked cases
I can add them in a following patch.
The 'early exit' checks if the cancellation entrypoint will check for pending
cancellation signal and act accordingly. As you can for this case 'tst-cancel4.c'
already cover alls the syscalls.
On Thu, 9 Oct 2014, Adhemerval Zanella wrote:
> open/open64 | tst-cancel4.c | no | yes |
> pread/pread64 | tst-cancel4.c | no | yes |
> pwrite/pwrite64 | tst-cancel4.c | no | yes |
I don't think pairing like that is legitimate. Each of those pairs is two
separate functions, and only one of each pair is tested in tst-cancel4.c.
The *64 functions need testing to at least the same extent.
On 09-10-2014 11:55, Joseph S. Myers wrote:
> On Thu, 9 Oct 2014, Adhemerval Zanella wrote:
>
>> open/open64 | tst-cancel4.c | no | yes |
>> pread/pread64 | tst-cancel4.c | no | yes |
>> pwrite/pwrite64 | tst-cancel4.c | no | yes |
> I don't think pairing like that is legitimate. Each of those pairs is two
> separate functions, and only one of each pair is tested in tst-cancel4.c.
> The *64 functions need testing to at least the same extent.
>
Right, will be suffice to add proper tests for open64/pread64/pwrite64 using
__USE_LARGEFILE64 on tst-cancel4.c to cover such cases?
On Thu, 9 Oct 2014, Adhemerval Zanella wrote:
> On 09-10-2014 11:55, Joseph S. Myers wrote:
> > On Thu, 9 Oct 2014, Adhemerval Zanella wrote:
> >
> >> open/open64 | tst-cancel4.c | no | yes |
> >> pread/pread64 | tst-cancel4.c | no | yes |
> >> pwrite/pwrite64 | tst-cancel4.c | no | yes |
> > I don't think pairing like that is legitimate. Each of those pairs is two
> > separate functions, and only one of each pair is tested in tst-cancel4.c.
> > The *64 functions need testing to at least the same extent.
> >
> Right, will be suffice to add proper tests for open64/pread64/pwrite64 using
> __USE_LARGEFILE64 on tst-cancel4.c to cover such cases?
You shouldn't need __USE_LARGEFILE64 (_GNU_SOURCE is used by default,
which implies __USE_LARGEFILE64). Yes, I think adding tests of those
functions similar to the existing ones of open/pread/pwrite should
suffice.
@@ -377,8 +377,7 @@ tests-reverse += tst-cancel5 tst-cancel23 tst-vfork1x tst-vfork2x
ifeq ($(run-built-tests),yes)
tests-special += $(objpfx)tst-stack3-mem.out $(objpfx)tst-oddstacklimit.out
ifeq ($(build-shared),yes)
-tests-special += $(objpfx)tst-tls6.out $(objpfx)tst-cleanup0-cmp.out \
- $(objpfx)tst-cancel-wrappers.out
+tests-special += $(objpfx)tst-tls6.out $(objpfx)tst-cleanup0-cmp.out
endif
endif
@@ -572,7 +571,7 @@ $(objpfx)$(multidir)/crtn.o: $(objpfx)crtn.o $(objpfx)$(multidir)/
endif
generated += libpthread_nonshared.a \
- multidir.mk tst-atfork2.mtrace tst-cancel-wrappers.out \
+ multidir.mk tst-atfork2.mtrace \
tst-tls6.out
generated += $(objpfx)tst-atfork2.mtrace \
@@ -587,18 +586,6 @@ generated += banner.h
LDFLAGS-pthread.so += -e __nptl_main
endif
-ifeq ($(run-built-tests),yes)
-ifeq (yes,$(build-shared))
-$(objpfx)tst-cancel-wrappers.out: tst-cancel-wrappers.sh
- $(SHELL) $< '$(NM)' \
- $(common-objpfx)libc_pic.a \
- $(common-objpfx)libc.a \
- $(objpfx)libpthread_pic.a \
- $(objpfx)libpthread.a > $@; \
- $(evaluate-test)
-endif
-endif
-
tst-exec4-ARGS = $(host-test-program-cmd)
$(objpfx)tst-execstack: $(libdl)
deleted file mode 100644
@@ -1,92 +0,0 @@
-#! /bin/sh
-# Test whether all cancelable functions are cancelable.
-# Copyright (C) 2002-2014 Free Software Foundation, Inc.
-# This file is part of the GNU C Library.
-# Contributed by Jakub Jelinek <jakub@redhat.com>, 2002.
-
-# The GNU C Library is free software; you can redistribute it and/or
-# modify it under the terms of the GNU Lesser General Public
-# License as published by the Free Software Foundation; either
-# version 2.1 of the License, or (at your option) any later version.
-
-# The GNU C Library 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
-# Lesser General Public License for more details.
-
-# You should have received a copy of the GNU Lesser General Public
-# License along with the GNU C Library; if not, see
-# <http://www.gnu.org/licenses/>.
-
-NM="$1"; shift
-while [ $# -gt 0 ]; do
- ( $NM -P $1; echo 'end[end]:' ) | gawk ' BEGIN {
-C["accept"]=1
-C["close"]=1
-C["connect"]=1
-C["creat"]=1
-C["fcntl"]=1
-C["fdatasync"]=1
-C["fsync"]=1
-C["msgrcv"]=1
-C["msgsnd"]=1
-C["msync"]=1
-C["nanosleep"]=1
-C["open"]=1
-C["open64"]=1
-C["pause"]=1
-C["poll"]=1
-C["pread"]=1
-C["pread64"]=1
-C["pselect"]=1
-C["pwrite"]=1
-C["pwrite64"]=1
-C["read"]=1
-C["readv"]=1
-C["recv"]=1
-C["recvfrom"]=1
-C["recvmsg"]=1
-C["select"]=1
-C["send"]=1
-C["sendmsg"]=1
-C["sendto"]=1
-C["sigpause"]=1
-C["sigsuspend"]=1
-C["sigwait"]=1
-C["sigwaitinfo"]=1
-C["tcdrain"]=1
-C["wait"]=1
-C["waitid"]=1
-C["waitpid"]=1
-C["write"]=1
-C["writev"]=1
-C["__xpg_sigpause"]=1
-}
-/:$/ {
- if (seen)
- {
- if (!seen_enable || !seen_disable)
- {
- printf "in '$1'(%s) %s'\''s cancellation missing\n", object, seen
- ret = 1
- }
- }
- seen=""
- seen_enable=""
- seen_disable=""
- object=gensub(/^.*\[(.*)\]:$/,"\\1","",$0)
- next
-}
-{
- if (C[$1] && $2 ~ /^[TW]$/)
- seen=$1
- else if ($1 ~ /^([.]|)__(libc|pthread)_enable_asynccancel$/ && $2 == "U")
- seen_enable=1
- else if ($1 ~ /^([.]|)__(libc|pthread)_disable_asynccancel$/ && $2 == "U")
- seen_disable=1
-}
-END {
- exit ret
-}' || exit
- shift
-done
@@ -33,6 +33,9 @@ tf (void *arg)
char buf[100000];
while (write (fd[1], buf, sizeof (buf)) > 0);
+ /* The write can return a value higher than 0 (meaning partial write)
+ due the SIGCANCEL, but the thread may still pending cancellation. */
+ pthread_testcancel ();
return (void *) 42l;
}
@@ -49,6 +49,9 @@ sh_body (void)
puts ("read succeeded");
exit (1);
}
+ /* The read can return a value higher than 0 (meaning partial reads)
+ due the SIGCANCEL, but the thread may still pending cancellation. */
+ pthread_testcancel ();
pthread_cleanup_pop (0);
}
@@ -84,7 +87,8 @@ tf_body (void)
puts ("read succeeded");
exit (1);
}
-
+ /* Check for partial read. */
+ pthread_testcancel ();
read (fd[0], &c, 1);
pthread_cleanup_pop (0);
@@ -50,6 +50,9 @@ sh_body (void)
puts ("read succeeded");
exit (1);
}
+ /* The write can return a value higher than 0 (meaning partial write)
+ due the SIGCANCEL, but the thread may still pending cancellation. */
+ pthread_testcancel ();
pthread_cleanup_pop (0);
}
@@ -85,6 +88,8 @@ tf_body (void)
puts ("read succeeded");
exit (1);
}
+ /* Check partial read. */
+ pthread_testcancel ();
read (fd[0], &c, 1);
@@ -38,8 +38,10 @@
#include <sys/un.h>
#include <sys/wait.h>
-#include "pthreadP.h"
-
+/* The signal used for asynchronous cancelation. */
+#ifndef SIGCANCEL
+# define SIGCANCEL __SIGRTMIN
+#endif
/* Since STREAMS are not supported in the standard Linux kernel and
there we don't advertise STREAMS as supported is no need to test
@@ -247,6 +249,9 @@ tf_write (void *arg)
char buf[WRITE_BUFFER_SIZE];
memset (buf, '\0', sizeof (buf));
s = write (fd, buf, sizeof (buf));
+ /* The write can return a value higher than 0 (meaning partial write)
+ due the SIGCANCEL, but the thread may still pending cancellation. */
+ pthread_testcancel ();
pthread_cleanup_pop (0);
@@ -1139,6 +1144,8 @@ tf_send (void *arg)
char mem[700000];
send (tempfd2, mem, arg == NULL ? sizeof (mem) : 1, 0);
+ /* Check for partial send. */
+ pthread_testcancel ();
pthread_cleanup_pop (0);