Messages in this thread | | | Date | Fri, 16 Mar 2012 08:49:00 -0700 | Subject | Re: PATCH [3/n]: Add __snseconds_t and __SNSECONDS_T_TYPE | From | "H.J. Lu" <> |
| |
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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |