From owner-dev-commits-src-main@freebsd.org Wed Sep 22 23:14:29 2021 Return-Path: Delivered-To: dev-commits-src-main@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 2582E673C5C for ; Wed, 22 Sep 2021 23:14:29 +0000 (UTC) (envelope-from jrtc27@jrtc27.com) Received: from mail-wr1-f45.google.com (mail-wr1-f45.google.com [209.85.221.45]) (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 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4HFDdc6WLvz3mmM for ; Wed, 22 Sep 2021 23:14:28 +0000 (UTC) (envelope-from jrtc27@jrtc27.com) Received: by mail-wr1-f45.google.com with SMTP id d21so11503419wra.12 for ; Wed, 22 Sep 2021 16:14:28 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; bh=tDBx0p+5weOTWVbCd/zRsgUMEklRVtaxcLO5zjkGz+w=; b=iEVCftS3L0Msfhcr9OqmgUQBusVmv3bjntJGaKW52l9+7KSXkqgBKZrDtRSXFFL6k7 58Jt5zAv0Bcl9eMM8l5jaf2gCZE8sfZ1ab43xQXxV1Pp+W4JBtTeI3pgC3iYY2dGXS/C ++7+Vtz9bXVI5W94V+knsAazQE6YSdLKP6TZT/xDUVXq3E1Z8tLQCJH3qlhPov0APS6X o0Iv2pFfRKvNEjaafmnyiDwk6yPZWaOBgKx9EDFCZv6UZba/KyMfPURXZ7hG6nZ0UhUe tPu6VFHTpBWHPrrjyNK0HN4Mb8IQ3wk4PMerjs9Tfm/fExPIfe8puQQNmIjoqo7lBvNH fpjA== X-Gm-Message-State: AOAM533bulOqHtzAVGO5DQTUeSl7iSn4I0ynL2ahFd0SPDA4U5KhqS+h BqjpcsyN0thsOWqkVYi8MvFgASVa5aH7GQ== X-Google-Smtp-Source: ABdhPJz7HyQl094o3sWrWAoTSGjFCg2Uct0vGhgpeCmRJ7CHS7Nsx3vbf563hkhoIjwgYmbsPobbNw== X-Received: by 2002:a5d:440d:: with SMTP id z13mr1545340wrq.433.1632352461991; Wed, 22 Sep 2021 16:14:21 -0700 (PDT) Received: from smtpclient.apple (global-5-143.nat-2.net.cam.ac.uk. [131.111.5.143]) by smtp.gmail.com with ESMTPSA id i27sm7385868wmb.40.2021.09.22.16.14.21 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Wed, 22 Sep 2021 16:14:21 -0700 (PDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 14.0 \(3654.120.0.1.13\)) Subject: Re: git: ebbc3140ca0d - main - truss: Decode correctly 64bits arguments on 32bits arm. From: Jessica Clarke In-Reply-To: <202109222305.18MN5mxe013139@gitrepo.freebsd.org> Date: Thu, 23 Sep 2021 00:14:21 +0100 Cc: "src-committers@freebsd.org" , "dev-commits-src-all@freebsd.org" , "dev-commits-src-main@freebsd.org" Content-Transfer-Encoding: quoted-printable Message-Id: <06FE486C-5D06-43FD-B64F-EFF902040BD3@freebsd.org> References: <202109222305.18MN5mxe013139@gitrepo.freebsd.org> To: Olivier Houchard X-Mailer: Apple Mail (2.3654.120.0.1.13) X-Rspamd-Queue-Id: 4HFDdc6WLvz3mmM X-Spamd-Bar: ---- Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[] X-BeenThere: dev-commits-src-main@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for the main branch of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 22 Sep 2021 23:14:29 -0000 On 23 Sep 2021, at 00:05, Olivier Houchard wrote: >=20 > The branch main has been updated by cognet: >=20 > URL: = https://cgit.FreeBSD.org/src/commit/?id=3Debbc3140ca0d7eee154f7a67ccdae7d3= d88d13fd >=20 > commit ebbc3140ca0d7eee154f7a67ccdae7d3d88d13fd > Author: Olivier Houchard > AuthorDate: 2021-09-22 22:45:42 +0000 > Commit: Olivier Houchard > CommitDate: 2021-09-22 23:04:16 +0000 >=20 > truss: Decode correctly 64bits arguments on 32bits arm. >=20 > When decoding 32bits arm syscall, make sure we account for the = padding when > decoding 64bits args. Do it too when using a 64bits truss on a = 32bits binary. >=20 > MFC After: 1 week > PR: 256199 > --- > usr.bin/truss/syscalls.c | 22 ++++++++++++++-------- > 1 file changed, 14 insertions(+), 8 deletions(-) >=20 > diff --git a/usr.bin/truss/syscalls.c b/usr.bin/truss/syscalls.c > index f7657f30b583..9cd53e71cc9b 100644 > --- a/usr.bin/truss/syscalls.c > +++ b/usr.bin/truss/syscalls.c > @@ -792,11 +792,14 @@ print_mask_arg32(bool (*decoder)(FILE *, = uint32_t, uint32_t *), FILE *fp, > * decoding arguments. > */ > static void > -quad_fixup(struct syscall_decode *sc) > +quad_fixup(struct procabi *abi, struct syscall_decode *sc) > { > int offset, prev; > u_int i; >=20 > +#ifndef __aarch64__ > + (void)abi; > +#endif > offset =3D 0; > prev =3D -1; > for (i =3D 0; i < sc->nargs; i++) { > @@ -810,17 +813,20 @@ quad_fixup(struct syscall_decode *sc) > switch (sc->args[i].type & ARG_MASK) { > case Quad: > case QuadHex: > -#ifdef __powerpc__ > +#if defined(__powerpc__) || defined(__arm__) || defined(__aarch64__) > /* > - * 64-bit arguments on 32-bit powerpc must be > + * 64-bit arguments on 32-bit powerpc and arm = must be > * 64-bit aligned. If the current offset is > * not aligned, the calling convention inserts > * a 32-bit pad argument that should be skipped. > */ > - if (sc->args[i].offset % 2 =3D=3D 1) { > - sc->args[i].offset++; > - offset++; > - } > +#ifdef __aarch64__ > + if (abi->pointer_size =3D=3D sizeof(uint32_t)) Why? The caller already checks this, it doesn=E2=80=99t need to be = checked again. You even had to update that caller to pass the ABI and the patch clearly shows the check there. Also, please put changes like this up for review in future, arichardson@ did a bunch of work getting non-native ABI support into truss with input from myself, so if you find you need anything other than things like adding to ifdefs then that=E2=80=99s news to us because = we=E2=80=99ve tested it with freebsd32, and downstream in CheriBSD with our added freebsd64 ABI (which is what you think of as normal 64-bit, but we add a new ABI for memory safety that is the native ABI, so we exercise even more weird cases than upstream can). Please revert everything other than adding to the existing ifdef and updating the comment. Jess > +#endif > + if (sc->args[i].offset % 2 =3D=3D 1) { > + sc->args[i].offset++; > + offset++; > + } > #endif > offset++; > default: > @@ -854,7 +860,7 @@ add_syscall(struct procabi *abi, u_int number, = struct syscall *sc) > * procabi instead. > */ > if (abi->pointer_size =3D=3D 4) > - quad_fixup(&sc->decode); > + quad_fixup(abi, &sc->decode); >=20 > if (number < nitems(abi->syscalls)) { > assert(abi->syscalls[number] =3D=3D NULL);