[3/4] nptl: Fix testcases for new pthread cancellation, mechanism

Message ID 541C2DDA.6050204@linux.vnet.ibm.com
State Superseded
Headers

Commit Message

Adhemerval Zanella Netto Sept. 19, 2014, 1:21 p.m. UTC
  With 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).

Tested on powerpc64 and x86_64.

--

	* debug/tst-backtrace5.c (handle_signal): Adjust to check for
	__syscall_call symbol.
	* 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

Joseph Myers Sept. 19, 2014, 5:02 p.m. UTC | #1
On Fri, 19 Sep 2014, Adhemerval Zanella wrote:

> This patches fixes the NPTL testcase that assumes the old behavior and

Do the fixes result in tests that work both with and without the main 
changes?  If so, they should go first.  (In general, for bisectability 
either you fix tests first to work both with and without the main changes, 
or the test fixes need to go in the same commit as the main changes even 
if posted separately for convenience.)

> also remove the tst-cancel-wrappers.sh test (which checks for symbols
> that does not exist anymore).

Is there a better way of achieving the substance of this test (i.e. 
verifying that all those functions are cancellable) that still works after 
the patch?
  
Adhemerval Zanella Netto Sept. 22, 2014, 1:14 p.m. UTC | #2
Hi Joseph, thanks for the review.


On 19-09-2014 14:02, Joseph S. Myers wrote:
> On Fri, 19 Sep 2014, Adhemerval Zanella wrote:
>
>> This patches fixes the NPTL testcase that assumes the old behavior and
> Do the fixes result in tests that work both with and without the main 
> changes?  If so, they should go first.  (In general, for bisectability 
> either you fix tests first to work both with and without the main changes, 
> or the test fixes need to go in the same commit as the main changes even 
> if posted separately for convenience.)

The debug/tst-backtrace5.c is adjusted to take in consideration the new stack chain
call for syscalls. The tst-cancelX modification work on both schemes (the thread
in cancelled in the syscall itself).

>
>> also remove the tst-cancel-wrappers.sh test (which checks for symbols
>> that does not exist anymore).
> Is there a better way of achieving the substance of this test (i.e. 
> verifying that all those functions are cancellable) that still works after 
> the patch?
>
I will double check, but later time I checked all the cancelable entry point in 
GLIBC are already being fairly tested by other nptl/tst-cancelX cases.
  

Patch

diff --git a/debug/tst-backtrace5.c b/debug/tst-backtrace5.c
index 4f55215..04cd770 100644
--- a/debug/tst-backtrace5.c
+++ b/debug/tst-backtrace5.c
@@ -73,11 +73,20 @@  handle_signal (int signum)
       FAIL ();
       return;
     }
-  /* Do not check name for signal trampoline.  */
+  /* The syscall symbol cancellation wrapper.  */
   i = 2;
-  if (!match (symbols[i++], "read"))
+  if (!match (symbols[i++], "__syscall_cancel"))
     {
       /* Perhaps symbols[2] is __kernel_vsyscall?  */
+      if (!match (symbols[i++], "__syscall_cancel"))
+	{
+	  FAIL ();
+	  return;
+	}
+    }
+  /* The syscall symbol itself.  */
+  if (!match (symbols[i++], "read"))
+    {
       if (!match (symbols[i++], "read"))
 	{
 	  FAIL ();
diff --git a/nptl/Makefile b/nptl/Makefile
index 968dffd..7d68dad 100644
--- a/nptl/Makefile
+++ b/nptl/Makefile
@@ -375,8 +375,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
 
@@ -570,7 +569,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 \
@@ -585,18 +584,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)
diff --git a/nptl/tst-cancel-wrappers.sh b/nptl/tst-cancel-wrappers.sh
deleted file mode 100644
index e41ed51..0000000
--- a/nptl/tst-cancel-wrappers.sh
+++ /dev/null
@@ -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
diff --git a/nptl/tst-cancel2.c b/nptl/tst-cancel2.c
index 2d834de..cb807c4 100644
--- a/nptl/tst-cancel2.c
+++ b/nptl/tst-cancel2.c
@@ -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;
 }
diff --git a/nptl/tst-cancel20.c b/nptl/tst-cancel20.c
index 703e558..0d54b21 100644
--- a/nptl/tst-cancel20.c
+++ b/nptl/tst-cancel20.c
@@ -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);
diff --git a/nptl/tst-cancel21.c b/nptl/tst-cancel21.c
index ddcea90..35a56dc 100644
--- a/nptl/tst-cancel21.c
+++ b/nptl/tst-cancel21.c
@@ -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);
 
diff --git a/nptl/tst-cancel4.c b/nptl/tst-cancel4.c
index 93080b2..fd4ea42 100644
--- a/nptl/tst-cancel4.c
+++ b/nptl/tst-cancel4.c
@@ -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);