[PATCH] v2 BZ# 18125: setcontext: Call exit, not _exit, after last linked context executes.

["Carlos O'Donell" <carlos at redhat dot com>]

On 03/13/2015 05:15 PM, Carlos O'Donell wrote:
> There appears to be a discrepancy among the implementations
> of setcontext with regards to the function called once the last
> linked-to context has finished executing via setcontext.
> 
> The POSIX standard says:
> ~~~
> If the uc_link member of the ucontext_t structure pointed to by 
> the ucp argument is equal to 0, then this context is the main 
> context, and the thread will exit when this context returns. 
> ~~~
> 
> It says "exit" not "exit immediately" nor "exit without running
> functions registered with atexit or on_exit."
> 
> Therefore the AArch64, ARM, hppa and NIOS II implementations are
> wrong and no test detects it.
> 
> It is questionable if this should even be fixed or just documented
> that the above 4 targets are wrong. The functions are deprecated
> and nobody should be using them, but at the same time it silly to
> have cross-target differences that make it hard to port old applications
> from say x86_64 to AArch64.
> 
> Therefore I'm inclined to fix the 4 arches, and checkin a regression
> test to prevent it from changing again.
> 
> Tested on x86_64 and hppa.
> 
> OK to commit?

Thanks Mike and Andreas for the review. It was certainly a sloppy
first version. Here's v2 with all the requested changes.

v2
- Use filename*
- Use const char buf[]
- Don't `return;' when it's pointless
- Remove pointless cast for `cf'
- Print PASS after checking for close failure
- Remove assignment inside condition `ret = setcontext(...)'
- Don't define TEST_FUNCTION.
- Quote tempfile (others don't need quoting).
- Don't call cleanup on exit since it's already done
- Filed Bug 18135 "setcontext should call phtread_exit after last context"
  for exit vs. pthread_exit issue, orthogonal to this issue
- Mention bug 18135 in the regression test.
- Renumbered regression test to to tst-setcontext3

Tested on x86_64 with no regressions.

OK to commit?

2015-03-16  Carlos O'Donell  <carlos@redhat.com>

	[BZ #18125]
	* stdlib/tst-setcontext3.c: New file.
	* stdlib/tst-setcontext3.sh: New file.
	* Makefile (tests): Add tst-setcontext3.
	(tst-setcontext3.out): Custom rule to run tst-setcontext3.sh
	to verify test program created output file.
	* sysdeps/unix/sysv/linux/aarch64/setcontext.S: Call exit.
	* sysdeps/unix/sysv/linux/arm/setcontext.S: Likewise.
	* sysdeps/unix/sysv/linux/hppa/setcontext.S: Likewise.
	* sysdeps/unix/sysv/linux/nios2/setcontext.S: Likewise.

diff --git a/stdlib/tst-setcontext3.c b/stdlib/tst-setcontext3.c
new file mode 100644
index 0000000..a9bdda9
--- /dev/null
+++ b/stdlib/tst-setcontext3.c
@@ -0,0 +1,138 @@
+/* Bug 18125: Verify setcontext calls exit() and not _exit().
+   Copyright (C) 2015 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   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/>.  */
+
+#include <errno.h>
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <ucontext.h>
+#include <unistd.h>
+#include <limits.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+
+/* Please note that depending on the outcome of Bug 18135 this test
+   may become invalid, and instead of testing for calling exit it
+   should be reworked to test for the last context calling
+   pthread_exit().  */
+
+static ucontext_t ctx;
+char *filename;
+
+/* It is intended that this function does nothing.  */
+static void
+cf (void)
+{
+  printf ("called context function\n");
+}
+
+static void
+exit_called (void)
+{
+  int fd;
+  ssize_t res;
+  const char buf[] = "Called exit function\n";
+
+  fd = open (filename, O_WRONLY | O_CREAT, S_IRUSR | S_IWUSR);
+  if (fd == -1)
+    {
+      printf ("FAIL: Unable to create test file %s\n", filename);
+      exit (1);
+    }
+  res = write (fd, buf, sizeof (buf));
+  if (res != sizeof (buf))
+    {
+      printf ("FAIL: Expected to write test file in one write call.\n");
+      exit (1);
+    }
+  res = close (fd);
+  if (res == -1)
+    {
+      printf ("FAIL: Failed to close test file.\n");
+      exit (1);
+    }
+  printf ("PASS: %s", buf);
+}
+
+/* The test expects a filename given by the wrapper calling script.
+   The test then registers an atexit handler that will create the
+   file to indicate that the atexit handler ran. Then the test
+   creates a context, modifies it with makecontext, and sets it.
+   The context has only a single context which then must exit.
+   If it incorrectly exits via _exit then the atexit handler is
+   not run, the file is not created, and the wrapper detects this
+   and fails the test.  This test cannot be done using an _exit
+   interposer since setcontext avoids the PLT and calls _exit
+   directly.  */
+static int
+do_test (int argc, char **argv)
+{
+  int ret;
+  char st1[32768];
+  ucontext_t tempctx = ctx;
+
+  if (argc < 2)
+    {
+      printf ("FAIL: Test missing filename argument.\n");
+      exit (1);
+    }
+
+  filename = argv[1];
+
+  atexit (exit_called);
+
+  puts ("making contexts");
+  if (getcontext (&ctx) != 0)
+    {
+      if (errno == ENOSYS)
+	{
+	  /* Exit with 77 to mark the test as UNSUPPORTED.  */
+	  printf ("UNSUPPORTED: getcontext not implemented.\n");
+	  exit (77);
+	}
+
+      printf ("FAIL: getcontext failed.\n");
+      exit (1);
+    }
+
+  ctx.uc_stack.ss_sp = st1;
+  ctx.uc_stack.ss_size = sizeof (st1);
+  ctx.uc_link = 0;
+  makecontext (&ctx, cf, 0);
+
+  /* Without this check, a stub makecontext can make us spin forever.  */
+  if (memcmp (&tempctx, &ctx, sizeof ctx) == 0)
+    {
+      puts ("UNSUPPORTED: makecontext was a no-op, presuming not implemented");
+      return 77;
+    }
+
+  ret = setcontext (&ctx);
+  if (ret != 0)
+    {
+      printf ("FAIL: setcontext returned with %d and errno of %d.\n", ret, errno);
+      exit (1);
+    }
+
+  printf ("FAIL: Impossibly returned to main.\n");
+  return 1;
+}
+
+#include "../test-skeleton.c"
diff --git a/stdlib/tst-setcontext3.sh b/stdlib/tst-setcontext3.sh
new file mode 100644
index 0000000..3aa3425
--- /dev/null
+++ b/stdlib/tst-setcontext3.sh
@@ -0,0 +1,54 @@
+#! /bin/bash
+# Bug 18125: Test the exit functionality of setcontext().
+# Copyright (C) 2015 Free Software Foundation, Inc.
+# This file is part of the GNU C Library.
+
+# 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/>.
+
+set -e
+
+common_objpfx=$1
+test_program_prefix_before_env=$2
+run_program_env=$3
+test_program_prefix_after_env=$4
+objpfx=$5
+
+test_pre="${test_program_prefix_before_env} ${run_program_env}"
+test="${test_program_prefix_after_env} ${objpfx}tst-setcontext2"
+out=${objpfx}tst-setcontext2.out
+
+tempfiles=()
+cleanup() {
+    rm -f "${tempfiles[@]}"
+}
+trap cleanup 0
+
+tempfile=$(mktemp "tst-setcontext2.XXXXXXXXXX")
+tempfiles+=("$tempfile")
+
+# We want to run the test program and see if secontext called
+# exit() and wrote out the test file we specified.  If the
+# test exits with a non-zero status this will fail because we
+# are using `set -e`.
+$test_pre $test "$tempfile"
+
+# Look for resulting file.
+if [ -e "$tempfile" ]; then
+  echo "PASS: tst-setcontext2 ran exit() and created $tempfile"
+  exit 0
+else
+  echo "FAIL: tst-setcontext2 did not create $tempfile"
+  exit 1
+fi
diff --git a/sysdeps/unix/sysv/linux/aarch64/setcontext.S b/sysdeps/unix/sysv/linux/aarch64/setcontext.S
index 6dd7836..ae67581 100644
--- a/sysdeps/unix/sysv/linux/aarch64/setcontext.S
+++ b/sysdeps/unix/sysv/linux/aarch64/setcontext.S
@@ -125,5 +125,5 @@ weak_alias (__setcontext, setcontext)
 ENTRY (__startcontext)
 	mov	x0, x19
 	cbnz	x0, __setcontext
-1:	b       HIDDEN_JUMPTARGET (_exit)
+1:	b       HIDDEN_JUMPTARGET (exit)
 END (__startcontext)
diff --git a/sysdeps/unix/sysv/linux/arm/setcontext.S b/sysdeps/unix/sysv/linux/arm/setcontext.S
index 5268e06..24c7294 100644
--- a/sysdeps/unix/sysv/linux/arm/setcontext.S
+++ b/sysdeps/unix/sysv/linux/arm/setcontext.S
@@ -91,7 +91,7 @@ ENTRY(__startcontext)
 	bne     PLTJMP(__setcontext)
 
 	@ New context was 0 - exit
-	b       PLTJMP(HIDDEN_JUMPTARGET(_exit))
+	b       PLTJMP(HIDDEN_JUMPTARGET(exit))
 END(__startcontext)
 
 #ifdef PIC
diff --git a/sysdeps/unix/sysv/linux/hppa/setcontext.S b/sysdeps/unix/sysv/linux/hppa/setcontext.S
index 7271410..abe87a9 100644
--- a/sysdeps/unix/sysv/linux/hppa/setcontext.S
+++ b/sysdeps/unix/sysv/linux/hppa/setcontext.S
@@ -18,6 +18,7 @@
    <http://www.gnu.org/licenses/>.  */
 
 #include <sysdep.h>
+#include <libc-symbols.h>
 
 #include "ucontext_i.h"
 
@@ -139,7 +140,7 @@ ENTRY(__setcontext)
 	nop
 
 	/* No further context available. Exit now.  */
-	bl	_exit, %r2
+	bl	HIDDEN_JUMPTARGET(exit), %r2
 	ldi	-1, %r26
 
 
diff --git a/sysdeps/unix/sysv/linux/nios2/setcontext.S b/sysdeps/unix/sysv/linux/nios2/setcontext.S
index 9a8dd87..f40b733 100644
--- a/sysdeps/unix/sysv/linux/nios2/setcontext.S
+++ b/sysdeps/unix/sysv/linux/nios2/setcontext.S
@@ -89,15 +89,15 @@ ENTRY(__startcontext)
 	mov	r4, r16
 	bne	r4, zero, __setcontext
 
-	/* If uc_link == zero, call _exit.  */
+	/* If uc_link == zero, call exit.  */
 #ifdef PIC
 	nextpc	r22
 1:	movhi	r8, %hiadj(_gp_got - 1b)
 	addi	r8, r8, %lo(_gp_got - 1b)
 	add	r22, r22, r8
-	ldw	r8, %call(HIDDEN_JUMPTARGET(_exit))(r22)
+	ldw	r8, %call(HIDDEN_JUMPTARGET(exit))(r22)
 	jmp	r8
 #else
-	jmpi	_exit
+	jmpi	exit
 #endif
 END(__startcontext)
---

Cheers,
Carlos.