git: ebbc3140ca0d - main - truss: Decode correctly 64bits arguments on 32bits arm.

[Olivier Houchard]

On Thu, Sep 23, 2021 at 12:14:21AM +0100, Jessica Clarke wrote:
> On 23 Sep 2021, at 00:05, Olivier Houchard <cognet at 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 at FreeBSD.org>
> > AuthorDate: 2021-09-22 22:45:42 +0000
> > Commit:     Olivier Houchard <cognet at 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