Re: [PATCH] printllval: fix 64bit unpacking on mips/o32 and xtensa

[Mike F. <va...@ge...>]

On Sunday 05 May 2013 19:38:01 Dmitry V. Levin wrote:
> On Sun, May 05, 2013 at 06:30:21PM -0400, Mike Frysinger wrote:
> > While ARM's EABI does aligning of 64bit values when making syscalls, its
> > OABI does not.  Note that this doesn't work atm in case someone feels
> > like fixing it :).  We don't keep tracking of the target ABI anywhere,
> > so this will require tweaking a few core places.
> > 
> > The mips/o32 ABI and xtensa arch also do 64bit aligning, so add it to the
> > printllval list for this.
> > 
> > Normally for sys_fadvise64_64 we'd handle the same list of arches, but
> > mips/o32 ABI is funky and doesn't shift -- it has 7 args.  So just add
> > xtensa to it.
> > 
> > * file.c (sys_fadvise64_64): Add XTENSA to the shifted list.
> > * util.c (printllval): Add LINUX_MIPSO32 and XTENSA to the shifted list.
> > ---
> > 
> >  file.c | 2 +-
> >  util.c | 3 ++-
> >  2 files changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/file.c b/file.c
> > index e5fc259..07f1ad9 100644
> > --- a/file.c
> > +++ b/file.c
> > @@ -2677,7 +2677,7 @@ sys_fadvise64_64(struct tcb *tcp)
> > 
> >  		printfd(tcp, tcp->u_arg[0]);
> >  		argn = printllval(tcp, ", %lld, ", 1);
> >  		argn = printllval(tcp, "%lld, ", argn);
> > 
> > -#if defined ARM || defined AARCH64 || defined POWERPC
> > +#if defined ARM || defined AARCH64 || defined POWERPC || defined XTENSA
> > 
> >  		printxval(advise, tcp->u_arg[1], "POSIX_FADV_???");
> >  
> >  #else
> >  
> >  		printxval(advise, tcp->u_arg[argn], "POSIX_FADV_???");
> > 
> > diff --git a/util.c b/util.c
> > index 970faa4..f44c7b6 100644
> > --- a/util.c
> > +++ b/util.c
> > @@ -207,8 +207,9 @@ printllval(struct tcb *tcp, const char *format, int
> > arg_no)
> > 
> >  		arg_no += 2;
> >  	
> >  	}
> >  
> >  #else
> > 
> > -# if defined(ARM) || defined(POWERPC)
> > +# if defined(ARM) || defined(POWERPC) || defined(LINUX_MIPSO32) ||
> > defined(XTENSA)
> >  	/* Align arg_no to the next even number. */
> > +	/* Note: This is broken for ARM/OABI. */
> 
> We have "defined(__ARM_EABI__)" checks in few places, so we could add more
> such checks if it helps.
> All ARM systems I can reach are EABI, so I wouldn't be able to test changes
> related to ARM OABI anyway.

it would help in the case where you have an OABI strace tracing OABI program, 
or EABI strace tracing EABI program.  but if you try to cross use OABI to 
trace EABI or vice versa, it still won't help.
-mike