Message-ID: <CAMe9rOqz0bC6hb=f0BCGzaRHhdnqi65gFCF0h8S0Okgo7TjSsQ@mail.gmail.com>
Date:	Fri, 16 Mar 2012 08:49:00 -0700
From:	"H.J. Lu" <hjl.tools@...il.com>
To:	Paul Eggert <eggert@...ucla.edu>
Cc:	LKML <linux-kernel@...r.kernel.org>,
	"H. Peter Anvin" <hpa@...or.com>,
	GNU C Library <libc-alpha@...rceware.org>
Subject: Re: PATCH [3/n]: Add __snseconds_t and __SNSECONDS_T_TYPE

On Fri, Mar 16, 2012 at 12:52 AM, Paul Eggert <eggert@...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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists