From nobody Wed Feb 21 00:51:04 2024 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4Tfd4k3nNhz5B1mJ for ; Wed, 21 Feb 2024 00:51:18 +0000 (UTC) (envelope-from jrtc27@jrtc27.com) Received: from mail-wr1-f52.google.com (mail-wr1-f52.google.com [209.85.221.52]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1D4" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Tfd4k0JZ4z4GWm for ; Wed, 21 Feb 2024 00:51:18 +0000 (UTC) (envelope-from jrtc27@jrtc27.com) Authentication-Results: mx1.freebsd.org; none Received: by mail-wr1-f52.google.com with SMTP id ffacd0b85a97d-33d61d51dd1so1120169f8f.3 for ; Tue, 20 Feb 2024 16:51:18 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1708476676; x=1709081476; h=to:references:message-id:content-transfer-encoding:cc:date :in-reply-to:from:subject:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=Py4Ty/VpEhx4nXvdO28WlCgbeiag7O2vbM3x+QGc7ig=; b=qg2o9+7jWcw9c67nEXzKsmKaGTrWyXFibgQSTNRic2cIVyw47NKqvp9qHwptmwd9zj q82By3GiFZz4/DGUw08e+mQgC1FTL5fsZtZ9lquEkx00eGGprQXg/A+uyvqdkiGV/r0l dd7ETl3J3kkrfsJhFU8BFWdGKuz7ib2vRekfToaYoS7nHeg026oNce1ktt3wpAyZyLm8 aggaeEIX/ZjF/zBJaTbIBdJ8kPP8mXABLAC2MO6jxvEm9PaP+H3wVr6pO4gmvhnJBqg4 sHHSlQ+l1CtHFVgmwU503rRHkjfVzcxF/ZZ6j/lu3fTO3C1vveM1Rt93t0tQZHztpryR /YLw== X-Forwarded-Encrypted: i=1; AJvYcCUijSbw4/D3CKO8gEdgbJGXGO2c3KDXizWJMuNmdll2PfmXJAEY+hQNxhEwKilHwMI0F+hbvge0wNOhy3ur2LezC0sHrio+xxD9PZ4vZ3zI X-Gm-Message-State: AOJu0YyVb+O+lWQWHh8D1RWfq84EtgKtJ3opR9TgbP7ZLANEbhgQo/V6 M8/Md2ViUNHuUDLefZMikOtsapqixxcRPnjQoZK8YKcnmvtIE7CxSswLlH+rFRCyVSSEufrvrRx G X-Google-Smtp-Source: AGHT+IEyv6WUEujAyhIcyWUoGGxuuwwXKrXObywKVyLqcgbpkHYLaAERKK9fECV40IUsA8CG7nqnVw== X-Received: by 2002:a05:6000:3:b0:33d:1cc1:bb08 with SMTP id h3-20020a056000000300b0033d1cc1bb08mr9447690wrx.22.1708476675827; Tue, 20 Feb 2024 16:51:15 -0800 (PST) Received: from smtpclient.apple ([131.111.5.246]) by smtp.gmail.com with ESMTPSA id r6-20020adff706000000b0033cf7eb4a85sm14992783wrp.65.2024.02.20.16.51.15 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Tue, 20 Feb 2024 16:51:15 -0800 (PST) Content-Type: text/plain; charset=utf-8 List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3774.200.91.1.1\)) Subject: Re: git: 8271d9b99a3b - main - libsys: remove usage of pthread_once and _once_stub From: Jessica Clarke In-Reply-To: <202402210029.41L0TOH5000231@gitrepo.freebsd.org> Date: Wed, 21 Feb 2024 00:51:04 +0000 Cc: "src-committers@freebsd.org" , "dev-commits-src-all@freebsd.org" , "dev-commits-src-main@freebsd.org" Content-Transfer-Encoding: quoted-printable Message-Id: <964A29A2-4C51-4037-8EBE-320008D48AE0@freebsd.org> References: <202402210029.41L0TOH5000231@gitrepo.freebsd.org> To: Konstantin Belousov X-Mailer: Apple Mail (2.3774.200.91.1.1) X-Spamd-Bar: ---- X-Rspamd-Queue-Id: 4Tfd4k0JZ4z4GWm X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:15169, ipnet:209.85.128.0/17, country:US] On 21 Feb 2024, at 00:29, Konstantin Belousov wrote: >=20 > The branch main has been updated by kib: >=20 > URL: = https://cgit.FreeBSD.org/src/commit/?id=3D8271d9b99a3b98c662ee9a6257a14428= 4b7e1728 >=20 > commit 8271d9b99a3b98c662ee9a6257a144284b7e1728 > Author: Konstantin Belousov > AuthorDate: 2024-02-20 14:45:29 +0000 > Commit: Konstantin Belousov > CommitDate: 2024-02-21 00:26:11 +0000 >=20 > libsys: remove usage of pthread_once and _once_stub >=20 > that existed in auxv.c, use simple bool gate instead. This leaves a > small window if two threads try to call _elf_aux_info(3) = simultaneously. > The situation is safe because auxv parsing is really idempotent. = The > parsed data is the same, and we store atomic types (int/long/ptr) = so > double-init does not matter. You still need to load acquire and store release aux_once though, otherwise you can see aux_once as true yet read the pre-initialised data. In practice that=E2=80=99s surely very hard to hit, but the code = as written is now wrong. Also, idempotence should probably be made unnecessary by using 0/1/2 state for uninitialised/initialising/ initialised, as it=E2=80=99s still technically UB from a C AM = perspective due to not being data race free if two threads initialise at the same time. Better to just do the correct thing rather than risk things going wrong. Jess > Reviewed by: brooks, imp > Sponsored by: The FreeBSD Foundation > Differential revision: https://reviews.freebsd.org/D43985 > --- > lib/libc/gen/Makefile.inc | 1 + > lib/{libsys =3D> libc/gen}/_once_stub.c | 0 > lib/libsys/Makefile.sys | 1 - > lib/libsys/auxv.c | 20 ++++++++++++++++++-- > 4 files changed, 19 insertions(+), 3 deletions(-) >=20 > diff --git a/lib/libc/gen/Makefile.inc b/lib/libc/gen/Makefile.inc > index 8d30e06cfed9..ce7a34d0e58e 100644 > --- a/lib/libc/gen/Makefile.inc > +++ b/lib/libc/gen/Makefile.inc > @@ -9,6 +9,7 @@ CONFSPACKAGE=3D runtime > SRCS+=3D \ > __pthread_mutex_init_calloc_cb_stub.c \ > __xuname.c \ > + _once_stub.c \ > _pthread_stubs.c \ > _rand48.c \ > _spinlock_stub.c \ > diff --git a/lib/libsys/_once_stub.c b/lib/libc/gen/_once_stub.c > similarity index 100% > rename from lib/libsys/_once_stub.c > rename to lib/libc/gen/_once_stub.c > diff --git a/lib/libsys/Makefile.sys b/lib/libsys/Makefile.sys > index cb9ca1749ba8..e33a11bacb57 100644 > --- a/lib/libsys/Makefile.sys > +++ b/lib/libsys/Makefile.sys > @@ -33,7 +33,6 @@ PSEUDO+=3D _clock_gettime.o _gettimeofday.o > SRCS+=3D \ > __error.c \ > __getosreldate.c \ > - _once_stub.c \ > getpagesize.c \ > getpagesizes.c \ > interposing_table.c > diff --git a/lib/libsys/auxv.c b/lib/libsys/auxv.c > index b0b3a8ed708b..88f49ef53be1 100644 > --- a/lib/libsys/auxv.c > +++ b/lib/libsys/auxv.c > @@ -31,6 +31,7 @@ > #include > #include > #include > +#include > #include > #include > #include "un-namespace.h" > @@ -40,6 +41,8 @@ extern int _DYNAMIC; > #pragma weak _DYNAMIC >=20 > void *__elf_aux_vector; > + > +#ifndef PIC > static pthread_once_t aux_vector_once =3D PTHREAD_ONCE_INIT; >=20 > static void > @@ -61,8 +64,9 @@ __init_elf_aux_vector(void) > return; > _once(&aux_vector_once, init_aux_vector_once); > } > +#endif >=20 > -static pthread_once_t aux_once =3D PTHREAD_ONCE_INIT; > +static bool aux_once =3D false; > static int pagesize, osreldate, canary_len, ncpus, pagesizes_len, = bsdflags; > static int hwcap_present, hwcap2_present; > static char *canary, *pagesizes, *execpath; > @@ -77,11 +81,19 @@ static void _init_aux_powerpc_fixup(void); > int _powerpc_elf_aux_info(int, void *, int); > #endif >=20 > +/* > + * This function might be called and actual body executed more than > + * once in multithreading environment. Due to this, it is and must > + * continue to be idempotent. All stores are atomic (no store > + * tearing), because we only assign to int/long/ptr. > + */ > static void > init_aux(void) > { > Elf_Auxinfo *aux; >=20 > + if (aux_once) > + return; > for (aux =3D __elf_aux_vector; aux->a_type !=3D AT_NULL; aux++) { > switch (aux->a_type) { > case AT_BSDFLAGS: > @@ -166,6 +178,8 @@ init_aux(void) > if (!powerpc_new_auxv_format) > _init_aux_powerpc_fixup(); > #endif > + > + aux_once =3D true; > } >=20 > #ifdef __powerpc__ > @@ -256,10 +270,12 @@ _elf_aux_info(int aux, void *buf, int buflen) > { > int res; >=20 > +#ifndef PIC > __init_elf_aux_vector(); > +#endif > if (__elf_aux_vector =3D=3D NULL) > return (ENOSYS); > - _once(&aux_once, init_aux); > + init_aux(); /* idempotent */ >=20 > if (buflen < 0) > return (EINVAL);