Re: PATCH [3/n]: Add __snseconds_t and __SNSECONDS_T_TYPE

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Fri, Mar 16, 2012 at 12:52 AM, Paul Eggert <eggert@cs.ucla.edu> wrote:
> On 03/15/2012 03:19 PM, H.J. Lu wrote:
>> https://lkml.org/lkml/2012/2/8/408
>
> That discussion does not seem to have considered the issue
> of pointers, nor the issue of printf that Russ Allbery pointed out.
> Here's an example from Kerrisk's "The Linux Programming Interface"
> <http://man7.org/tlpi/code/online/dist/timers/t_clock_nanosleep.c.html>
>
> ? ? ? ? ? ?printf("... Remaining: %ld.%09ld",
> ? ? ? ? ? ? ? ? ? ?(long) remain.tv_sec, remain.tv_nsec);
>
> The proposed change breaks code like this.

This will print the lower 32bit of remain.tv_nsec since for x32 each integer
argument lakes a register or a 8byte slot.

>>> ?struct timespec t;
>>> ?long *p = &t->tv_nsec;
>>> Such applications work fine now and conform to POSIX
>>
>> GCC will complain about "incompatible pointer type".
>
> True, and admittedly taking the address of tv_nsec is rarer than
> printing it. ?Still, it's just a warning and GCC goes ahead and builds
> the program, and such warnings are often ignored.
>
>> timespec is used in quite a few system calls. Checking all places
>> which need to sign-extend is quite complex.
>
> Many system calls copy timespec values from the kernel to the user;
> these would be unaffected. ?For syscalls that copy from the user
> to the kernel, one could change glibc code like this:
>
> ?/* The Linux kernel can in some situations update the timeout value.
> ? ? We do not want that so use a local variable. ?*/
> ?struct timespec tval;
> ?if (timeout != NULL)
> ? ?{
> ? ? ?tval = *timeout;
> ? ? ?timeout = &tval;
> ? ?}
>
> (taken from glibc/sysdeps/unix/sysv/linux/pselect.c) to something like this:
>
> ?/* The Linux kernel can in some situations update the timeout value,
> ? ? or require a properly sign-extended timespec. ?*/
> ?struct timespec tval;
> ?if (timeout != NULL)
> ? ?{
> ? ? ?copy_timespec (&tval, timeout);
> ? ? ?timeout = &tval;
> ? ?}
>
> where copy_timespec is an inline function that merely copies on existing
> platforms, and also sign-extends tv_nsec on x32. ?This doesn't appear complex,
> though admittedly it does slow things down slightly on x32.
>
> Another option, perhaps, would be to change the Linux kernel to
> know about x32 binaries and to sign-extend tv_nsec inside the kernel,
> when copying struct timespec objects from the user to the kernel.
>
> Yet another option, I guess, would be to change POSIX so that tv_nsec could
> be of type wider than 'long'. ?However, this would seem to run afoul of
> POSIX's intent, which is that system types like suseconds_t should
> not be wider than 'long'; see
> <http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/sys_types.h.html>.
> This constraint is to support uses like 'printf'.
> Given the likelihood of breaking programs, it may be better simply
> to conform to POSIX in this area, rather than change POSIX.

I'd prefer to change POSIX. This isn't the only place where x32 isn't
100% compatible with POSIX.

-- 
H.J.