Re: [PATCH] off_t: fix register pair calculation for 64-bit case

[Arnd Bergmann <arnd at arndb dot de>]

On Monday, June 27, 2016 9:26:46 AM CEST Yury Norov wrote:
> On Fri, Jun 24, 2016 at 03:15:50PM +0200, Arnd Bergmann wrote:
> > On Friday, June 24, 2016 5:57:26 AM CEST H.J. Lu wrote:
> > > On Fri, Jun 24, 2016 at 5:41 AM, Yury Norov <ynorov@caviumnetworks.com> wrote:
> > > > On Fri, Jun 24, 2016 at 05:30:32AM -0700, H.J. Lu wrote:
> > > >> On Fri, Jun 24, 2016 at 5:23 AM, Yury Norov <ynorov@caviumnetworks.com> wrote:
> > > >> > There are 3 syscall wrappers under sysdeps/unix/sysv/linux that
> > > >> > calculate register pair for off_t like this:
> > > >> >         __LONG_LONG_PAIR (offset >> 31, offset)
> > > >> >
> > > >> > While it works for 32-bit off_t, new 32-bit APIs that use 64-bit
> > > >> > off_t will be broken with it. This patch redirects affected syscalls
> > > >> > to their 64-bit versions. It also saves few instructions and symbols
> > > >> > in glibc, as 32-bit syscall wrappers are not generated anymore.
> > > >>
> > > >> If you have 64-bit register, should you use wordsize-64, like
> > > >>
> > > >> sysdeps/unix/sysv/linux/wordsize-64
> > > >>
> > > >> H.J.
> > > >
> > > > Sometimes it's not possible. AARCh64/ILP32 requires to pass 64-bit
> > > > parameters as pair. (this is one of two options for ILP32 that is
> > > > under discussion)
> > > 
> > > You should still use wordsize-64 and make special exceptions if needed.
> > 
> > Can the syscall ABI be a single exception then? I think at this
> > point the syscall interface for aarch64 ILP32 is exactly the same
> > as for 32-bit RISC-V.
> > 
> > I guess it makes sense to use sysdeps/wordsize-64/ and 
> > sysdeps/ieee754/dbl-64/wordsize-64/, but the
> > sysdeps/unix/sysv/linux/wordsize-64/ directory seems to only
> > contain files for syscalls that differ between 32-bit and
> > 64-bit architectures, so each one of them would otherwise
> > need a separate override that redirects to the normal 32-bit
> > syscall.
> > 
> Hi Arnd, H.J. Lu, others,
> 
> I'm not so experienced in the glibc, and it seems I lost your point.

I'm also not very experienced in glibc, it's possible that I'm the
one who's confused

> The whole idea of ILP32 patchset is to be a counterpart for kernel code
> that clears top halves of registers unconditionally at now. It means we
> cannot pass any 64-bit value in a single register, and that's what
> the code under sysdeps/unix/sysv/linux/wordsize-64 does. So I don't
> understand how we can use it.

The code in sysdeps/unix/sysv/linux/wordsize-64 seems to be made for
the 64-bit syscall API, which is not appropriate here as the kernel
port uses the 32-bit syscall API, now basically unchanged.

This is the same as tile64/ilp32 does, but different from x86-64/ilp32
(x32).

> Regarding this patch. As far as I understand, each ABI can define size
> of it's types with no relation to register size. And modern
> 32-bit ABIs should have off_t, ino_t etc 64-bit. So we have off_t
> 64-bit but pass it in a pair. That's what the code under
> sysdeps/unix/sysv/linux/ does, except that it does not do it right.
>
> I didn't find the limitation on sysdeps/unix/sysv/linux/ to have off_t
> exactly 32-bit, and it means it should work correct for both cases.
> And therefore my patch fixes real bug.
>
> In other hand, if sysdeps/unix/sysv/linux/ should work with 32-bit
> off_t only, I suggest to describe it explicitly with code like this:
> 
>         #ifdef __OFF_T_MATCHES_OFF64_T
>         # error  off_t is 32-bit only
>         #endif
> 
> where needed. But then I'll still have to use sysdeps/unix/sysv/linux/
> for ILP32, and will redirect off_t-related syscalls in platform code.

I agree your patch looks fine, and it fixes the problem for 32-bit
RISC-V as well.

Redirecting off_t based syscalls to architecture specific code sounds
wrong: We should do the same thing for aarch64/ilp32 and 32-bit
risc-v, whatever we end up doing. The alternative that I think
Mike Frysinger was hinting at would be to leave off_t as 32-bit even
on aarch64/ilp32, but then just not use it. If you build your compiler
to always pass _FILE_OFFSET_BITS=64, you can leave this part of
glibc completely untouched and will get the symbols for handling
32-bit off_t, but all applications built against the libc will
use 64-bit off_t anyway. To me, that sounds like a bigger hack
for the whole system, but it makes the glibc platform specific code
a bit simpler because we avoid the special case.

	Arnd