commit f0419e6a10740a672b28e112c409ae24f5e890ab

Author: Jakub Jelinek <jakub@redhat.com>

Date:   Thu Mar 4 15:15:33 2021 +0100

    [PATCH] pthread_once hangs when init routine throws an exception [BZ #18435]

    This is another attempt at making pthread_once handle throwing exceptions

    from the init routine callback.  As the new testcases show, just switching

    to the cleanup attribute based cleanup does fix the tst-once5 test, but

    breaks the new tst-oncey3 test.  That is because when throwing exceptions,

    only the unwind info registered cleanups (i.e. C++ destructors or cleanup

    attribute), when cancelling threads and there has been unwind info from the

    cancellation point up to whatever needs cleanup both unwind info registered

    cleanups and THREAD_SETMEM (self, cleanup, ...) registered cleanups are

    invoked, but once we hit some frame with no unwind info, only the

    THREAD_SETMEM (self, cleanup, ...) registered cleanups are invoked.

    So, to stay fully backwards compatible (allow init routines without

    unwind info which encounter cancellation points) and handle exception throwing

    we actually need to register the pthread_once cleanups in both unwind info

    and in the THREAD_SETMEM (self, cleanup, ...) way.

    If an exception is thrown, only the former will happen and we in that case

    need to also unregister the THREAD_SETMEM (self, cleanup, ...) registered

    handler, because otherwise after catching the exception the user code could

    call deeper into the stack some cancellation point, get cancelled and then

    a stale cleanup handler would clobber stack and probably crash.

    If a thread calling init routine is cancelled and unwind info ends before

    the pthread_once frame, it will be cleaned up through self->cleanup as

    before.  And if unwind info is present, unwind_stop first calls the

    self->cleanup registered handler for the frame, then it will call the

    unwind info registered handler but that will already see __do_it == 0

    and do nothing.

# Conflicts:

#	nptl/Makefile

#	(The usual cleanups because they don't match.)

#	sysdeps/pthread/Makefile

#	(The usual cleanups because all the other tests aren't moved.)

diff --git a/nptl/Makefile b/nptl/Makefile

index dcf3868869767015..70a3be23ecfcd9c9 100644

--- a/nptl/Makefile

+++ b/nptl/Makefile

@@ -334,10 +334,6 @@ xtests = tst-setuid1 tst-setuid1-static tst-setuid2 \

 	tst-mutexpp1 tst-mutexpp6 tst-mutexpp10

 test-srcs = tst-oddstacklimit

-# Test expected to fail on most targets (except x86_64) due to bug

-# 18435 - pthread_once hangs when init routine throws an exception.

-test-xfail-tst-once5 = yes

-

 # Files which must not be linked with libpthread.

 tests-nolibpthread = tst-unload

diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h

index a2d48b2015cd385c..7ddc166cf32414c4 100644

--- a/nptl/pthreadP.h

+++ b/nptl/pthreadP.h

@@ -571,6 +571,67 @@ extern void __pthread_cleanup_pop (struct _pthread_cleanup_buffer *buffer,

 # undef pthread_cleanup_pop

 # define pthread_cleanup_pop(execute)                   \

   __pthread_cleanup_pop (&_buffer, (execute)); }

+

+# if defined __EXCEPTIONS && !defined __cplusplus

+/* Structure to hold the cleanup handler information.  */

+struct __pthread_cleanup_combined_frame

+{

+  void (*__cancel_routine) (void *);

+  void *__cancel_arg;

+  int __do_it;

+  struct _pthread_cleanup_buffer __buffer;

+};

+

+/* Special cleanup macros which register cleanup both using

+   __pthread_cleanup_{push,pop} and using cleanup attribute.  This is needed

+   for pthread_once, so that it supports both throwing exceptions from the

+   pthread_once callback (only cleanup attribute works there) and cancellation

+   of the thread running the callback if the callback or some routines it

+   calls don't have unwind information.  */

+

+static __always_inline void

+__pthread_cleanup_combined_routine (struct __pthread_cleanup_combined_frame

+				    *__frame)

+{

+  if (__frame->__do_it)

+    {

+      __frame->__cancel_routine (__frame->__cancel_arg);

+      __frame->__do_it = 0;

+      __pthread_cleanup_pop (&__frame->__buffer, 0);

+    }

+}

+

+static inline void

+__pthread_cleanup_combined_routine_voidptr (void *__arg)

+{

+  struct __pthread_cleanup_combined_frame *__frame

+    = (struct __pthread_cleanup_combined_frame *) __arg;

+  if (__frame->__do_it)

+    {

+      __frame->__cancel_routine (__frame->__cancel_arg);

+      __frame->__do_it = 0;

+    }

+}

+

+#  define pthread_cleanup_combined_push(routine, arg) \

+  do {									      \

+    void (*__cancel_routine) (void *) = (routine);			      \

+    struct __pthread_cleanup_combined_frame __clframe			      \

+      __attribute__ ((__cleanup__ (__pthread_cleanup_combined_routine)))      \

+      = { .__cancel_routine = __cancel_routine, .__cancel_arg = (arg),	      \

+	  .__do_it = 1 };						      \

+    __pthread_cleanup_push (&__clframe.__buffer,			      \

+			    __pthread_cleanup_combined_routine_voidptr,	      \

+			    &__clframe);

+

+#  define pthread_cleanup_combined_pop(execute) \

+    __pthread_cleanup_pop (&__clframe.__buffer, 0);			      \

+    __clframe.__do_it = 0;						      \

+    if (execute)							      \

+      __cancel_routine (__clframe.__cancel_arg);			      \

+  } while (0)

+

+# endif

 #endif

 extern void __pthread_cleanup_push_defer (struct _pthread_cleanup_buffer *buffer,

diff --git a/nptl/pthread_once.c b/nptl/pthread_once.c

index 1653226286dc3539..45e965e8743d9412 100644

--- a/nptl/pthread_once.c

+++ b/nptl/pthread_once.c

@@ -111,11 +111,11 @@ __pthread_once_slow (pthread_once_t *once_control, void (*init_routine) (void))

       /* This thread is the first here.  Do the initialization.

 	 Register a cleanup handler so that in case the thread gets

 	 interrupted the initialization can be restarted.  */

-      pthread_cleanup_push (clear_once_control, once_control);

+      pthread_cleanup_combined_push (clear_once_control, once_control);

       init_routine ();

-      pthread_cleanup_pop (0);

+      pthread_cleanup_combined_pop (0);

       /* Mark *once_control as having finished the initialization.  We need

diff --git a/nptl/tst-once5.cc b/nptl/tst-once5.cc

index d232266c3ace89d9..dda18e610c9114bc 100644

--- a/nptl/tst-once5.cc

+++ b/nptl/tst-once5.cc

@@ -59,7 +59,7 @@ do_test (void)

                " throwing an exception", stderr);

     }

     catch (OnceException) {

-      if (1 < niter)

+      if (niter > 1)

         fputs ("pthread_once unexpectedly threw", stderr);

       result = 0;

     }

@@ -75,7 +75,5 @@ do_test (void)

   return result;

 }

-// The test currently hangs and is XFAILed.  Reduce the timeout.

-#define TIMEOUT 1

 #define TEST_FUNCTION do_test ()

 #include "../test-skeleton.c"

diff --git a/sysdeps/pthread/Makefile b/sysdeps/pthread/Makefile

index 14ef04247cb84ad3..80a71f3f9f0e72ae 100644

--- a/sysdeps/pthread/Makefile

+++ b/sysdeps/pthread/Makefile

@@ -35,7 +35,7 @@ tst-create1mod.so-no-z-defs = yes

 tests += tst-once1 tst-once2 tst-once3 tst-once4

-tests += tst-oncex3 tst-oncex4

+tests += tst-oncex3 tst-oncex4 tst-oncey3 tst-oncey4

 ifeq ($(build-shared),yes)

 # Build all the modules even when not actually running test programs.

@@ -44,6 +44,8 @@ endif

 CFLAGS-tst-oncex3.c += -fexceptions

 CFLAGS-tst-oncex4.c += -fexceptions

+CFLAGS-tst-oncey3.c += -fno-exceptions -fno-asynchronous-unwind-tables

+CFLAGS-tst-oncey4.c += -fno-exceptions -fno-asynchronous-unwind-tables

 modules-names += tst-create1mod

 test-modules = $(addprefix $(objpfx),$(addsuffix .so,$(modules-names)))

diff --git a/sysdeps/pthread/tst-oncey3.c b/sysdeps/pthread/tst-oncey3.c

new file mode 100644

index 0000000000000000..08225b88dc06b979

--- /dev/null

+++ b/sysdeps/pthread/tst-oncey3.c

@@ -0,0 +1 @@

+#include "tst-once3.c"

diff --git a/sysdeps/pthread/tst-oncey4.c b/sysdeps/pthread/tst-oncey4.c

new file mode 100644

index 0000000000000000..9b4d98f3f13c265a

--- /dev/null

+++ b/sysdeps/pthread/tst-oncey4.c

@@ -0,0 +1 @@

+#include "tst-once4.c"