From owner-dev-commits-src-all@freebsd.org Wed Sep 22 23:14:29 2021 Return-Path: Delivered-To: dev-commits-src-all@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 2553D673C5B for ; Wed, 22 Sep 2021 23:14:29 +0000 (UTC) (envelope-from jrtc27@jrtc27.com) Received: from mail-wr1-f54.google.com (mail-wr1-f54.google.com [209.85.221.54]) (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 4HFDdc6jdkz3mpF for ; Wed, 22 Sep 2021 23:14:28 +0000 (UTC) (envelope-from jrtc27@jrtc27.com) Received: by mail-wr1-f54.google.com with SMTP id w29so11574933wra.8 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=0maba2MjJNwAOEPTQ3kJAi7cJ0TiJkNv4IUavi3i2ZPXjRR1sCYeyxGIW4XYBOjDAp jQO9PNJ8NVmPo+xvTboLRywwRwOfNo7oqtiHZCg03TkjsRmLTEPNw57ydrUo4vnITUIO 6T9xwEvk6qqKFt779eyZEpizleTWV3xGUy8c7qBrGzqugao0eBf7XLZcoFSqG2UabCWp 0o0GZXi2MvMDq7fxH85EnZHkf9LvvCPe4s/qDvGhBz8lsD0Sjfuy2cXP9lDOJjmbvQg1 KKjlzdZQcgjrXX9YoGi9ez7Jto6BsPBLM8H4Wn7o3nFBAAZA2jhpNY4qLoELZ/TOdzBD n8tg== X-Gm-Message-State: AOAM532XdaSyoiBmXT7fAFFo35NcheoDz+/Y7IBNCLghSqafF+VChxZv NC12O5yYXuecH510NUm4J0ShVg== 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: 4HFDdc6jdkz3mpF 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-all@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for all branches 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);