Patchwork [v2,02/21] nptl: Fix testcases for new pthread cancellation mechanism

login
register
mail settings
Submitter Adhemerval Zanella Netto
Date Feb. 26, 2018, 9:03 p.m.
Message ID <1519679016-12241-3-git-send-email-adhemerval.zanella@linaro.org>
Download mbox | patch
Permalink /patch/26077/
State New
Headers show

Comments

Adhemerval Zanella Netto - Feb. 26, 2018, 9:03 p.m.
From: Adhemerval Zanella <adhemerval.zanella@linaro.com>

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.

The main change is due the fact programs need to act in syscalls with
side-effects (for instance, to avoid leak of allocated resources or
handle partial read/write).

This patch changes the NPTL testcase that assumes the old behavior and
also remove the tst-cancel-wrappers.sh test (which checks for symbols
that will not exist anymore).  For tst-cancel{2,3} case it remove the
pipe close because it might cause the write syscall to return with
side effects if the close is executed before the pthread cancel.

It also changes how to call the read syscall on tst-backtrace{5,6}
to use syscall instead of read cancelable syscall to avoid need to
handle the cancelable bridge function calls.  It requires a change
on powerpc syscall implementation to create a stackframe, since
powerpc backtrace rely on such information.

Checked on i686-linux-gnu, x86_64-linux-gnu, x86_64-linux-gnux32,
aarch64-linux-gnu, arm-linux-gnueabihf, powerpc64le-linux-gnu,
powerpc-linux-gnu, sparcv9-linux-gnu, and sparc64-linux-gnu.

	* debug/tst-backtrace5.c (handle_signal): Check for syscall
	instead of read.
	(fn): Issue the read syscall instead of call the cancellable
	syscall.
	* nptl/Makefile [$(run-built-tests) = yes] (tests-special): Remove
	tst-cancel-wrappers.sh.
	* nptl/tst-cancel-wrappers.sh: Remove file.
	* nptl/tst-cancel2.c (do_test): Do not close pipe.
	* nptl/tst-cancel3.c (do_test): Likewise.
	* nptl/tst-cancel4.c (tf_write): Handle cancelled syscall with
	side-effects.
	(tf_send): Likewise.
	* sysdeps/unix/sysv/linux/powerpc/syscall.S (syscall): Create stack
	frame.
---
 ChangeLog                   | 14 ++++++-
 debug/tst-backtrace5.c      | 28 ++++++++------
 nptl/Makefile               | 17 +--------
 nptl/tst-cancel-wrappers.sh | 92 ---------------------------------------------
 nptl/tst-cancel2.c          |  3 --
 nptl/tst-cancel3.c          |  3 --
 nptl/tst-cancel4.c          |  8 ++++
 7 files changed, 39 insertions(+), 126 deletions(-)
 delete mode 100644 nptl/tst-cancel-wrappers.sh
Andreas Schwab - Feb. 26, 2018, 9:43 p.m.
On Feb 26 2018, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:

> @@ -743,6 +747,10 @@ tf_send (void *arg)
>    char mem[WRITE_BUFFER_SIZE];
>  
>    send (tempfd2, mem, arg == NULL ? sizeof (mem) : 1, 0);
> +  /* Thez send can return a value higher than 0 (meaning partial send)

s/Thez/The/

Andreas.
Adhemerval Zanella Netto - Feb. 27, 2018, 12:05 p.m.
On 26/02/2018 18:43, Andreas Schwab wrote:
> On Feb 26 2018, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
> 
>> @@ -743,6 +747,10 @@ tf_send (void *arg)
>>    char mem[WRITE_BUFFER_SIZE];
>>  
>>    send (tempfd2, mem, arg == NULL ? sizeof (mem) : 1, 0);
>> +  /* Thez send can return a value higher than 0 (meaning partial send)
> 
> s/Thez/The/
> 
> Andreas.
> 

Ack, fixed locally.

Patch

diff --git a/debug/tst-backtrace5.c b/debug/tst-backtrace5.c
index 0e6fb1a..a6d66c8 100644
--- a/debug/tst-backtrace5.c
+++ b/debug/tst-backtrace5.c
@@ -23,6 +23,7 @@ 
 #include <stdlib.h>
 #include <string.h>
 #include <sys/types.h>
+#include <sys/syscall.h>
 #include <signal.h>
 #include <unistd.h>
 
@@ -33,7 +34,7 @@ 
 #endif
 
 /* The backtrace should include at least handle_signal, a signal
-   trampoline, read, 3 * fn, and do_test.  */
+   trampoline, syscall, 3 * fn, and do_test.  */
 #define NUM_FUNCTIONS 7
 
 void
@@ -71,16 +72,18 @@  handle_signal (int signum)
     }
   /* Do not check name for signal trampoline.  */
   i = 2;
-  if (!match (symbols[i++], "read"))
+
+  if (match (symbols[i], "__kernel_vsyscall"))
+    i++;
+
+  /* We are using syscall(...) instead of read.  */
+  if (!match (symbols[i++], "syscall"))
     {
-      /* Perhaps symbols[2] is __kernel_vsyscall?  */
-      if (!match (symbols[i++], "read"))
-	{
-	  FAIL ();
-	  return;
-	}
+      FAIL ();
+      return;
     }
-  for (; i < n - 1; i++)
+
+  for (; i < n - 2; i++)
     if (!match (symbols[i], "fn"))
       {
 	FAIL ();
@@ -123,8 +126,11 @@  fn (int c, int flags)
       _exit (0);
     }
 
-  /* In the parent.  */
-  read (pipefd[0], r, 1);
+  /* To avoid need to handle cancellation syscall backtrace (which call
+     the function using both the generic bridge (__syscall_cancel) and
+     the architecture defined one (__syscall_cancel_arch), issue the
+     syscall directly.  */
+  syscall (__NR_read, pipefd[0], r, 1);
 
   return 0;
 }
diff --git a/nptl/Makefile b/nptl/Makefile
index de37fb4..76ecf6c 100644
--- a/nptl/Makefile
+++ b/nptl/Makefile
@@ -452,8 +452,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
 
@@ -682,7 +681,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 \
@@ -694,18 +693,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 0c5b614..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-2018 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-cancel2.c b/nptl/tst-cancel2.c
index e2926bd..45e9a61 100644
--- a/nptl/tst-cancel2.c
+++ b/nptl/tst-cancel2.c
@@ -73,9 +73,6 @@  do_test (void)
       return 1;
     }
 
-  /* This will cause the write in the child to return.  */
-  close (fd[0]);
-
   if (pthread_join (th, &r) != 0)
     {
       puts ("join failed");
diff --git a/nptl/tst-cancel3.c b/nptl/tst-cancel3.c
index a82c8f2..67f6543 100644
--- a/nptl/tst-cancel3.c
+++ b/nptl/tst-cancel3.c
@@ -75,9 +75,6 @@  do_test (void)
       return 1;
     }
 
-  /* This will cause the read in the child to return.  */
-  close (fd[0]);
-
   if (pthread_join (th, &r) != 0)
     {
       puts ("join failed");
diff --git a/nptl/tst-cancel4.c b/nptl/tst-cancel4.c
index 0532538..2614f9a 100644
--- a/nptl/tst-cancel4.c
+++ b/nptl/tst-cancel4.c
@@ -166,6 +166,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);
 
@@ -743,6 +747,10 @@  tf_send (void *arg)
   char mem[WRITE_BUFFER_SIZE];
 
   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);