This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Fix LO_HI_LONG definition
On 06/07/2016 08:37, Florian Weimer wrote:
> On 07/05/2016 03:59 PM, Adhemerval Zanella wrote:
>>
>>
>> On 05/07/2016 10:18, Florian Weimer wrote:
>>> On 07/04/2016 07:08 PM, Adhemerval Zanella wrote:
>>>> The p{read,write}v{64} consolidation patch [1] added a wrong guard
>>>> for LO_HI_LONG definition. It currently uses both
>>>> '__WORDSIZE == 64' and 'defined __ASSUME_WORDSIZE64_ILP32' to set
>>>> the value to be passed in one argument, otherwise it will be split
>>>> in two.
>>>>
>>>> However it fails on MIPS64n32 where syscalls n32 uses the compat
>>>> implementation in the kernel meaning the off_t arguments are passed
>>>> in two separate registers.
>>>
>>> Was this bug caught by the glibc test suite on the affected architecture?
>>
>> Yes, Joseph reported it [1].
>>
>> [1] https://sourceware.org/ml/libc-alpha/2016-07/msg00051.html
>
> Thanks. I looked at existing uses of __OFF_T_MATCHES_OFF64_T, and it seems that it is used as if indicating whether off_t and off64_t are the same type.
>
> I'm surprised it's the right conditional here because just because the types are the same in user space, the system call convention supports 64-bit arguments.
And with you remark I noted this fix is incorrect. Since kernel commit
601cc11d054 the non-compat preadv/pwritev in same order, so the LO_HI_LONG
is the same regardless off/off64_t size or wordsize.
The tests were passing on 64-bit because files were small enough so
the high part of offset is 0 regardless. I will send an updated fix.
For testcase, I am not sure it is acceptable to create a 4GB file to
check on high part of offset (so I am accepting recommendations).