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

[Dmitry V. Levin]

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.

>  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.


-- 
ldv