From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id C582E3857713 for ; Mon, 6 Nov 2023 12:15:52 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org C582E3857713 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org C582E3857713 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1699272954; cv=none; b=j6jfGt0qNgmX7xLXhkJWY6jIOnAc1GfM77DTBdrNoDV40zEebab+PwZJYVKC0y72i16OoZn5GlX14rm3teY4wsnC0TiFfP1L++Xv1tFK9QUSiUQoYbDxOBRzpXmT/LvjJqsbgoL7QXTkFFHn0truYl+8RvwTOKPx+evenR5TyBo= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1699272954; c=relaxed/simple; bh=/v0951sH/svgJ0OPBhwJf6yaDsl8CLhkIO6YYU5OQVA=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=skzoxoCxCCmEdVo2ocrlbM6tXVZHtwlxa7dSuMPvIGRLntUOnHkhP2js2OWLag7ZdiwKF0arSd2pSXhlbObMatXmYLsIiNP+4MQcV+zaGDr/yxEfXfOP+x7iEw5VxDW7zD6KouPhVpMpTywdj7YkfMWWRxV9FpZXJuQA5piyzHI= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1699272952; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=mvB/J0voRZSdWuwnl/GOSnRIbDRN5FR6tWC09FZ8buI=; b=H0b2O5ANEooZHl9OnZY6sAbQ1LqVO8VzxNQWTkvxNSOR3a7obATYotuRctNKM2uDv31mAS ebKONBvDq/Ms9f9ofhMlIm2pqsd9B1PWMbFy/uvL9Dnbl2u7Ywn2nVApmcr9KH7BLlHRzc 3YIXK5gWyMyHTgLdlzZ6JCx6GUsyYD0= Received: from mail-yw1-f199.google.com (mail-yw1-f199.google.com [209.85.128.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-502-qiWMdQA5P8OycYaEFeAW3A-1; Mon, 06 Nov 2023 07:15:41 -0500 X-MC-Unique: qiWMdQA5P8OycYaEFeAW3A-1 Received: by mail-yw1-f199.google.com with SMTP id 00721157ae682-5afc00161daso49930137b3.1 for ; Mon, 06 Nov 2023 04:15:41 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1699272941; x=1699877741; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=mvB/J0voRZSdWuwnl/GOSnRIbDRN5FR6tWC09FZ8buI=; b=K1gh83LbjrPA4eHLYKOD12paLDjAlzmZqkFp93aL2pFElmLOZLhE9R22Eo708nBuW6 2I5IOQ6x4u7lm+lC37kOGUDs+Ji7XGPgx9LV3YKMSrsgCle4oCS4sgw//qUhywyEELNU 1dNyMjvlZNjA8hUyuf7+dToQV1XpH0glTTOd13FtvaGXJOlP9aZaSu1aFi7jS+j2peZH CGLZZGDF1Ibud/knIILLlThfaz3IezoTdLnscEefh1Tw3n8NeXIMiI2cQE8I1c3bAzKc ByljxhRXtErOHf+EtNUccLz+6oKLtbDvAo76MYwDD/fxVUOop6+bdNgC5t0WrUCjkegR Z+6Q== X-Gm-Message-State: AOJu0YxjkMF+upqNGPGbbsXT5Su0Ze9MW9v4ZwvI0vTJeGEu6nOxRpwd eh1pVlukjxi8f9EUwA+cOWH66O+gNLt3oW2r1ikPyzaq6whQhO3Xd7dpba9/F2s3GPAZ4Lod29U Vv0GB5i6BFdu09fMgYDzUYqlK3qZzu/s= X-Received: by 2002:a05:690c:15:b0:5b3:3eb5:6624 with SMTP id bc21-20020a05690c001500b005b33eb56624mr8452679ywb.46.1699272940889; Mon, 06 Nov 2023 04:15:40 -0800 (PST) X-Google-Smtp-Source: AGHT+IEHPrwjq8GBrXE8XsO2HD3dusZKto0IiBSTMsTIylBwVT5Y9P/Tc0W4uCmm+l3YVvgVNT7c82V1RVcHZpDF2CE= X-Received: by 2002:a05:690c:15:b0:5b3:3eb5:6624 with SMTP id bc21-20020a05690c001500b005b33eb56624mr8452433ywb.46.1699272935617; Mon, 06 Nov 2023 04:15:35 -0800 (PST) MIME-Version: 1.0 References: <11125.123110606520900751@us-mta-255.us.mimecast.lan> In-Reply-To: <11125.123110606520900751@us-mta-255.us.mimecast.lan> From: Jonathan Wakely Date: Mon, 6 Nov 2023 12:15:24 +0000 Message-ID: Subject: Re: [PATCH] libstdc++/112351 - deal with __gthread_once failure during locale init To: Richard Biener Cc: gcc-patches@gcc.gnu.org, libstdc++@gcc.gnu.org X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-12.2 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_NONE,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Mon, 6 Nov 2023 at 11:52, Richard Biener wrote: > > The following makes the C++98 locale init path follow the way the > C++11 performs initialization. This way we deal with pthread_once > failing, falling back to non-threadsafe initialization which, given we > initialize from the library, should be serialized by the dynamic > loader already. > > Bootstrapped and tested on x86_64-unknown-linux-gnu, OK for trunk? > And GCC 13 branch? > > Thanks, > Richard. > > PR libstdc++/112351 > libstdc++-v3/ > * src/c++98/locale.cc (locale::facet::_S_get_c_locale): > Always perform non-threadsafe init when threadsafe init > failed. > --- > libstdc++-v3/src/c++98/locale.cc | 7 ++----- > 1 file changed, 2 insertions(+), 5 deletions(-) > > diff --git a/libstdc++-v3/src/c++98/locale.cc b/libstdc++-v3/src/c++98/locale.cc > index d308140bab7..e9bec1db3b6 100644 > --- a/libstdc++-v3/src/c++98/locale.cc > +++ b/libstdc++-v3/src/c++98/locale.cc > @@ -216,12 +216,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > #ifdef __GTHREADS > if (__gthread_active_p()) > __gthread_once(&_S_once, _S_initialize_once); > - else > #endif > - { > - if (!_S_c_locale) > - _S_initialize_once(); > - } > + if (__builtin_expect (!_S_c_locale, 0)) > + _S_initialize_once(); > return _S_c_locale; > } I think this has a problem, which is handled correctly in src/c++11/locale_init.cc by checking _S_classic inside the _S_initialize_once function. If the first call to __gthread_once does fail then _S_once will not be changed. We will fall through to calling _S_initialize_once directly (which is not thread-safe) and set _S_c_locale. The next time we call _S_initialize, __gthread_once will try to run again, and because _S_once was not changed, it might call _S_initialize_once() again, which writes to _S_c_locale again (possibly causing a data race). I don't think the slightly different code in src/c++11/locale_init.cc is different in order to handle __gthread_once failing, I think it's different because the effects of locale::facet::_S_initialize_once() and locale::_S_initialize_once() are different. One is safe to call more than once, and the other isn't. I don't think we need to care about __gthread_once failing at all, do we? There are no error conditions for pthread_once, it always returns 0 (previous POSIX revisions said it could return EINVAL for an uninitialized pthread_once_t but that can't happen here as it's correctly initialized in src/c++11/locale.cc). Is the concern that it can fail for non-posix thread models? (I didn't check if any of them can actually fail)