From patchwork Wed Mar 3 12:52:27 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Jelinek X-Patchwork-Id: 1446621 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=sourceware.org (client-ip=8.43.85.97; helo=sourceware.org; envelope-from=libc-alpha-bounces@sourceware.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=oJoVuir/; dkim-atps=neutral Received: from sourceware.org (unknown [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4DrDRv1WQmz9sPf for ; Wed, 3 Mar 2021 23:52:43 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 2E23A3850419; Wed, 3 Mar 2021 12:52:36 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 2E23A3850419 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1614775956; bh=SFzNTSaFSchwGYT71T1iCJV0vrGN0enMNjQ+gyEslxA=; h=Date:To:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=oJoVuir/6b1Hjt+A1hiYHavUmeKES3hV7RFX9wUhCjVFcXZxwjnTEW6OIQUkDR4qf Uwr+StLc46juTG+s9HPmsHdqfiOlNRJLfD0Ld7s330siiL+v/jNEbobe4XppQxJ9FJ hVQOclp0f8eLpRlpEoF894RFtdhdF1Vbc2LFSAHc= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id 08B6D3851C1C for ; Wed, 3 Mar 2021 12:52:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 08B6D3851C1C Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-122-LmSEWOIIM_O_W2JF5zY_3Q-1; Wed, 03 Mar 2021 07:52:31 -0500 X-MC-Unique: LmSEWOIIM_O_W2JF5zY_3Q-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id AB616100A8EC for ; Wed, 3 Mar 2021 12:52:30 +0000 (UTC) Received: from tucnak.zalov.cz (ovpn-112-197.ams2.redhat.com [10.36.112.197]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 567B819C48 for ; Wed, 3 Mar 2021 12:52:30 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.16.1/8.16.1) with ESMTPS id 123CqR7r3886633 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT) for ; Wed, 3 Mar 2021 13:52:27 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.16.1/8.16.1/Submit) id 123CqRs93886632 for libc-alpha@sourceware.org; Wed, 3 Mar 2021 13:52:27 +0100 Date: Wed, 3 Mar 2021 13:52:27 +0100 To: libc-alpha@sourceware.org Subject: [PATCH] pthread_once hangs when init routine throws an exception [BZ #18435] Message-ID: <20210303125227.GO3748@tucnak> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Disposition: inline X-Spam-Status: No, score=-12.0 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Jakub Jelinek via Libc-alpha From: Jakub Jelinek Reply-To: Jakub Jelinek Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" 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. Jakub diff --git a/nptl/Makefile b/nptl/Makefile index 5f85dd7854..33766eaf7a 100644 --- a/nptl/Makefile +++ b/nptl/Makefile @@ -386,10 +386,6 @@ xtests += tst-eintr1 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 - gen-as-const-headers = unwindbuf.sym \ pthread-pi-defines.sym diff --git a/nptl/cleanup_compat.c b/nptl/cleanup_compat.c index fec88c2f86..b5b848338a 100644 --- a/nptl/cleanup_compat.c +++ b/nptl/cleanup_compat.c @@ -48,3 +48,11 @@ _pthread_cleanup_pop (struct _pthread_cleanup_buffer *buffer, int execute) buffer->__routine (buffer->__arg); } strong_alias (_pthread_cleanup_pop, __pthread_cleanup_pop) + +void +__pthread_cleanup_cleanup (struct _pthread_cleanup_buffer *buffer) +{ + struct pthread *self = THREAD_SELF; + if (THREAD_GETMEM (self, cleanup) == buffer) + THREAD_SETMEM (self, cleanup, buffer->__prev); +} diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h index d2fd0826fe..297426be1e 100644 --- a/nptl/pthreadP.h +++ b/nptl/pthreadP.h @@ -604,6 +604,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)); } + +extern void __pthread_cleanup_cleanup (struct _pthread_cleanup_buffer *buffer) + attribute_hidden; + +# if defined __GNUC__ && 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 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_cleanup (&__frame->__buffer); + } +} + +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 { \ + struct __pthread_cleanup_combined_frame __clframe \ + __attribute__ ((__cleanup__ (__pthread_cleanup_combined_routine))) \ + = { .__cancel_routine = (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 = (execute); \ + } 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 28d97097c7..7645da222a 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 b797ab3562..60fe1ef820 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 eeb64f9fb0..53b65ef349 100644 --- a/sysdeps/pthread/Makefile +++ b/sysdeps/pthread/Makefile @@ -231,7 +231,7 @@ generated += $(objpfx)tst-atfork2.mtrace \ tests-internal += tst-cancel25 tst-robust8 -tests += tst-oncex3 tst-oncex4 +tests += tst-oncex3 tst-oncex4 tst-oncey3 tst-oncey4 modules-names += tst-join7mod @@ -242,6 +242,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 $(objpfx)tst-join7: $(libdl) $(shared-thread-library) $(objpfx)tst-join7.out: $(objpfx)tst-join7mod.so