From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ej1-x629.google.com (mail-ej1-x629.google.com [IPv6:2a00:1450:4864:20::629]) by sourceware.org (Postfix) with ESMTPS id 9E53E3858403 for ; Sat, 3 Feb 2024 13:29:52 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 9E53E3858403 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 9E53E3858403 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a00:1450:4864:20::629 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1706966994; cv=none; b=sUcTwf2u83r61h0rGgSwsj7BDzIxZKfu5V1aeFXg8JuzRip9MmI6/oyZSomTXiY+KDQJ/TxGBBW4xfgVSKRWqopiFoWlmz5YSs1k6l+6dpFv768NuteRDBNM2fbY8TmDQRssyDP68su3+Hiqhzs0TXj5ACdVUGP9e4ELzfRYYog= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1706966994; c=relaxed/simple; bh=BmZN30W2SbtZyDQs/cYF/VO07lMQ+HF/PHfGmMbHaa4=; h=DKIM-Signature:From:Mime-Version:Subject:Date:Message-Id:To; b=O8nsRHE5EIx02wNecptzg/0NPiAWvzdQh52DSzhbJ3OnemoJERADboAh2yZb/0aLYISDJWGvbCwOrwBOTpASdaD+cxOYpiZ+r3K/RZ7bpv9Cf4XZelIpD+PB4ab4oNNbBs7/31K+lerYsTSdD0AAEbNOL9AKFA/bMllmbVy/Gys= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-ej1-x629.google.com with SMTP id a640c23a62f3a-a370e63835cso182251866b.1 for ; Sat, 03 Feb 2024 05:29:52 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1706966991; x=1707571791; darn=gcc.gnu.org; h=to:in-reply-to:cc:references:message-id:date:subject:mime-version :from:content-transfer-encoding:from:to:cc:subject:date:message-id :reply-to; bh=xoR3sz+MhY4/Xv2tkwfPccZqVu62sxH3I+DAoFKinIs=; b=lrUgbBDe/mQlFQRPil1G2RO0huRzwCsyls6WDSOYsBgnGLDeoRXV5R/3KUWeiHaOe7 8EYYlZgT4cDppcegqw7/eYW3JdLIffBmVSPiE9oAMY9Dv/ns9qGUBIbsSDTEnE9wixDM cBFwKtNsMT6lvTWUrkgObxYHUAo0OOZXCaMUnyxvnO8c3xw9H6027NcNAQfExa9Ez/wy cElTkZgQ0+6vJPQHHND+8SrbSDJ9EAGkr9orc5ij7lanL85VlUcopeiWCZiHiheBjJKC rPXYR+yImG0L0PhpR2zGyidWTQRLy9g4prC/wP6rMd/2wpnOHLNQX1VFVbUWMK6zETwi wjLQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1706966991; x=1707571791; h=to:in-reply-to:cc:references:message-id:date:subject:mime-version :from:content-transfer-encoding:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=xoR3sz+MhY4/Xv2tkwfPccZqVu62sxH3I+DAoFKinIs=; b=Tuevy6h8yGz7pb+ZqhrSjCUG60fVzuZLCnGLIrMF5ijMA2IYuluKB/7yxoVQrsOiBi 5bGRhg3EUrGC2B/YK+g8r0G9sTQkryBghy1GqFvmxOzflqYcAPdBSXaxZhq5ia8Ex30K I1nlueE/r/Z9lxkAKIelwl5TfiMPHeOqYHP/9StWqwuqg9djBKN7DhQmQV50SodHdvK1 bUlpA5fphkawovn541QONgVyFwzabmacBWoL/tPlHRKcKlKmVbOMdt9Cyjwbbi9yNwwV Rl1qFqMCY84FPwjsW6dF5s0dFQgiGYm3KEVuc6QfVe2qQ1kwwB4782PS3CFbxBK4UXHf 51Jw== X-Gm-Message-State: AOJu0YxsZVrhQ2xgzLBhgU5V5Pif9HSL+tjhyuyZmJ7Ur30Bi+7+kt9Z Niywwf4qD5x9krNqji2WFAe89VYLIReIbNhU1TLVR9jmeeFxpxfU X-Google-Smtp-Source: AGHT+IFB4oAkPuDK0c1QPC5Wx3Q4wNz9Ga3cMrhVZXWGUmXcBt3OrmGVk7zdano+OidGgb5C65Q4Tg== X-Received: by 2002:a17:906:a295:b0:a27:6e73:a248 with SMTP id i21-20020a170906a29500b00a276e73a248mr6333203ejz.68.1706966991049; Sat, 03 Feb 2024 05:29:51 -0800 (PST) X-Forwarded-Encrypted: i=0; AJvYcCVn/oLwHouVX2zp5Xm2P0An1K23eXwyxYVHJJq9uWAjbJnZmXU5QWxQHNHnwDgw87rdUJPuQ5APiVBirWArf+s3SWWwwR33GlItiUR2kbIIsdlylhJ4NiWQNdhkOOotkYmPPw== Received: from smtpclient.apple (dynamic-095-118-110-069.95.118.pool.telefonica.de. [95.118.110.69]) by smtp.gmail.com with ESMTPSA id x12-20020a1709064bcc00b00a35bfc2d570sm2016108ejv.6.2024.02.03.05.29.50 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 03 Feb 2024 05:29:50 -0800 (PST) Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable From: Richard Biener Mime-Version: 1.0 (1.0) Subject: Re: [PATCH] wide-int: Fix up wi::bswap_large [PR113722] Date: Sat, 3 Feb 2024 14:29:39 +0100 Message-Id: <4D9EEF67-E733-4EE9-B7EB-0D4614194EE9@gmail.com> References: Cc: Richard Biener , richard.sandiford@arm.com, gcc-patches@gcc.gnu.org In-Reply-To: To: Jakub Jelinek X-Mailer: iPhone Mail (21D50) X-Spam-Status: No, score=-3.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: > Am 03.02.2024 um 09:46 schrieb Jakub Jelinek : >=20 > =EF=BB=BFHi! >=20 > Since bswap has been converted from a method to a function we miscompile > the following testcase. The problem is the assumption that the passed in > len argument (number of limbs in the xval array) is the upper bound for th= e > bswap result, which is true only if precision is <=3D 64. If precision is= > larger than that, e.g. 128 as in the testcase, if the argument has only > one limb (i.e. 0 to ~(unsigned HOST_WIDE_INT) 0), the result can still > need 2 limbs for that precision, or generally BLOCKS_NEEDED (precision) > limbs, it all depends on how many least significant limbs of the operand > are zero. bswap_large as implemented only cleared len limbs of result, > then swapped the bytes (invoking UB when oring something in all the limbs > above it) and finally passed len to canonize, saying that more limbs > aren't needed. >=20 > The following patch fixes it by renaming len to xlen (so that it is clear > it is X's length), using it solely for safe_uhwi argument when we attempt > to read from X, and using new len =3D BLOCKS_NEEDED (precision) instead in= > the other two spots (i.e. when clearing the val array, turned it also > into memset, and in canonize argument). wi::bswap asserts it isn't invoke= d > on widest_int, so we are always invoked on wide_int or similar and those > have preallocated result sized for the corresponding precision (i.e. > BLOCKS_NEEDED (precision)). >=20 > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? Ok Richard=20 > 2024-02-03 Jakub Jelinek >=20 > PR middle-end/113722 > * wide-int.cc (wi::bswap_large): Rename third argument from > len to xlen and adjust use in safe_uhwi. Add len variable, set > it to BLOCKS_NEEDED (precision) and use it for clearing of val > and as canonize argument. Clear val using memset instead of > a loop. > =20 > * gcc.dg/pr113722.c: New test. >=20 > --- gcc/wide-int.cc.jj 2024-01-03 11:51:42.077584823 +0100 > +++ gcc/wide-int.cc 2024-02-02 18:13:34.993332159 +0100 > @@ -729,20 +729,19 @@ wi::set_bit_large (HOST_WIDE_INT *val, c > } > } >=20 > -/* Byte swap the integer represented by XVAL and LEN into VAL. Return > +/* Byte swap the integer represented by XVAL and XLEN into VAL. Return > the number of blocks in VAL. Both XVAL and VAL have PRECISION bits. *= / > unsigned int > wi::bswap_large (HOST_WIDE_INT *val, const HOST_WIDE_INT *xval, > - unsigned int len, unsigned int precision) > + unsigned int xlen, unsigned int precision) > { > - unsigned int i, s; > + unsigned int s, len =3D BLOCKS_NEEDED (precision); >=20 > /* This is not a well defined operation if the precision is not a > multiple of 8. */ > gcc_assert ((precision & 0x7) =3D=3D 0); >=20 > - for (i =3D 0; i < len; i++) > - val[i] =3D 0; > + memset (val, 0, sizeof (unsigned HOST_WIDE_INT) * len); >=20 > /* Only swap the bytes that are not the padding. */ > for (s =3D 0; s < precision; s +=3D 8) > @@ -753,7 +752,7 @@ wi::bswap_large (HOST_WIDE_INT *val, con > unsigned int block =3D s / HOST_BITS_PER_WIDE_INT; > unsigned int offset =3D s & (HOST_BITS_PER_WIDE_INT - 1); >=20 > - byte =3D (safe_uhwi (xval, len, block) >> offset) & 0xff; > + byte =3D (safe_uhwi (xval, xlen, block) >> offset) & 0xff; >=20 > block =3D d / HOST_BITS_PER_WIDE_INT; > offset =3D d & (HOST_BITS_PER_WIDE_INT - 1); > --- gcc/testsuite/gcc.dg/pr113722.c.jj 2024-02-02 18:25:22.702561427 +0= 100 > +++ gcc/testsuite/gcc.dg/pr113722.c 2024-02-02 18:21:00.109186858 +0100= > @@ -0,0 +1,22 @@ > +/* PR middle-end/113722 */ > +/* { dg-do run { target int128 } } */ > +/* { dg-options "-O2" } */ > + > +int > +main () > +{ > + unsigned __int128 a =3D __builtin_bswap128 ((unsigned __int128) 2); > + if (a !=3D ((unsigned __int128) 2) << 120) > + __builtin_abort (); > + a =3D __builtin_bswap128 ((unsigned __int128) 0xdeadbeefULL); > + if (a !=3D ((unsigned __int128) 0xefbeaddeULL) << 96) > + __builtin_abort (); > + a =3D __builtin_bswap128 (((unsigned __int128) 0xdeadbeefULL) << 64); > + if (a !=3D ((unsigned __int128) 0xefbeaddeULL) << 32) > + __builtin_abort (); > + a =3D __builtin_bswap128 ((((unsigned __int128) 0xdeadbeefULL) << 64) > + | 0xcafed00dfeedbac1ULL); > + if (a !=3D ((((unsigned __int128) 0xc1baedfe0dd0fecaULL) << 64) > + | (((unsigned __int128) 0xefbeaddeULL) << 32))) > + __builtin_abort (); > +} >=20 > Jakub >=20