PATCH [3/n]: Add __snseconds_t and __SNSECONDS_T_TYPE
A message that is not in the archive (sent off-list or not yet received): [email protected]
A message that is not in the archive (sent off-list or not yet received): [email protected]
A message that is not in the archive (sent off-list or not yet received): CAMe9rOpXRcO6a4cFEU=s1Gjzzt87zvyzuLhAWupc2pOX6az6ig@mail.gmail.com
A message that is not in the archive (sent off-list or not yet received): [email protected]
A message that is not in the archive (sent off-list or not yet received): CAMe9rOpvnhPmpmsLedmOjjYB9SghZiov-cOUi+wrYvKaP+c9pQ@mail.gmail.com
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 10 lines · Paul Eggert
Quoted 2 lines · H.J. Lu
What is the real consequence of using long long on
tv_nsec, except for not POSIX compliant? Will it lead to wrong code?It would break applications that do anything like this:
struct timespec t;
long *p =&t->tv_nsec;Such applications work fine now and conform to POSIX, but would
GCC will complain about "incompatible pointer type".
Quoted 2 lines
either not compile or (worse) might compile and do the wrong thing, if
tv_nsecwere wider than 'long'.
We had a discussion on Linux kernel mailing list:
https://lkml.org/lkml/2012/2/8/408
We thought it was OK to have long long on tv_nsec.
Signature · 1 line
H.J.
Quoted 1 line · H.J. Lu
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.
Quoted 5 lines
Quoted 3 lines
struct timespec t;
long *p =&t->tv_nsec;
Such applications work fine now and conform to POSIXGCC 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.
Quoted 2 lines
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.
x32 and width of blksize_t, suseconds_t
Come to think of it, the proposed x32 patch may have issues with blksize_t and suseconds_t as well. POSIX says that an x32 implementation must support at least one programming environment (presumably settable via a feature test macro) where blksize_t and suseconds_t are no wider than 'long'; see
<http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/sys_types.h.html>.
But if I understand things correctly, x32 glibc would define these to be 'long long'. This issue affects system calls such as 'stat' and 'select'.
Re: x32 and width of blksize_t, suseconds_t
Quoted 9 lines · Paul Eggert
Come to think of it, the proposed x32 patch may have issues with
blksize_tandsuseconds_tas well. POSIX says that an x32 implementation must support at least one programming environment (presumably settable via a feature test macro) whereblksize_tandsuseconds_tare no wider than 'long'; see
<http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/sys_types.h.html>.
But if I understand things correctly, x32 glibc would define these to be 'long long'. This issue affects system calls such as 'stat' and 'select'.
That's just not going to happen, sorry. Why on Earth is this a requirement? It makes no sense whatsoever.
-hpa
Signature · 2 lines
H. Peter Anvin, Intel Open Source Technology Center I work for Intel. I don't speak on their behalf.
Re: x32 and width of blksize_t, suseconds_t
Quoted 11 lines · H. Peter Anvin
Quoted 9 lines · Paul Eggert
Come to think of it, the proposed x32 patch may have issues with
blksize_tandsuseconds_tas well. POSIX says that an x32 implementation must support at least one programming environment (presumably settable via a feature test macro) whereblksize_tandsuseconds_tare no wider than 'long'; see
<http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/sys_types.h.html>.
But if I understand things correctly, x32 glibc would define these to be 'long long'. This issue affects system calls such as 'stat' and 'select'.
Quoted 2 lines
That's just not going to happen, sorry. Why on Earth is this a requirement? It makes no sense whatsoever.
It's probably to support code written for C90. Note that it's the POSIX-conforming system as a whole that is to support such an environment
- but it might be an environment that uses the traditional 32-bit or 64-bit ABIs, it needn't be one using the x32 ABI. "getconf POSIX_V7_WIDTH_RESTRICTED_ENVS" must print the names of the environments meeting this requirement; if one is "POSIX_V7_ILP32_OFF32", for example, then "getconf POSIX_V7_ILP32_OFF32_CFLAGS" will print the flags (maybe -m32) and likewise for _LDFLAGS and _LIBS. In glibc this means defining appropriate macros in bits/environment.h to give the right flags - and also probably changing posix/confstr.c which has comments saying "Currently this means all environment which the system allows." (the x32 patch series may need to adjust this in some way).
Signature · 2 lines
Joseph S. Myers [email protected]
Re: x32 and width of blksize_t, suseconds_t
Quoted 16 lines · Joseph S. Myers
Quoted 11 lines · H. Peter Anvin
Quoted 9 lines · Paul Eggert
Come to think of it, the proposed x32 patch may have issues with
blksize_tandsuseconds_tas well. POSIX says that an x32 implementation must support at least one programming environment (presumably settable via a feature test macro) whereblksize_tandsuseconds_tare no wider than 'long'; see
<http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/sys_types.h.html>.
But if I understand things correctly, x32 glibc would define these to be 'long long'. This issue affects system calls such as 'stat' and 'select'.Quoted 2 lines
That's just not going to happen, sorry. Why on Earth is this a requirement? It makes no sense whatsoever.
Quoted 13 lines
It's probably to support code written for C90. Note that it's the POSIX-conforming system as a whole that is to support such an environment
- but it might be an environment that uses the traditional 32-bit or 64-bit ABIs, it needn't be one using the x32 ABI. "getconfPOSIX_V7_WIDTH_RESTRICTED_ENVS" must print the names of the environments meeting this requirement; if one is "POSIX_V7_ILP32_OFF32", for example, then "getconfPOSIX_V7_ILP32_OFF32_CFLAGS" will print the flags (maybe -m32) and likewise for_LDFLAGSand_LIBS. In glibc this means defining appropriate macros inbits/environment.hto give the right flags - and also probably changingposix/confstr.cwhich has comments saying "Currently this means all environment which the system allows." (the x32 patch series may need to adjust this in some way).
I took a look at POSIX getconf. It only supports one 32bit ABI and one 64bit ABI. I can't find a way to support another 32bit ABI. I decided not to change it for x32. If you use POSIX getconf to select a 32bit ABI, you will get IA32.
Signature · 1 line
H.J.
Quoted 8 lines · Paul Eggert
Quoted 1 line · H.J. Lu
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>
Quoted 4 lines
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.
Quoted 45 lines
Quoted 5 lines
Quoted 3 lines
struct timespec t;
long *p =&t->tv_nsec;
Such applications work fine now and conform to POSIXGCC will complain about "incompatible pointer type".
True, and admittedly taking the address of
tv_nsecis rarer than printing it. Still, it's just a warning and GCC goes ahead and builds the program, and such warnings are often ignored.Quoted 2 lines
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_timespecis an inline function that merely copies on existing platforms, and also sign-extendstv_nsecon 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_nsecinside the kernel, when copying struct timespec objects from the user to the kernel.
Quoted 8 lines
Yet another option, I guess, would be to change POSIX so that
tv_nseccould be of type wider than 'long'. However, this would seem to run afoul of POSIX's intent, which is that system types likesuseconds_tshould 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.
Signature · 1 line
H.J.
Quoted 2 lines · H.J. Lu
I'd prefer to change POSIX. This isn't the only place where x32 isn't 100% compatible with POSIX.
If this approach is taken, the compatibility issues with tv_nsec, blksize_t, and suseconds_t should be documented in the glibc manual, in areas where programmers using the interfaces are likely to find out about the problem. Could you propose a patch to do that?
It should be possible to change POSIX at some point, once the POSIX folks decide that C90 compatibility is no longer an issue. If POSIX conformance is not a goal of x32, then this is not an urgent matter -- which is probably just as well, as I think they still worry about C90.
In the meantime, I guess applications will just have to know not to take the address of a tv_nsec member. I just now added a note about this in the Gnulib portability manual, where it talks about glitches on systems that don't conform to POSIX:
http://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=47834c92f8122f4ba5e6fca5199a4611425bfe69
Quoted 8 lines · Paul Eggert
Quoted 2 lines · H.J. Lu
I'd prefer to change POSIX. This isn't the only place where x32 isn't 100% compatible with POSIX.
If this approach is taken, the compatibility issues with
tv_nsec,blksize_t, andsuseconds_tshould be documented in the glibc manual, in areas where programmers using the interfaces are likely to find out about the problem. Could you propose a patch to do that?
I will do that once we know the full scope for POSIX issue under x32.
Quoted 11 lines
It should be possible to change POSIX at some point, once the POSIX folks decide that C90 compatibility is no longer an issue. If POSIX conformance is not a goal of x32, then this is not an urgent matter -- which is probably just as well, as I think they still worry about C90.
In the meantime, I guess applications will just have to know not to take the address of a
tv_nsecmember. I just now added a note about this in the Gnulib portability manual, where it talks about glitches on systems that don't conform to POSIX:http://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=47834c92f8122f4ba5e6fca5199a4611425bfe69
Thanks.
Signature · 1 line
H.J.