[PATCH 2/2] printrusage: Correctly print the long types from the rusage struct

[Alistair Francis]

On Fri, Mar 6, 2020 at 6:45 PM Dmitry V. Levin <ldv at altlinux.org> wrote:
>
> On Fri, Mar 06, 2020 at 06:29:00PM -0800, Alistair Francis wrote:
> > On Fri, Mar 6, 2020 at 6:22 PM Dmitry V. Levin <ldv at altlinux.org> wrote:
> > >
> > > On Fri, Mar 06, 2020 at 05:25:49PM -0800, Alistair Francis wrote:
> > > > On Thu, Mar 5, 2020 at 5:05 PM Dmitry V. Levin <ldv at altlinux.org> wrote:
> > > > >
> > > > > On Wed, Mar 04, 2020 at 09:25:20PM -0800, Alistair Francis wrote:
> > > > > > On Wed, Mar 4, 2020 at 6:04 PM Dmitry V. Levin wrote:
> > > > > > > On Wed, Mar 04, 2020 at 05:30:34PM -0800, Alistair Francis wrote:
> > > > > > > > The kernel's rusage struct uses the kernel's __kernel_old_timeval which
> > > > > > > > means that even for 32-bit archs with 64-bit time_t (like RV32) the time
> > > > > > > > values are 32-bit.
> > > > > > > >
> > > > > > > > There are two possible options here:
> > > > > > > >  1. Use the glibc getrusage() function. Which takes a struct rusage with
> > > > > > > >     a 64-bit time_t but does a syscall with a 32-bit time_t and handles
> > > > > > > >     the conversion internally.
> > > > > > > >  2. Specify our own rusage struct that uses a long for the time.
> > > > > > > >
> > > > > > > > This patch fixes the failure useing option 2 above as we then get to
> > > > > > > > call the raw syscall.
> > > > > > > >
> > > > > > > > Signed-off-by: Alistair Francis <alistair.francis at wdc.com>
> > > > > > > > ---
> > > > > > > >  print_timeval.c   | 16 ++++++++++++++++
> > > > > > > >  printrusage.c     |  4 ++--
> > > > > > > >  tests/getrusage.c | 28 ++++++++++++++++++++++++++--
> > > > > > > >  3 files changed, 44 insertions(+), 4 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/print_timeval.c b/print_timeval.c
> > > > > > > > index ff87b27b..36bc843e 100644
> > > > > > > > --- a/print_timeval.c
> > > > > > > > +++ b/print_timeval.c
> > > > > > > > @@ -11,6 +11,10 @@
> > > > > > > >  #include DEF_MPERS_TYPE(timeval_t)
> > > > > > > >
> > > > > > > >  typedef struct timeval timeval_t;
> > > > > > > > +typedef struct old_timeval_t {
> > > > > > > > +     long tv_sec;
> > > > > > > > +     long tv_usec;
> > > > > > > > +} old_timeval_t;
> > > > > > >
> > > > > > > Sorry but this definition doesn't match the definition of struct
> > > > > > > __kernel_old_timeval in include/uapi/linux/time_types.h and
> > > > > > > arch/sparc/include/uapi/asm/posix_types.h files.
> > > > > >
> > > > > > Do you mean it should be __kernel_long_t [1]? I can change it to
> > > > > > __kernel_long_t .
> > > > > >
> > > > > > I'm not sure what to do to handle the space difference [2] though. I
> > > > > > think it ends up being long but I don't see a nice way to handle it.
> > > > >
> > > > > It definitely has to be __kernel_long_t on everything but sparc where
> > > > > tv_usec has to be int if I read arch/sparc/include/uapi/asm/posix_types.h
> > > > > correctly.
> > > > >
> > > > > BTW, since include/uapi/linux/time.h along with
> > > > > include/uapi/asm-generic/posix_types.h define struct timeval as
> > > > > essentially the same structure as struct __kernel_old_timeval,
> > > > > why do we need a separate printer for old_timeval_t?
> > > > > Is it due to libc that may define its struct timeval in an incompatible way?
> > > >
> > > > Yes, we end up using the libc struct timeval (which is not the same as
> > > > __kernel_old_timeval).
> > > >
> > > > I can actually just use __kernel_old_timeval directly and that works.
> > >
> > > So what should we do?  Should we stop using struct timeval in print_timeval.c
> > > and switch to a locally defined struct kernel_old_timeval?
> >
> > I think we have to support both. I'll send a new version of the patch
> > now so it's clear, but we need to support these two:
> >  1. struct timeval form libc - This uses 64-bit time_t in y2038 systems
>
> I don't see why do we need struct timeval in strace decoders if that
> definition might be different from the structure used by the kernel.

I'm not clear what you mean here.

The current print_timeval.c prints the libc struct timeval, which
isn't always the same as the timeval in rusage. We just need a nice
way to handle that.

>
> >  2. __kernel_old_timeval from Linux - This uses 32-bit time_t for
> > y2038 systems and is used inside rusage
>
> We cannot use __kernel_old_timeval directly from Linux headers as the
> latter can be too old to provide the definition we need.

Argh, that seemed too easy.

Do you have thoughts on the best way to handle the two timeval's then?

Alistair

>
>
> --
> ldv