From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from esa3.mentor.iphmx.com (esa3.mentor.iphmx.com [68.232.137.180]) by sourceware.org (Postfix) with ESMTPS id C983E3858C3A for ; Tue, 9 Nov 2021 16:07:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C983E3858C3A Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=codesourcery.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mentor.com IronPort-SDR: Cftkp8iuet3vbBw5xRSXkQlENfq7atFOouMjV0qLJ2TZ5e0RP2Z7x0geVdRmVCxMtjEQmrJkeJ juRdfw4xVATnNiMhltITEaunLipzAB3NLxSu/FskP1enyQTEjgJJPj8ufJ3mPr/9H4zNArrrYO cKrRZeJJHm5t8JXGONDHnxuHLXMxWgXsbPG9pqWbZZaZXNFBC4Kz5K0ZXqkIzIwz0InKY2eMsy MOGS8JA7RN46yCyZDfGmuvq+AnIX39CFgCQ4DDIbqBqnx23+/4SON9toRgGSijC2RvsyjuHMHg WwMiBEDyr+ZaIpHyn68LWLO0 X-IronPort-AV: E=Sophos;i="5.87,220,1631606400"; d="scan'208,223";a="68095389" Received: from orw-gwy-02-in.mentorg.com ([192.94.38.167]) by esa3.mentor.iphmx.com with ESMTP; 09 Nov 2021 08:07:06 -0800 IronPort-SDR: K5Fb/ir2vIKgNJPq+UfVh95B73A2zLJtVXNN5lJTWWPqf1jGPRvoUlV+u0kMdnD64A/Pt/JWQe i9ohvQiDg4OC3uxDEv7OwNP/HHNPP71KIC4KE9tyVaId4QbuzDWQMClAoplyKIl9ZMcgL8QLjZ +P/gszktc0YEFw8WDCxX6M+ZK3ErN50YjQSI+dTngEFnFc3NvJ2l2dAKMUsFVVXxhUddqNotC9 NXqq1jBm+Z2JjhhS8mwMHgICzvZ3oyWRhj3t/QsCd/iaiUyAbvn2WXT+tmMogKorOI7exi7iKg JxQ= From: Thomas Schwinge To: Jakub Jelinek , CC: Tobias Burnus Subject: Re: [committed] openmp: Fix up strtoul and strtoull uses in libgomp In-Reply-To: <20211015144633.GF304296@tucnak> References: <20211015144633.GF304296@tucnak> User-Agent: Notmuch/0.29.3+94~g74c3f1b (https://notmuchmail.org) Emacs/27.1 (x86_64-pc-linux-gnu) Date: Tue, 9 Nov 2021 17:06:53 +0100 Message-ID: <87czn9xqhu.fsf@euler.schwinge.homeip.net> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Originating-IP: [137.202.0.90] X-ClientProxiedBy: SVR-IES-MBX-04.mgc.mentorg.com (139.181.222.4) To svr-ies-mbx-01.mgc.mentorg.com (139.181.222.1) X-Spam-Status: No, score=-12.1 required=5.0 tests=BAYES_00, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 09 Nov 2021 16:07:09 -0000 --=-=-= Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hi! On 2021-10-15T16:46:33+0200, Jakub Jelinek via Gcc-patches wrote: > also discovered that the hang was a result of making wrong assumptions > about strtoul/strtoull. (Also 'strtol'.) ;-) > All the uses were for portability setting > errno =3D 0 before the calls and treating non-zero errno after the call > as invalid input, but for the case where there are no valid digits at > all strtoul may set errno to EINVAL, but doesn't have to and with > glibc doesn't do that. So, this patch goes through all the strtoul calls > and next to errno !=3D 0 checks adds also endptr =3D=3D startptr check. > Haven't done it in places where we immediately reject strtoul returning 0 > the same as we reject errno !=3D 0, because strtoul must return 0 in the > case where it sets endptr to the start pointer. In some spots the code > was using errno =3D 0; x =3D strtoul (p, &p, 10); if (errno) { /*invalid*= / } > and those spots had to be changed to > errno =3D 0; x =3D strtoul (p, &end, 10); if (errno || end =3D=3D p) { /*= invalid*/ } > p =3D end; ACK. > Regtested on x86_64-linux and i686-linux, committed to trunk. Thanks for addressing that one, too -- but evidently not properly tested the one OpenACC change: > (parse_gomp_openacc_dim): Likewise. Avoid strict aliasing violatio= n. > Make code valid C89. (Why the C89 "re-formatting", by the way? Surely we're "violating" that in a lot of other places?) > --- libgomp/env.c.jj 2021-10-14 22:04:30.594333475 +0200 > +++ libgomp/env.c 2021-10-15 14:07:07.464919497 +0200 > @@ -1202,27 +1207,30 @@ parse_gomp_openacc_dim (void) > /* The syntax is the same as for the -fopenacc-dim compilation option.= */ > const char *var_name =3D "GOMP_OPENACC_DIM"; > const char *env_var =3D getenv (var_name); > + const char *pos =3D env_var; > + int i; > + > if (!env_var) > return; > > - const char *pos =3D env_var; > - int i; > for (i =3D 0; *pos && i !=3D GOMP_DIM_MAX; i++) > { > + char *eptr; > + long val; > + > if (i && *pos++ !=3D ':') > break; > > if (*pos =3D=3D ':') > continue; > > - const char *eptr; > errno =3D 0; > - long val =3D strtol (pos, (char **)&eptr, 10); > - if (errno || val < 0 || (unsigned)val !=3D val) > + val =3D strtol (pos, &eptr, 10); > + if (errno || eptr !=3D pos || val < 0 || (unsigned)val !=3D val) > break; Instead of 'eptr !=3D pos', this needs to be 'eptr =3D=3D pos', like everyw= here else. (That there are no diagnostics for malformed 'GOMP_OPENACC_DIM', is a different topic, of course.) > > goacc_default_dims[i] =3D (int)val; > - pos =3D eptr; > + pos =3D (const char *) eptr; > } > } Pushed to master branch commit 00c9ce13a64e324dabd8dfd236882919a3119479 "Restore 'GOMP_OPENACC_DIM' environment variable parsing", see attached. Gr=C3=BC=C3=9Fe Thomas ----------------- Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstra=C3=9Fe 201= , 80634 M=C3=BCnchen; Gesellschaft mit beschr=C3=A4nkter Haftung; Gesch=C3= =A4ftsf=C3=BChrer: Thomas Heurung, Frank Th=C3=BCrauf; Sitz der Gesellschaf= t: M=C3=BCnchen; Registergericht M=C3=BCnchen, HRB 106955 --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename="0001-Restore-GOMP_OPENACC_DIM-environment-variable-parsin.patch" >From 00c9ce13a64e324dabd8dfd236882919a3119479 Mon Sep 17 00:00:00 2001 From: Thomas Schwinge Date: Fri, 5 Nov 2021 14:42:21 +0100 Subject: [PATCH] Restore 'GOMP_OPENACC_DIM' environment variable parsing ... that got broken by recent commit c057ed9c52c6a63a1a692268f916b1a9131cd4b7 "openmp: Fix up strtoul and strtoull uses in libgomp", resulting in spurious FAILs for tests specifying 'dg-set-target-env-var "GOMP_OPENACC_DIM" "[...]"'. libgomp/ * env.c (parse_gomp_openacc_dim): Restore parsing. --- libgomp/env.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libgomp/env.c b/libgomp/env.c index df10ff656b6..75018e8c252 100644 --- a/libgomp/env.c +++ b/libgomp/env.c @@ -1243,7 +1243,7 @@ parse_gomp_openacc_dim (void) errno = 0; val = strtol (pos, &eptr, 10); - if (errno || eptr != pos || val < 0 || (unsigned)val != val) + if (errno || eptr == pos || val < 0 || (unsigned)val != val) break; goacc_default_dims[i] = (int)val; -- 2.33.0 --=-=-=--