From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-3.4 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 14104 invoked from network); 3 Jul 2023 21:23:26 -0000 Received: from second.openwall.net (193.110.157.125) by inbox.vuxu.org with ESMTPUTF8; 3 Jul 2023 21:23:26 -0000 Received: (qmail 12097 invoked by uid 550); 3 Jul 2023 21:23:22 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 12053 invoked from network); 3 Jul 2023 21:23:21 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=inria.fr; s=dc; h=date:from:to:subject:in-reply-to:references:message-id: mime-version:content-transfer-encoding; bh=wCKvn2Bb1/ge6Ojv/HFccrtbhWaDr3qz/aa/BeU/k/I=; b=UJSM+9iuBvPHF1BNaqooUAjcS6p0fZElXGusMbgCGSzeUcZqKp3pCmJV Rs1cMVeP+yEMsBdiQ+XN+lB0Nt64bnf9DRuDyM3oG9XXBMtxeYcTp4p3p jnNTI1vkv7PigHcbavt4udheMiPSCN+vj4FaXoKUehOOGNtHNq0WDSNH9 Y=; Authentication-Results: mail2-relais-roc.national.inria.fr; dkim=none (message not signed) header.i=none; spf=SoftFail smtp.mailfrom=jens.gustedt@inria.fr; dmarc=fail (p=none dis=none) d=inria.fr X-IronPort-AV: E=Sophos;i="6.01,178,1684792800"; d="scan'208";a="115832328" Date: Mon, 03 Jul 2023 23:23:00 +0200 From: Jens Gustedt To: musl@lists.openwall.com User-Agent: K-9 Mail for Android In-Reply-To: <20230703195957.GZ4163@brightrain.aerifal.cx> References: <1688401586.hkqjuyrd3s.none.ref@localhost> <1688401586.hkqjuyrd3s.none@localhost> <20230703195957.GZ4163@brightrain.aerifal.cx> Message-ID: <52DE631F-9A10-42E5-A72B-9CD282EB61CB@inria.fr> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [musl] fix various warnings/theoretical UB Hello, Am 3=2E Juli 2023 21:59:57 MESZ schrieb Rich Felker : > On Mon, Jul 03, 2023 at 01:55:57PM -0400, Alex Xu (Hello71) wrote: > > See attached patches=2E >=20 > > From 978f2cded65ce73450277d3fde48f038b339d5f9 Mon Sep 17 00:00:00 2001 > > From: "Alex Xu (Hello71)" > > Date: Sun, 2 Jul 2023 20:28:23 -0400 > > Subject: [PATCH 1/4] volatile static -> static volatile > >=20 > > C11 6=2E11=2E5p1: > >=20 > > > The placement of a storage-class specifier other than at the > > > beginning of the declaration specifiers in a declaration is an > > > obsolescent feature=2E > >=20 > > gcc also warns about this=2E > > --- > > src/time/timer_create=2Ec | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > >=20 > > diff --git a/src/time/timer_create=2Ec b/src/time/timer_create=2Ec > > index cd32c945=2E=2E9216b3ab 100644 > > --- a/src/time/timer_create=2Ec > > +++ b/src/time/timer_create=2Ec > > @@ -61,7 +61,7 @@ static void *start(void *arg) > > =20 > > int timer_create(clockid_t clk, struct sigevent *restrict evp, timer_= t *restrict res) > > { > > - volatile static int init =3D 0; > > + static volatile int init =3D 0; > > pthread_t td; > > pthread_attr_t attr; > > int r; > > --=20 > > 2=2E41=2E0 >=20 > No objection to this change=2E It's contrary to usual style=2E I would s= ay > let's convert to pthread_once, but this code is slated for removal > anyway once signals are no longer used for SIGEV_THREAD timers=2E >=20 > > From b98f243e7921ddff6978ee9b0ce9f08efaa17951 Mon Sep 17 00:00:00 2001 > > From: "Alex Xu (Hello71)" > > Date: Sun, 2 Jul 2023 20:29:41 -0400 > > Subject: [PATCH 2/4] __year_to_secs: fix dangling pointer > >=20 > > C11 6=2E5=2E2=2E5p5: > >=20 > > > If the compound literal occurs outside the body of a function, the > > > object has static storage duration; otherwise, it has automatic > > > storage duration associated with the enclosing block=2E > >=20 > > gcc also warns about this=2E > > --- > > src/time/__year_to_secs=2Ec | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > >=20 > > diff --git a/src/time/__year_to_secs=2Ec b/src/time/__year_to_secs=2Ec > > index 2824ec6d=2E=2Ed215880a 100644 > > --- a/src/time/__year_to_secs=2Ec > > +++ b/src/time/__year_to_secs=2Ec > > @@ -10,9 +10,9 @@ long long __year_to_secs(long long year, int *is_lea= p) > > return 31536000*(y-70) + 86400*leaps; > > } > > =20 > > - int cycles, centuries, leaps, rem; > > + int cycles, centuries, leaps, rem, tmp; > > =20 > > - if (!is_leap) is_leap =3D &(int){0}; > > + if (!is_leap) is_leap =3D &tmp; > > cycles =3D (year-100) / 400; > > rem =3D (year-100) % 400; > > if (rem < 0) { > > --=20 > > 2=2E41=2E0 >=20 > Seems like a bogus warning=2E The enclosing block is the whole function, No, the `if` statement forms a block of itself, and then the dependent sta= tement forms yet another block=2E We rectify the terminology a bit in C23 hopefully make it easier to read w= ithout changing semantics=20 > the same as the lifetime of the pointer=2E This might merit > investigation on whether GCC is doing something wrong though=2E=2E >=20 > > From a30c4ab397af040d10d978d97dd4a6835d4b99a8 Mon Sep 17 00:00:00 2001 > > From: "Alex Xu (Hello71)" > > Date: Sun, 2 Jul 2023 20:54:45 -0400 > > Subject: [PATCH 3/4] fix mismatched VLA parameter types > >=20 > > gcc warns about this, and it's probably technically UB > > --- > > src/internal/procfdname=2Ec | 2 +- > > src/prng/seed48=2Ec | 2 +- > > 2 files changed, 2 insertions(+), 2 deletions(-) > >=20 > > diff --git a/src/internal/procfdname=2Ec b/src/internal/procfdname=2Ec > > index fd7306ab=2E=2Ebfa3e7e5 100644 > > --- a/src/internal/procfdname=2Ec > > +++ b/src/internal/procfdname=2Ec > > @@ -1,6 +1,6 @@ > > #include "syscall=2Eh" > > =20 > > -void __procfdname(char *buf, unsigned fd) > > +void __procfdname(char buf[static 15+3*sizeof(int)], unsigned fd) > > { > > unsigned i, j; > > for (i=3D0; (buf[i] =3D "/proc/self/fd/"[i]); i++); >=20 > This was raised/proposed before and is probably an okay change, but > I'd like to understand what the reason "it's probably technically UB" > is=2E >=20 > > diff --git a/src/prng/seed48=2Ec b/src/prng/seed48=2Ec > > index bce7b339=2E=2E7b789086 100644 > > --- a/src/prng/seed48=2Ec > > +++ b/src/prng/seed48=2Ec > > @@ -2,7 +2,7 @@ > > #include > > #include "rand48=2Eh" > > =20 > > -unsigned short *seed48(unsigned short *s) > > +unsigned short *seed48(unsigned short s[3]) > > { > > static unsigned short p[3]; > > memcpy(p, __seed48, sizeof p); > > --=20 >=20 > This one is almost surely not UB because there's no static and the 3 > is ignored=2E The question is just whether the static produces a > difference in the declaration type that makes them clash=2E >=20 > Rich Jens --=20 Jens Gustedt - INRIA & ICube, Strasbourg, France