From owner-dev-commits-src-main@freebsd.org Wed Sep 22 23:28:56 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 BC28E674B18; Wed, 22 Sep 2021 23:28:56 +0000 (UTC) (envelope-from cognet@ci0.org) Received: from kanar.ci0.org (kanar.ci0.org [IPv6:2001:bc8:35e6::1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "sd-143795", Issuer "sd-143795" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 4HFDyJ4867z3q6b; Wed, 22 Sep 2021 23:28:56 +0000 (UTC) (envelope-from cognet@ci0.org) Received: from kanar.ci0.org (localhost [127.0.0.1]) by kanar.ci0.org (8.16.1/8.16.1) with ESMTPS id 18MNRR2I067003 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NO); Thu, 23 Sep 2021 01:27:27 +0200 (CEST) (envelope-from cognet@ci0.org) Received: (from doginou@localhost) by kanar.ci0.org (8.16.1/8.16.1/Submit) id 18MNRRQk067002; Thu, 23 Sep 2021 01:27:27 +0200 (CEST) (envelope-from cognet@ci0.org) X-Authentication-Warning: kanar.ci0.org: doginou set sender to cognet@ci0.org using -f Date: Thu, 23 Sep 2021 01:27:27 +0200 From: Olivier Houchard To: Jessica Clarke Cc: "src-committers@freebsd.org" , "dev-commits-src-all@freebsd.org" , "dev-commits-src-main@freebsd.org" Subject: Re: git: ebbc3140ca0d - main - truss: Decode correctly 64bits arguments on 32bits arm. Message-ID: References: <202109222305.18MN5mxe013139@gitrepo.freebsd.org> <06FE486C-5D06-43FD-B64F-EFF902040BD3@freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <06FE486C-5D06-43FD-B64F-EFF902040BD3@freebsd.org> X-Rspamd-Queue-Id: 4HFDyJ4867z3q6b 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:28:56 -0000 On Thu, Sep 23, 2021 at 12:14:21AM +0100, Jessica Clarke wrote: > On 23 Sep 2021, at 00:05, Olivier Houchard wrote: > > > > The branch main has been updated by cognet: > > > > URL: https://cgit.FreeBSD.org/src/commit/?id=ebbc3140ca0d7eee154f7a67ccdae7d3d88d13fd > > > > commit ebbc3140ca0d7eee154f7a67ccdae7d3d88d13fd > > Author: Olivier Houchard > > AuthorDate: 2021-09-22 22:45:42 +0000 > > Commit: Olivier Houchard > > CommitDate: 2021-09-22 23:04:16 +0000 > > > > truss: Decode correctly 64bits arguments on 32bits arm. > > > > 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. > > > > MFC After: 1 week > > PR: 256199 > > --- > > usr.bin/truss/syscalls.c | 22 ++++++++++++++-------- > > 1 file changed, 14 insertions(+), 8 deletions(-) > > > > 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; > > > > +#ifndef __aarch64__ > > + (void)abi; > > +#endif > > offset = 0; > > prev = -1; > > for (i = 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 == 1) { > > - sc->args[i].offset++; > > - offset++; > > - } > > +#ifdef __aarch64__ > > + if (abi->pointer_size == sizeof(uint32_t)) > > Why? The caller already checks this, it doesn???t 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???s news to us because we???ve > 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. > Oops, apologizes, it's just that I am an idiot. It is now reverted to a hopefully more sane state. Olivier