Date: Thu, 23 Sep 2021 01:27:27 +0200 From: Olivier Houchard <cognet@ci0.org> To: Jessica Clarke <jrtc27@freebsd.org> Cc: "src-committers@freebsd.org" <src-committers@freebsd.org>, "dev-commits-src-all@freebsd.org" <dev-commits-src-all@freebsd.org>, "dev-commits-src-main@freebsd.org" <dev-commits-src-main@freebsd.org> Subject: Re: git: ebbc3140ca0d - main - truss: Decode correctly 64bits arguments on 32bits arm. Message-ID: <YUu73/9/l7/zpE6K@ci0.org> In-Reply-To: <06FE486C-5D06-43FD-B64F-EFF902040BD3@freebsd.org> References: <202109222305.18MN5mxe013139@gitrepo.freebsd.org> <06FE486C-5D06-43FD-B64F-EFF902040BD3@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Sep 23, 2021 at 12:14:21AM +0100, Jessica Clarke wrote: > On 23 Sep 2021, at 00:05, Olivier Houchard <cognet@FreeBSD.org> wrote: > > > > The branch main has been updated by cognet: > > > > URL: https://cgit.FreeBSD.org/src/commit/?id=ebbc3140ca0d7eee154f7a67ccdae7d3d88d13fd > > > > commit ebbc3140ca0d7eee154f7a67ccdae7d3d88d13fd > > Author: Olivier Houchard <cognet@FreeBSD.org> > > AuthorDate: 2021-09-22 22:45:42 +0000 > > Commit: Olivier Houchard <cognet@FreeBSD.org> > > 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?YUu73/9/l7/zpE6K>