Re: [PATCH] nptl: Fix testcases for new pthread cancellation, mechanism

[Adhemerval Zanella <adhemerval dot zanella at linaro dot org>]

Ping.

On 20-05-2015 11:10, Adhemerval Zanella wrote:
> Hi
> 
> 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).  I also added more test presented in
> tst-cancel-wrappers.sh in other tests.
> 
> Checked in i386, x86_64, powerpc32, powerpc64le, arm, and aarch64.
> 
> --
> 
> 	* nptl/Makefile [$(run-built-tests) = yes] (tests-special): Remove
> 	tst-cancel-wrappers.sh.
> 	* nptl/tst-cancel-wrappers.sh: Remove file.
> 	* nptl/tst-cancel2.c (tf): Add pthread_testcancel after cancellable
> 	syscall.
> 	* nptl/tst-cancel20.c (sh_body): Likewise.
> 	* nptl/tst-cancel21.c (sh_body): Likewise.
> 	(tf_body): Likewise.
> 	* nptl/tst-cancel4.c (tf_Write): Likewise.
> 	(tf_send): Likewise.
> 	(tf_open64): New test: check for open64 cancellable syscall.
> 	(tf_pread64): New test: check for pread64 cancellable syscall.
> 	(tf_pwrite64): New test: check for pwrite64 cancellable syscall.
> 
> ---
> 
> diff --git a/ChangeLog b/ChangeLog
> index 1deba46..033282a 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,5 +1,19 @@
>  2015-05-20  Adhemerval Zanella  <adhemerval.zanella@linaro.org>
>  
> +	* nptl/Makefile [$(run-built-tests) = yes] (tests-special): Remove
> +	tst-cancel-wrappers.sh.
> +	* nptl/tst-cancel-wrappers.sh: Remove file.
> +	* nptl/tst-cancel2.c (tf): Add pthread_testcancel after cancellable
> +	syscall.
> +	* nptl/tst-cancel20.c (sh_body): Likewise.
> +	* nptl/tst-cancel21.c (sh_body): Likewise.
> +	(tf_body): Likewise.
> +	* nptl/tst-cancel4.c (tf_Write): Likewise.
> +	(tf_send): Likewise.
> +	(tf_open64): New test: check for open64 cancellable syscall.
> +	(tf_pread64): New test: check for pread64 cancellable syscall.
> +	(tf_pwrite64): New test: check for pwrite64 cancellable syscall.
> +
>  	* sysdeps/unix/sysdep.h [SYSCALL_CANCEL]: New macro: define
>  	cancellable syscalls.
>  	(SYS_ify): Add guard to no redefine it.
> diff --git a/nptl/Makefile b/nptl/Makefile
> index 8e99452..c25a497 100644
> --- a/nptl/Makefile
> +++ b/nptl/Makefile
> @@ -385,8 +385,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
>  
> @@ -593,7 +592,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 \
> @@ -608,18 +607,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 b2d8302..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-2015 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 1a74992..3d34169 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 51b558e..614708a 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 b54f236..b1370e1 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 e50afd7..0a41d95 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);
>  
> @@ -1143,6 +1148,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);
>  
> @@ -1425,6 +1432,38 @@ tf_open (void *arg)
>    exit (1);
>  }
>  
> +static void *
> +tf_open64 (void *arg)
> +{
> +  if (arg == NULL)
> +    // XXX If somebody can provide a portable test case in which open()
> +    // blocks we can enable this test to run in both rounds.
> +    abort ();
> +
> +  int r = pthread_barrier_wait (&b2);
> +  if (r != 0 && r != PTHREAD_BARRIER_SERIAL_THREAD)
> +    {
> +      printf ("%s: barrier_wait failed\n", __FUNCTION__);
> +      exit (1);
> +    }
> +
> +  r = pthread_barrier_wait (&b2);
> +  if (r != 0 && r != PTHREAD_BARRIER_SERIAL_THREAD)
> +    {
> +      printf ("%s: 2nd barrier_wait failed\n", __FUNCTION__);
> +      exit (1);
> +    }
> +
> +  pthread_cleanup_push (cl, NULL);
> +
> +  open64 ("Makefile", O_RDONLY);
> +
> +  pthread_cleanup_pop (0);
> +
> +  printf ("%s: open returned\n", __FUNCTION__);
> +
> +  exit (1);
> +}
>  
>  static void *
>  tf_close (void *arg)
> @@ -1510,6 +1549,46 @@ tf_pread (void *arg)
>    exit (1);
>  }
>  
> +static void *
> +tf_pread64 (void *arg)
> +{
> +  if (arg == NULL)
> +    // XXX If somebody can provide a portable test case in which pread()
> +    // blocks we can enable this test to run in both rounds.
> +    abort ();
> +
> +  tempfd = open64 ("Makefile", O_RDONLY);
> +  if (tempfd == -1)
> +    {
> +      printf ("%s: cannot open64 Makefile\n", __FUNCTION__);
> +      exit (1);
> +    }
> +
> +  int r = pthread_barrier_wait (&b2);
> +  if (r != 0 && r != PTHREAD_BARRIER_SERIAL_THREAD)
> +    {
> +      printf ("%s: barrier_wait failed\n", __FUNCTION__);
> +      exit (1);
> +    }
> +
> +  r = pthread_barrier_wait (&b2);
> +  if (r != 0 && r != PTHREAD_BARRIER_SERIAL_THREAD)
> +    {
> +      printf ("%s: 2nd barrier_wait failed\n", __FUNCTION__);
> +      exit (1);
> +    }
> +
> +  pthread_cleanup_push (cl, NULL);
> +
> +  char mem[10];
> +  pread64 (tempfd, mem, sizeof (mem), 0);
> +
> +  pthread_cleanup_pop (0);
> +
> +  printf ("%s: pread64 returned\n", __FUNCTION__);
> +
> +  exit (1);
> +}
>  
>  static void *
>  tf_pwrite (void *arg)
> @@ -1554,6 +1633,48 @@ tf_pwrite (void *arg)
>    exit (1);
>  }
>  
> +static void *
> +tf_pwrite64 (void *arg)
> +{
> +  if (arg == NULL)
> +    // XXX If somebody can provide a portable test case in which pwrite()
> +    // blocks we can enable this test to run in both rounds.
> +    abort ();
> +
> +  char fname[] = "/tmp/tst-cancel4-fd-XXXXXX";
> +  tempfd = mkstemp (fname);
> +  if (tempfd == -1)
> +    {
> +      printf ("%s: mkstemp failed\n", __FUNCTION__);
> +      exit (1);
> +    }
> +  unlink (fname);
> +
> +  int r = pthread_barrier_wait (&b2);
> +  if (r != 0 && r != PTHREAD_BARRIER_SERIAL_THREAD)
> +    {
> +      printf ("%s: barrier_wait failed\n", __FUNCTION__);
> +      exit (1);
> +    }
> +
> +  r = pthread_barrier_wait (&b2);
> +  if (r != 0 && r != PTHREAD_BARRIER_SERIAL_THREAD)
> +    {
> +      printf ("%s: 2nd barrier_wait failed\n", __FUNCTION__);
> +      exit (1);
> +    }
> +
> +  pthread_cleanup_push (cl, NULL);
> +
> +  char mem[10];
> +  pwrite64 (tempfd, mem, sizeof (mem), 0);
> +
> +  pthread_cleanup_pop (0);
> +
> +  printf ("%s: pwrite64 returned\n", __FUNCTION__);
> +
> +  exit (1);
> +}
>  
>  static void *
>  tf_fsync (void *arg)
> @@ -2141,9 +2262,12 @@ static struct
>    ADD_TEST (recvfrom, 2, 0),
>    ADD_TEST (recvmsg, 2, 0),
>    ADD_TEST (open, 2, 1),
> +  ADD_TEST (open64, 2, 1),
>    ADD_TEST (close, 2, 1),
>    ADD_TEST (pread, 2, 1),
> +  ADD_TEST (pread64, 2, 1),
>    ADD_TEST (pwrite, 2, 1),
> +  ADD_TEST (pwrite64, 2, 1),
>    ADD_TEST (fsync, 2, 1),
>    ADD_TEST (fdatasync, 2, 1),
>    ADD_TEST (msync, 2, 1),
>