This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [Y2038] Fifth draft of the Y2038 design document
On Mon, Feb 27, 2017 at 12:02 PM, Albert ARIBAUD
<albert.aribaud@3adev.fr> wrote:
> Hi Arnd,
>
> On Thu, 23 Feb 2017 21:24:22 +0100, Arnd Bergmann <arnd@arndb.de>
> wrote :
>
>> On Thu, Feb 23, 2017 at 6:31 PM, Joseph Myers <joseph@codesourcery.com> wrote:
>> > On Thu, 23 Feb 2017, Arnd Bergmann wrote:
>> >
>> [...]
>> >>
>> >> I had thought of that as well, but wasn't sure if we could rely on bitfields
>> >> to do the right thing across C standard versions and non-gcc compilers.
>> >> I experimentally found that gcc-4.9 and higher accept that version without
>> >> a -Wpedantic warning when using --std=c99 or --std=c11, but they warn
>> >> about it with --std=c89 or --std=gnu89. Older gcc versions as far back as
>> >> gcc-3.4 (the oldest I could try) always warn about anonymous bitfields
>> >> with -pedantic, but I could still silence that warning using the
>> >> __extension__ keyword. I think that's good enough.
>> >
>> > What's the warning you see, with what exact testcase? Unnamed bit-fields
>> > (provided the declared type is int / signed int / unsigned int, not any
>> > other integer or enum type), and initializers ignoring them, date back at
>> > least to the third public review draft of C89 (13 May 1988), which is the
>> > oldest version I have to hand.
>>
>> Sorry, my mistake, I used "long : 32", which causes "warning: type of
>> bit-field '<anonymous>' is a GCC extension [-Wpedantic]". With 'int'
>> it works fine.
>
> So IIUC (and assuming we keep tv_nsec a long in APIs) we /could/ pad
> struct timespec with an anonymous bitfield and still be compatible with
> (most) existing user source code, and it is considered acceptable (but
> should be duly documented). Added this in rev. 122of the design doc.
This solves the problem in user space, but it still leaves the other problem
that Zack and I mentioned on the user/kernel boundary: we have to zero
out the padding since at least 64-bit kernels with 32-bit user space
would interpret the padding as nanoseconds over 1 billion. We may also
want to make the kernel behavior consistent between 32-bit and 64-bit
kernels, so the kernel will either always ignore the padding (which is
a bit more work for the kernel) or it will always reject nonzero upper bits
of the nanoseconds (as 64-bit kernels do today for native tasks, this
means more work in 32-bit glibc, or using 64-bit nanoseconds as
Zack suggested).
Right now, I have this in the kernel, to do the zeroing whenever we
copy a __kernel_timespec from user space into a timespec64
in the kernel:
typedef __s64 __kernel_time64_t;
struct __kernel_timespec {
__kernel_time64_t tv_sec;
__s64 tv_nsec;
};
int get_timespec64(struct timespec64 *ts,
const struct __kernel_timespec __user *uts)
{
struct __kernel_timespec kts;
int ret;
ret = copy_from_user(&kts, uts, sizeof(kts));
if (ret)
return -EFAULT;
ts->tv_sec = kts.tv_sec;
if (!IS_ENABLED(CONFIG_64BIT) || in_compat_syscall())
kts.tv_nsec &= 0xfffffffful;
ts->tv_nsec = kts.tv_nsec;
return 0;
}
This should catch anything that passes in a timespec by itself
(clock_settime, clock_nanosleep, timer_settime, timerfd_settime,
ppoll, pselect6, io_getevents, recvmmsg, mq_timedsend,
mq_timedreceive, semtimedop, utimensat, rt_sigtimedwait,
futex and some ioctls), but not structures that embed a timespec
(fortunately no syscalls, very few ioctls). The external function
call plus the in_compat_syscall() check will add a couple of cycles
on 64-bit architectures, for both 32-bit and 64-bit tasks. We could
optimize this slightly using
if (IS_ENABLED(CONFIG_64BIT))
kts.tv_nsec &= current->tv_nsec_mask;
Arnd