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

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

Ping.

On 14-10-2015 16:32, Adhemerval Zanella wrote:
> Changed from previous version:
> 
> - Removed tst-cancel{2,20,21} changes since they are not required.
> 
> 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).
> 
> It also add two more testcase to check both the returned and the errno
> value on case of a cancelled syscall.
> 
> Tested on i686, x86_64, x32, powerpc64le, and aarch64.
> 
> 	* nptl/Makefile [$(run-built-tests) = yes] (tests-special): Remove
> 	tst-cancel-wrappers.sh.
> 	* nptl/tst-cancel-wrappers.sh: Remove file.
> 	* nptl/tst-cancel4.c (tf_Write): Likewise.
> 	(tf_send): Likewise.
> 	(cl_fifo): New function: pipe handling for open/open64.
> 	(tf_sigpause): Use sigpause instead of __xpg_sigpausea and use
> 	SIGINT instead of SIGCANCEL.
> 	(tf_open): Use mkfifo to check for early cancel.
> 	(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.
> ---
>  ChangeLog                   |  15 ++++
>  nptl/Makefile               |  17 +---
>  nptl/tst-cancel-wrappers.sh |  92 ----------------------
>  nptl/tst-cancel4.c          | 185 ++++++++++++++++++++++++++++++++++++++++----
>  4 files changed, 185 insertions(+), 124 deletions(-)
>  delete mode 100644 nptl/tst-cancel-wrappers.sh
> 
> diff --git a/nptl/Makefile b/nptl/Makefile
> index 311b1a7..b7ff3f1 100644
> --- a/nptl/Makefile
> +++ b/nptl/Makefile
> @@ -398,8 +398,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
>  
> @@ -615,7 +614,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 \
> @@ -632,18 +631,6 @@ LDFLAGS-pthread.so += -e __nptl_main
>  $(objpfx)pt-interp.os: $(common-objpfx)runtime-linker.h
>  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 d492a54..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", 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-cancel4.c b/nptl/tst-cancel4.c
> index e50afd7..d63ddfd 100644
> --- a/nptl/tst-cancel4.c
> +++ b/nptl/tst-cancel4.c
> @@ -38,8 +38,6 @@
>  #include <sys/un.h>
>  #include <sys/wait.h>
>  
> -#include "pthreadP.h"
> -
>  
>  /* Since STREAMS are not supported in the standard Linux kernel and
>     there we don't advertise STREAMS as supported is no need to test
> @@ -117,7 +115,20 @@ cl (void *arg)
>    ++cl_called;
>  }
>  
> +/* Named pipe used to check for blocking open.  It should be closed
> +   after the cancellation handling.  */
> +static char fifoname[] = "/tmp/tst-cancel4-fifo-XXXXXX";
> +static int fifofd;
>  
> +static void
> +cl_fifo (void *arg)
> +{
> +  ++cl_called;
> +
> +  unlink (fifoname);
> +  close (fifofd);
> +  fifofd = -1;
> +}
>  
>  static void *
>  tf_read  (void *arg)
> @@ -247,6 +258,10 @@ 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 to the SIGCANCEL, but the thread may still be pending
> +     cancellation.  */
> +  pthread_testcancel ();
>  
>    pthread_cleanup_pop (0);
>  
> @@ -781,13 +796,7 @@ tf_sigpause (void *arg)
>  
>    pthread_cleanup_push (cl, NULL);
>  
> -#ifdef SIGCANCEL
> -  /* Just for fun block the cancellation signal.  We need to use
> -     __xpg_sigpause since otherwise we will get the BSD version.  */
> -  __xpg_sigpause (SIGCANCEL);
> -#else
> -  pause ();
> -#endif
> +  sigpause (sigmask (SIGINT));
>  
>    pthread_cleanup_pop (0);
>  
> @@ -1143,6 +1152,10 @@ tf_send (void *arg)
>    char mem[700000];
>  
>    send (tempfd2, mem, arg == NULL ? sizeof (mem) : 1, 0);
> +  /* Thez send can return a value higher than 0 (meaning partial send)
> +     due to the SIGCANCEL, but the thread may still be pending
> +     cancellation.  */
> +  pthread_testcancel ();
>  
>    pthread_cleanup_pop (0);
>  
> @@ -1396,9 +1409,23 @@ static void *
>  tf_open (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 ();
> +    {
> +      fifofd = mkfifo (fifoname, S_IWUSR | S_IRUSR);
> +      if (fifofd == -1)
> +	{
> +	  printf ("%s: mkfifo failed\n", __FUNCTION__);
> +	  exit (1);
> +	}
> +    }
> +  else
> +    {
> +      int r = pthread_barrier_wait (&b2);
> +      if (r != 0 && r != PTHREAD_BARRIER_SERIAL_THREAD)
> +	{
> +	  printf ("%s: barrier_wait failed\n", __FUNCTION__);
> +	  exit (1);
> +	}
> +    }
>  
>    int r = pthread_barrier_wait (&b2);
>    if (r != 0 && r != PTHREAD_BARRIER_SERIAL_THREAD)
> @@ -1407,16 +1434,49 @@ tf_open (void *arg)
>        exit (1);
>      }
>  
> -  r = pthread_barrier_wait (&b2);
> +  pthread_cleanup_push (cl_fifo, NULL);
> +
> +  open (arg ? "Makefile" : fifoname, O_RDONLY);
> +
> +  pthread_cleanup_pop (0);
> +
> +  printf ("%s: open returned\n", __FUNCTION__);
> +
> +  exit (1);
> +}
> +
> +static void *
> +tf_open64 (void *arg)
> +{
> +  if (arg == NULL)
> +    {
> +      fifofd = mkfifo (fifoname, S_IWUSR | S_IRUSR);
> +      if (fifofd == -1)
> +	{
> +	  printf ("%s: mkfifo failed\n", __FUNCTION__);
> +	  exit (1);
> +	}
> +    }
> +  else
> +    {
> +      int r = pthread_barrier_wait (&b2);
> +      if (r != 0 && r != PTHREAD_BARRIER_SERIAL_THREAD)
> +	{
> +	  printf ("%s: barrier_wait failed\n", __FUNCTION__);
> +	  exit (1);
> +	}
> +    }
> +
> +  int r = pthread_barrier_wait (&b2);
>    if (r != 0 && r != PTHREAD_BARRIER_SERIAL_THREAD)
>      {
> -      printf ("%s: 2nd barrier_wait failed\n", __FUNCTION__);
> +      printf ("%s: barrier_wait failed\n", __FUNCTION__);
>        exit (1);
>      }
>  
> -  pthread_cleanup_push (cl, NULL);
> +  pthread_cleanup_push (cl_fifo, NULL);
>  
> -  open ("Makefile", O_RDONLY);
> +  open64 (arg ? "Makefile" : fifoname, O_RDONLY);
>  
>    pthread_cleanup_pop (0);
>  
> @@ -1510,6 +1570,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 +1654,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)
> @@ -2140,10 +2282,13 @@ static struct
>    ADD_TEST (recv, 2, 0),
>    ADD_TEST (recvfrom, 2, 0),
>    ADD_TEST (recvmsg, 2, 0),
> -  ADD_TEST (open, 2, 1),
> +  ADD_TEST (open, 2, 0),
> +  ADD_TEST (open64, 2, 0),
>    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),
> @@ -2185,6 +2330,12 @@ do_test (void)
>      }
>    setsockopt (fds[1], SOL_SOCKET, SO_SNDBUF, &val, sizeof(val));
>  
> +  if (mktemp (fifoname) == NULL)
> +    {
> +      printf ("%s: cannot generate temp file name\n", __FUNCTION__);
> +      exit (1);
> +    }
> +
>    int result = 0;
>    size_t cnt;
>    for (cnt = 0; cnt < ntest_tf; ++cnt)
>