Re: [PATCH v3 3/3] system_data_types.7: note struct timespec::tv_nsec type for x32 and portability

Re: [PATCH v3 3/3] system_data_types.7: note struct timespec::tv_nsec type for x32 and portability

[Alejandro Colomar (man-pages)]

[CC += Jakub, glibc]

Hi наб (and other readers),

On 12/6/21 23:03, наб wrote:
> There are three files that govern userspace struct timespec on glibc:
> 1. bits/wordsize.h, defining:
>     (a) __WORDSIZE to 32 on ILP32 and 64 on LP64
>     (b) on x32: __SYSCALL_WORDSIZE to 64
> 2. bits/timesize.h, defining
>     (a) __TIMESIZE to __WORDSIZE, except on x32 where it's 64
> 3. bits/types/struct_timespec.h, declaring struct timespec as:
>       struct timespec
>       {
>        __time_t tv_sec;      /* Seconds.  */
>       #if __WORDSIZE == 64 \
>        || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64) \
>        || __TIMESIZE == 32
>        __syscall_slong_t tv_nsec;    /* Nanoseconds.  */
>       #else
>       # if __BYTE_ORDER == __BIG_ENDIAN
>        int: 32;           /* Padding.  */
>        long int tv_nsec;  /* Nanoseconds.  */
>       # else
>        long int tv_nsec;  /* Nanoseconds.  */
>        int: 32;           /* Padding.  */
>       # endif
>       #endif
>       };
>     this has two side-effects: struct timespec
>     (a) is always sizeof==time_t+8, and
>     (b) has tv_nsec as __syscall_slong_t
>         *and* !is_same<__syscall_slong_t, long>
>         if using LP64 syscalls on an ILP32 system, i.e. on x32.
> 
> This means, that the simplified
>    struct timespec {
>        time_t  tv_sec;  /* Seconds */
>        long    tv_nsec; /* Nanoseconds [0 .. 999999999] */
>    };
> declaration is *invalid* for x32,
> where struct timespec::tv_nsec is an int64_t (long long).
> 
> Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
> ---
> The reasoning is explained in the commit message,
> but I elucidated it in a more approachable way in the Notes,
> which also include the link from Jakub Wilk upthread.
> 

I (more or less) understand the deduction of why that happens on certain 
systems,  what I was wondering is if there's an intention behind that, 
or if it's just an accident.  Why not just use plain 'long' always as 
POSIX says it should be.  I don't see the feature part of this bug.

> Nevertheless, log2(10^(3*3) - 1) <= 31 is a good point!

Yes, that's what I really don't understand.  'long' is at least 32 bits, 
which is enough for 1G.  Was there ever a good reason for glibc to use 
64 bits?  It seems like a wrong decision that is maintained for 
consistency.  If that's it I'm in favor of mentioning it in a Bugs 
section (as Jakub proposed), instead of a Notes section.  Maybe someone 
from glibc may give a rationale for this deviation from POSIX.

> I also added this as a slight portability guide toward the end.

Nice.

> 
> This replaces 3/4 and 4/4.
> 
Okay.


>   man7/system_data_types.7 | 30 ++++++++++++++++++++++++++++++
>   1 file changed, 30 insertions(+)
> 
> diff --git a/man7/system_data_types.7 b/man7/system_data_types.7
> index 1e6a3f74c..cce17fc3e 100644
> --- a/man7/system_data_types.7
> +++ b/man7/system_data_types.7
> @@ -1553,6 +1553,36 @@ Describes times in seconds and nanoseconds.
>   .IR "Conforming to" :
>   C11 and later; POSIX.1-2001 and later.
>   .PP
> +.IR Notes :
> +.I tv_nsec
> +is the
> +.I syscall
> +long, though this affects only fringe architectures like X32,
> +which is ILP32, but uses the LP64 AMD64 syscall ABI.

I'd add an explicit mention that this is for glibc, since it deviates 
from the "Conforming to".  Maybe preceding the paragraph with "In/On (I 
never knew if there goes in or on) glibc," would do.

> +.br

We don't use '.br'.  In this case I think just continuing in the same 
paragraph would be fine (i.e., no break at all).

> +In reality, the field ends up being defined as:

Here I'd add a blank line with '.PP'.

> +.EX
> +.in +4

Please, revert the order of .in and .EX.
It's meaningless, but I prefer consistency.  See man-pages(7), which 
shows this order, and also the following:

$ grep -rn '^\.EX$' -A1 man? | grep '\.in' | wc -l
2
$ grep -rn '^\.EX$' -B1 man? | grep '\.in' | wc -l
1048


One of those 2 cases was this patch.

> +#if !(__x86_64__ && __ILP32__ /* == x32 */)

Now I notice:

Shouldn't this use defined()?

#if !(defined(__x86_64__) && defined(__ILP32__) /* == x32 */)

Also, I prefer to avoid complexity in mentally parsing the ifs, so I'd 
reorder it to remove the negation:

#if defined(__x86_64__) && defined(__ILP32__)  /* == x32 */
	long long tv_nsec;
#else
	long      tv_nsec;
#endif

And also note a cosmetic minor thing: at least two spaces before a comment.


Cheers,
Alex

> +    long      tv_nsec;
> +#else
> +    long long tv_nsec;
> +#endif
> +.in
> +.EE
> +.PP
> +This is a long-standing and long-enshrined
> +.UR https://sourceware.org/bugzilla/show_bug.cgi?id=16437
> +glibc bug
> +.I #16437
> +.UE ,
> +an incompatible extension to the standards;
> +however, as even a 32-bit
> +.I long
> +can hold the entire
> +.I tv_nsec
> +range, it's safe to forcibly down-cast it to the standard type.
> +.PP
>   .IR "See also" :
>   .BR clock_gettime (2),
>   .BR clock_nanosleep (2),
> 


-- 
Alejandro Colomar
Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/

Re: [PATCH v3 3/3] system_data_types.7: note struct timespec::tv_nsec type for x32 and portability

[наб]

[-- Attachment #1: Type: text/plain, Size: 6210 bytes --]

On Mon, Dec 06, 2021 at 11:56:19PM +0100, Alejandro Colomar (man-pages) wrote:
> I (more or less) understand the deduction of why that happens on certain
> systems,  what I was wondering is if there's an intention behind that, or if
> it's just an accident.  Why not just use plain 'long' always as POSIX says
> it should be.  I don't see the feature part of this bug.
>
> 'long' is at least 32 bits, which
> is enough for 1G.  Was there ever a good reason for glibc to use 64 bits?  It
> seems like a wrong decision that is maintained for consistency.   If that's it
> I'm in favor of mentioning it in a Bugs section (as Jakub proposed), instead
> of a Notes section.  Maybe someone from glibc may give a rationale for this
> deviation from POSIX.
AFAICT, as you say, this isn't much more than a side-effect
of using the amd64 ABI and not being too careful with how it interacts
with the POSIX requirement (it's entirely feasible to re-wrap
a user-space timespec, or even use the bitfield trick glibc
already does to not have to do it), but then I'm far from an expert;
indeed, maybe someone with more glibc involvement will know.

Updating to Bugs for now, maybe we can downgrade later.

> I'd add an explicit mention that this is for glibc, since it deviates from
> the "Conforming to".
Fair enough, though I don't actually know if this is glibc-exclusive.
I assume it is, since I didn't find a timespec in the musl git
repository and the linux uapi header has the straight-forward
definition, but I can't be sure because I don't actually run
(well, even know of) any x32 musl systems I could verify this on.

> Maybe preceding the paragraph with "In/On (I never
> knew if there goes in or on) glibc," would do.
Out of those two: on (in would point to glibc internals),
but I'd actually go for "Under", here, since glibc is an over-arching
(literally) universe-defining context, rather than an add-on,
or something build your program on top of.

For my own curiosity: which preposition would you use in Spanish here?

> On 12/6/21 23:03, наб wrote:
> > +.br
> We don't use '.br'.  In this case I think just continuing in the same
> paragraph would be fine (i.e., no break at all).
> > +In reality, the field ends up being defined as:
> Here I'd add a blank line with '.PP'.
> > +.EX
> > +.in +4
> Please, revert the order of .in and .EX.
> It's meaningless, but I prefer consistency.  See man-pages(7), which shows
> this order, and also the following:
> 
> $ grep -rn '^\.EX$' -A1 man? | grep '\.in' | wc -l
> 2
> $ grep -rn '^\.EX$' -B1 man? | grep '\.in' | wc -l
> 1048
> 
> 
> One of those 2 cases was this patch.
Sure, sure, and sure

> > +#if !(__x86_64__ && __ILP32__ /* == x32 */)
> 
> Now I notice:
> 
> Shouldn't this use defined()?
> 
> #if !(defined(__x86_64__) && defined(__ILP32__) /* == x32 */)
Eeeeh, not really? That's functionally identical but, like,
very verbose for no good reason.

> Also, I prefer to avoid complexity in mentally parsing the ifs, so I'd
> reorder it to remove the negation:
> 
> #if defined(__x86_64__) && defined(__ILP32__)  /* == x32 */
> 	long long tv_nsec;
> #else
> 	long      tv_nsec;
> #endif
long first made more sense when this was in-line,
but I agree, non-negated is better.

> And also note a cosmetic minor thing: at least two spaces before a comment.
Sure


Updated scissor-patch below. I've also fixed some grammar errors toward
the tail-end of the (now) Bugs section.

Best,
наб

-- >8 --
There are three files that govern userspace struct timespec on glibc:
1. bits/wordsize.h, defining:
   (a) __WORDSIZE to 32 on ILP32 and 64 on LP64
   (b) on x32: __SYSCALL_WORDSIZE to 64
2. bits/timesize.h, defining
   (a) __TIMESIZE to __WORDSIZE, except on x32 where it's 64
3. bits/types/struct_timespec.h, declaring struct timespec as:
     struct timespec
     {
      __time_t tv_sec;      /* Seconds.  */
     #if __WORDSIZE == 64 \
      || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64) \
      || __TIMESIZE == 32
      __syscall_slong_t tv_nsec;    /* Nanoseconds.  */
     #else
     # if __BYTE_ORDER == __BIG_ENDIAN
      int: 32;           /* Padding.  */
      long int tv_nsec;  /* Nanoseconds.  */
     # else
      long int tv_nsec;  /* Nanoseconds.  */
      int: 32;           /* Padding.  */
     # endif
     #endif
     };
   this has two side-effects: struct timespec
   (a) is always sizeof==time_t+8, and
   (b) has tv_nsec as __syscall_slong_t
       *and* !is_same<__syscall_slong_t, long>
       if using LP64 syscalls on an ILP32 system, i.e. on x32.

This means, that the simplified
  struct timespec {
      time_t  tv_sec;  /* Seconds */
      long    tv_nsec; /* Nanoseconds [0 .. 999999999] */
  };
declaration is *invalid* for x32,
where struct timespec::tv_nsec is an int64_t (long long).

Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
---
 man7/system_data_types.7 | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/man7/system_data_types.7 b/man7/system_data_types.7
index 1e6a3f74c..66c9a5d3d 100644
--- a/man7/system_data_types.7
+++ b/man7/system_data_types.7
@@ -1553,6 +1553,37 @@ Describes times in seconds and nanoseconds.
 .IR "Conforming to" :
 C11 and later; POSIX.1-2001 and later.
 .PP
+.IR Bugs :
+Under glibc,
+.I tv_nsec
+is the
+.I syscall
+long, though this affects only fringe architectures like X32,
+which is ILP32, but uses the LP64 AMD64 syscall ABI.
+In reality, the field ends up being defined as:
+.PP
+.in +4
+.EX
+#if __x86_64__ && __ILP32__  /* == x32 */
+    long long tv_nsec;
+#else
+    long      tv_nsec;
+#endif
+.EE
+.in
+.PP
+This is a long-standing and long-enshrined
+.UR https://sourceware.org/bugzilla/show_bug.cgi?id=16437
+glibc bug
+.I #16437
+.UE ,
+and an incompatible extension to the standards;
+however, as even a 32-bit
+.I long
+can hold the entire
+.I tv_nsec
+range, it's always safe to forcibly down-cast it to the standard type.
+.PP
 .IR "See also" :
 .BR clock_gettime (2),
 .BR clock_nanosleep (2),
-- 
2.30.2

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 3/3] system_data_types.7: note struct timespec::tv_nsec type for x32 and portability

[Zack Weinberg]

On Mon, Dec 6, 2021, at 6:31 PM, наб via Libc-alpha wrote:
> On Mon, Dec 06, 2021 at 11:56:19PM +0100, Alejandro Colomar (man-pages) wrote:
>> I (more or less) understand the deduction of why that happens on certain
>> systems,  what I was wondering is if there's an intention behind that, or if
>> it's just an accident.  Why not just use plain 'long' always as POSIX says
>> it should be.  I don't see the feature part of this bug.
>>
>> 'long' is at least 32 bits, which
>> is enough for 1G.  Was there ever a good reason for glibc to use 64 bits?  It
>> seems like a wrong decision that is maintained for consistency.   If that's it
>> I'm in favor of mentioning it in a Bugs section (as Jakub proposed), instead
>> of a Notes section.  Maybe someone from glibc may give a rationale for this
>> deviation from POSIX.
> AFAICT, as you say, this isn't much more than a side-effect
> of using the amd64 ABI and not being too careful with how it interacts
> with the POSIX requirement (it's entirely feasible to re-wrap
> a user-space timespec, or even use the bitfield trick glibc
> already does to not have to do it), but then I'm far from an expert;
> indeed, maybe someone with more glibc involvement will know.

Depending on the kernel, the architecture, the ABI, and the state of _TIME_BITS, the format of `struct timespec` as expected by system calls may be any of

struct timespec { int32_t tv_sec; int32_t tv_nsec; };
struct timespec { int64_t tv_sec; int64_t tv_nsec; };
struct timespec { int64_t tv_sec; int32_t __padding; int32_t tv_nsec; }
struct timespec { int64_t tv_sec; int32_t tv_nsec; int32_t __padding; }

These may be incompatible with using bare `long` for tv_nsec in the user space headers.  The problem cases are when the kernel expects option 2 but user space's `long` is only 32 bits wide, or if the kernel expects option 3 or 4 when `long` is 64 bits wide and the endianness of `long` doesn't agree with the position of the padding.  I don't remember which ABIs actually manifest an incompatibility, but I remember this being discussed repeatedly during the project to make 64-bit time_t available to ILP32 ABIs.

The C library CANNOT paper over the incompatibility by converting formats in the syscall wrappers, because `struct timespec` objects get embedded in an unbounded set of `ioctl` parameter blocks, `sendmsg` ancillary data structures, etc. etc. etc.  We can't even make a list.

Personally I consider the situation a defect in POSIX and C11; the definition of `struct timespec` should always have been

struct timespec { time_t tv_sec; nsec_t tv_nsec; };

where nsec_t is an integer type that can represent the range [0, 10**9).  Last time this came up, Joseph didn't think there was any chance of persuading either the Austin Group or WG14 to make this change, unfortunately.

zw

Re: [PATCH v3 3/3] system_data_types.7: note struct timespec::tv_nsec type for x32 and portability

[Alejandro Colomar (man-pages)]

On 12/7/21 00:31, наб wrote:
> On Mon, Dec 06, 2021 at 11:56:19PM +0100, Alejandro Colomar (man-pages) wrote:
>> I (more or less) understand the deduction of why that happens on certain
>> systems,  what I was wondering is if there's an intention behind that, or if
>> it's just an accident.  Why not just use plain 'long' always as POSIX says
>> it should be.  I don't see the feature part of this bug.
>>
>> 'long' is at least 32 bits, which
>> is enough for 1G.  Was there ever a good reason for glibc to use 64 bits?  It
>> seems like a wrong decision that is maintained for consistency.   If that's it
>> I'm in favor of mentioning it in a Bugs section (as Jakub proposed), instead
>> of a Notes section.  Maybe someone from glibc may give a rationale for this
>> deviation from POSIX.
> AFAICT, as you say, this isn't much more than a side-effect
> of using the amd64 ABI and not being too careful with how it interacts
> with the POSIX requirement (it's entirely feasible to re-wrap
> a user-space timespec, or even use the bitfield trick glibc
> already does to not have to do it), but then I'm far from an expert;
> indeed, maybe someone with more glibc involvement will know.
> 
> Updating to Bugs for now, maybe we can downgrade later.

Sure,  I'll delay applying this patch to allow for comments.

> 
>> Maybe preceding the paragraph with "In/On (I never
>> knew if there goes in or on) glibc," would do.
> Out of those two: on (in would point to glibc internals),
> but I'd actually go for "Under", here, since glibc is an over-arching
> (literally) universe-defining context, rather than an add-on,
> or something build your program on top of.

Interesting :)

> 
> For my own curiosity: which preposition would you use in Spanish here?
> 
I would say "en" (which normally translates to "in" in English).

Under some circumstances I might use "con" ("with" in English), but it 
would be rarer.

>> On 12/6/21 23:03, наб wrote:
>>> +.br
>> We don't use '.br'.  In this case I think just continuing in the same
>> paragraph would be fine (i.e., no break at all).
>>> +In reality, the field ends up being defined as:
>> Here I'd add a blank line with '.PP'.
>>> +.EX
>>> +.in +4
>> Please, revert the order of .in and .EX.
>> It's meaningless, but I prefer consistency.  See man-pages(7), which shows
>> this order, and also the following:
>>
>> $ grep -rn '^\.EX$' -A1 man? | grep '\.in' | wc -l
>> 2
>> $ grep -rn '^\.EX$' -B1 man? | grep '\.in' | wc -l
>> 1048
>>
>>
>> One of those 2 cases was this patch.
> Sure, sure, and sure
> 
>>> +#if !(__x86_64__ && __ILP32__ /* == x32 */)
>>
>> Now I notice:
>>
>> Shouldn't this use defined()?
>>
>> #if !(defined(__x86_64__) && defined(__ILP32__) /* == x32 */)
> Eeeeh, not really? That's functionally identical but, like,
> very verbose for no good reason.

Are those defined to actual values?  Or are they defined just empty?
I thought they were empty (but have never used those macros, so don't 
know at all), in which case it would matter:


user@sqli:~/src/test$ cat defined.c
#define C
#define D

#if A && B
#warning AB
#else
#warning notAB
#endif

#if C && D
#warning CD
#else
#warning notCD
#endif
user@sqli:~/src/test$ cc -Wall -Wextra defined.c
defined.c:7:2: warning: #warning notAB [-Wcpp]
     7 | #warning notAB
       |  ^~~~~~~
defined.c:10:7: error: operator '&&' has no left operand
    10 | #if C && D
       |       ^~
defined.c:13:2: warning: #warning notCD [-Wcpp]
    13 | #warning notCD
       |  ^~~~~~~



Cheers,
Alex


-- 
Alejandro Colomar
Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/

Re: [PATCH v3 3/3] system_data_types.7: note struct timespec::tv_nsec type for x32 and portability

[Alejandro Colomar (man-pages)]

Hi, Zack!

On 12/7/21 01:18, Zack Weinberg wrote:
> Depending on the kernel, the architecture, the ABI, and the state of _TIME_BITS, the format of `struct timespec` as expected by system calls may be any of
> 
> struct timespec { int32_t tv_sec; int32_t tv_nsec; };
> struct timespec { int64_t tv_sec; int64_t tv_nsec; };
> struct timespec { int64_t tv_sec; int32_t __padding; int32_t tv_nsec; }
> struct timespec { int64_t tv_sec; int32_t tv_nsec; int32_t __padding; }
> 
> These may be incompatible with using bare `long` for tv_nsec in the user space headers.  The problem cases are when the kernel expects option 2 but user space's `long` is only 32 bits wide, or if the kernel expects option 3 or 4 when `long` is 64 bits wide and the endianness of `long` doesn't agree with the position of the padding.  I don't remember which ABIs actually manifest an incompatibility, but I remember this being discussed repeatedly during the project to make 64-bit time_t available to ILP32 ABIs.
> 
> The C library CANNOT paper over the incompatibility by converting formats in the syscall wrappers, because `struct timespec` objects get embedded in an unbounded set of `ioctl` parameter blocks, `sendmsg` ancillary data structures, etc. etc. etc.  We can't even make a list.
> 
> Personally I consider the situation a defect in POSIX and C11; the definition of `struct timespec` should always have been
> 
> struct timespec { time_t tv_sec; nsec_t tv_nsec; };

I can agree with that.

> 
> where nsec_t is an integer type that can represent the range [0, 10**9).  Last time this came up, Joseph didn't think there was any chance of persuading either the Austin Group or WG14 to make this change, unfortunately.

Okay, I can understand that.

How about defining ns[ec[onds]]_t (and possibly ms[ec[onds]]_t) in 
glibc, and using them wherever ms or ns are used, such as in this case, 
instead of a random type?

I think since glibc already ignores POSIX, it could go a bit further and 
use an interface that is consistent (use nseconds_t always instead of 
sometimes long sometimes long long), more readable than POSIX (a 
variable of type nseconds_t already tells you a lot about it), and again 
consistent (since we already have useconds_t, using nseconds_t would add 
consistency to the types used in <time.h>.

Also, that would make clear that you should cast it to long to print it 
(and using pointers would already make sure that you have the correct 
pointer using a nseconds_t one).  And of course, if a program wants to 
be portable, it can just typedef it to long when using other libcs not 
using nseconds_t.

Thanks,
Alex


-- 
Alejandro Colomar
Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/

Re: [PATCH v3 3/3] system_data_types.7: note struct timespec::tv_nsec type for x32 and portability

[наб]

[-- Attachment #1: Type: text/plain, Size: 1685 bytes --]

On Tue, Dec 07, 2021 at 01:38:34AM +0100, Alejandro Colomar (man-pages) wrote:
> On 12/7/21 00:31, наб wrote:
> > For my own curiosity: which preposition would you use in Spanish here?
> I would say "en" (which normally translates to "in" in English).
> 
> Under some circumstances I might use "con" ("with" in English), but it would
> be rarer.
Huh! That's neat, I wouldn't've expected it!

> > Eeeeh, not really? That's functionally identical but, like,
> > very verbose for no good reason.
> Are those defined to actual values?  Or are they defined just empty?
> I thought they were empty (but have never used those macros, so don't know
> at all), in which case it would matter:
They're either undefined (=> 0) or defined to a truthy value
(1, realistically, but you see versions in some other APIs).
It'd also be logically valid to define them to 0,
but this is rare because people do define-checks.

A "default" -D macro, like all features are, is 1,
so your test program is more accurately represented as:
-- >8 --
nabijaczleweli@tarta:~/uwu$ cat defined.c
#if A && B
#warning AB
#else
#warning notAB
#endif

#if C && D
#warning CD
#else
#warning notCD
#endif
nabijaczleweli@tarta:~/uwu$ cc -E defined.c > /dev/null
defined.c:4:2: warning: notAB [-W#warnings]
#warning notAB
 ^
defined.c:10:2: warning: notCD [-W#warnings]
#warning notCD
 ^
2 warnings generated.
nabijaczleweli@tarta:~/uwu$ cc -DA -DB -E defined.c > /dev/null
defined.c:2:2: warning: AB [-W#warnings]
#warning AB
 ^
defined.c:10:2: warning: notCD [-W#warnings]
#warning notCD
 ^
2 warnings generated.
-- >8 --

Which behaves as expected.

Best,
наб

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 3/3] system_data_types.7: note struct timespec::tv_nsec type for x32 and portability

[Alejandro Colomar (man-pages)]

On 12/7/21 02:08, наб wrote:
> On Tue, Dec 07, 2021 at 01:38:34AM +0100, Alejandro Colomar (man-pages) wrote:
>> On 12/7/21 00:31, наб wrote:
>>> For my own curiosity: which preposition would you use in Spanish here?
>> I would say "en" (which normally translates to "in" in English).
>>
>> Under some circumstances I might use "con" ("with" in English), but it would
>> be rarer.
> Huh! That's neat, I wouldn't've expected it!
> 
>>> Eeeeh, not really? That's functionally identical but, like,
>>> very verbose for no good reason.
>> Are those defined to actual values?  Or are they defined just empty?
>> I thought they were empty (but have never used those macros, so don't know
>> at all), in which case it would matter:
> They're either undefined (=> 0) or defined to a truthy value
> (1, realistically, but you see versions in some other APIs).
> It'd also be logically valid to define them to 0,
> but this is rare because people do define-checks.
> 
> A "default" -D macro, like all features are, is 1,

Huh, I didn't expect that.  Good to know it.
You can see that I don't use them a lot :)

So, nothing else to add to the patch; LGTM.  I'll leave up to you to 
decide if Notes or Bugs, and will wait also until tomorrow to merge it 
to see if someone has anything to add.

Thanks a lot!

Kind regards,
Alex


-- 
Alejandro Colomar
Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/

Re: [PATCH v3 3/3] system_data_types.7: note struct timespec::tv_nsec type for x32 and portability

[наб]

[-- Attachment #1: Type: text/plain, Size: 5382 bytes --]

On Tue, Dec 07, 2021 at 01:52:52AM +0100, Alejandro Colomar (man-pages) wrote:
> On 12/7/21 01:18, Zack Weinberg wrote:
> > Depending on the kernel, the architecture, the ABI, and the state of _TIME_BITS, the format of `struct timespec` as expected by system calls may be any of
> > 
> > struct timespec { int32_t tv_sec; int32_t tv_nsec; };
> > struct timespec { int64_t tv_sec; int64_t tv_nsec; };
> > struct timespec { int64_t tv_sec; int32_t __padding; int32_t tv_nsec; }
> > struct timespec { int64_t tv_sec; int32_t tv_nsec; int32_t __padding; }
> > 
> > These may be incompatible with using bare `long` for tv_nsec in the user space headers.  The problem cases are when the kernel expects option 2 but user space's `long` is only 32 bits wide, or if the kernel expects option 3 or 4 when `long` is 64 bits wide and the endianness of `long` doesn't agree with the position of the padding.  I don't remember which ABIs actually manifest an incompatibility, but I remember this being discussed repeatedly during the project to make 64-bit time_t available to ILP32 ABIs.
> > 
> > The C library CANNOT paper over the incompatibility by converting formats in the syscall wrappers, because `struct timespec` objects get embedded in an unbounded set of `ioctl` parameter blocks, `sendmsg` ancillary data structures, etc. etc. etc.  We can't even make a list.
This I forgot to take into account, mea culpa!

> > Personally I consider the situation a defect in POSIX and C11; the definition of `struct timespec` should always have been
> > 
> > struct timespec { time_t tv_sec; nsec_t tv_nsec; };
> > 
> > where nsec_t is an integer type that can represent the range [0, 10**9).  Last time this came up, Joseph didn't think there was any chance of persuading either the Austin Group or WG14 to make this change, unfortunately.
Sadly, agree on both fronts.

Looking through "timespec" on Aardvark for prior art reveals nothing,
except for a likely resolution to any proposal of this sort:
> Although we agree that it would have been better if these functions had
> been designed this way to begin with, we believe that making the change
> now will break existing, conforming code with no real benefit. 

Considering this is, as outlined above, a real ABI-meets-standard moment
that affects all users of the Linux syscall ABI, I rolled back to Notes,
replaced glibc references with a "this is unfortunate but unfixable"
stanza, and a link to this post in the libc-alpha archive in the comment.

Regrettably, I think this is the best we can do,
save for committing a plan9. Scissor-patch below.

Cheers,
наб

-- >8 --
There are three files that govern userspace struct timespec on glibc:
1. bits/wordsize.h, defining:
   (a) __WORDSIZE to 32 on ILP32 and 64 on LP64
   (b) on x32: __SYSCALL_WORDSIZE to 64
2. bits/timesize.h, defining
   (a) __TIMESIZE to __WORDSIZE, except on x32 where it's 64
3. bits/types/struct_timespec.h, declaring struct timespec as:
     struct timespec
     {
      __time_t tv_sec;      /* Seconds.  */
     #if __WORDSIZE == 64 \
      || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64) \
      || __TIMESIZE == 32
      __syscall_slong_t tv_nsec;    /* Nanoseconds.  */
     #else
     # if __BYTE_ORDER == __BIG_ENDIAN
      int: 32;           /* Padding.  */
      long int tv_nsec;  /* Nanoseconds.  */
     # else
      long int tv_nsec;  /* Nanoseconds.  */
      int: 32;           /* Padding.  */
     # endif
     #endif
     };
   this has two side-effects: struct timespec
   (a) is always sizeof==time_t+8, and
   (b) has tv_nsec as __syscall_slong_t
       *and* !is_same<__syscall_slong_t, long>
       if using LP64 syscalls on an ILP32 system, i.e. on x32.

This means, that the simplified
  struct timespec {
      time_t  tv_sec;  /* Seconds */
      long    tv_nsec; /* Nanoseconds [0 .. 999999999] */
  };
declaration is *invalid* for x32,
where struct timespec::tv_nsec is an int64_t (long long).

Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
---
 man7/system_data_types.7 | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/man7/system_data_types.7 b/man7/system_data_types.7
index 1e6a3f74c..46fcf013d 100644
--- a/man7/system_data_types.7
+++ b/man7/system_data_types.7
@@ -1553,6 +1553,36 @@ Describes times in seconds and nanoseconds.
 .IR "Conforming to" :
 C11 and later; POSIX.1-2001 and later.
 .PP
+.IR Notes :
+.I tv_nsec
+is the
+.I syscall
+long, though this affects only fringe architectures like X32,
+which is ILP32, but uses the LP64 AMD64 syscall ABI.
+In reality, the field ends up being defined as:
+.PP
+.in +4
+.EX
+#if __x86_64__ && __ILP32__  /* == x32 */
+    long long tv_nsec;
+#else
+    long      tv_nsec;
+#endif
+.EE
+.in
+.PP
+This is one of the many unfortunate side-effects of the intersection of
+.I tv_nsec
+being defined as
+.IR long ,
+and cannot be addressed while preserving backward compatibility.
+.\" https://sourceware.org/pipermail/libc-alpha/2021-December/133702.html
+However, as even a 32-bit
+.I long
+can hold the entire
+.I tv_nsec
+range, it's always safe to forcibly down-cast it to the standard type.
+.PP
 .IR "See also" :
 .BR clock_gettime (2),
 .BR clock_nanosleep (2),
-- 
2.30.2


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 3/3] system_data_types.7: note struct timespec::tv_nsec type for x32 and portability

[Joseph Myers]

On Tue, 7 Dec 2021, наб via Libc-alpha wrote:

> Looking through "timespec" on Aardvark for prior art reveals nothing,
> except for a likely resolution to any proposal of this sort:
> > Although we agree that it would have been better if these functions had
> > been designed this way to begin with, we believe that making the change
> > now will break existing, conforming code with no real benefit. 

Geoff Clare said (austin-group-l, Thu, 29 May 2014 16:20:22 +0100):

  C11 requires tv_nsec to be type long, which means that if we change
  it to be a new snseconds_t type in Issue 8, we would have to require
  that snseconds_t is defined as long in order not to conflict with C11.

and Rich Felker (Thu, 29 May 2014 13:08:59 -0400):

  This is just a linux kernel bug which needs to be fixed. They have a
  number of other such bugs in x32 too. It's possible to work around it
  in userspace on the library side (we do this in musl libc) but it's a
  bit costly/painful and glibc does not do so yet. There's an open bug
  for it which I filed:

and I don't see any other responses in that discussion.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH v3 3/3] system_data_types.7: note struct timespec::tv_nsec type for x32 and portability

[Florian Weimer]

* Joseph Myers:

> On Tue, 7 Dec 2021, наб via Libc-alpha wrote:
>
>> Looking through "timespec" on Aardvark for prior art reveals nothing,
>> except for a likely resolution to any proposal of this sort:
>> > Although we agree that it would have been better if these functions had
>> > been designed this way to begin with, we believe that making the change
>> > now will break existing, conforming code with no real benefit. 
>
> Geoff Clare said (austin-group-l, Thu, 29 May 2014 16:20:22 +0100):
>
>   C11 requires tv_nsec to be type long, which means that if we change
>   it to be a new snseconds_t type in Issue 8, we would have to require
>   that snseconds_t is defined as long in order not to conflict with C11.
>
> and Rich Felker (Thu, 29 May 2014 13:08:59 -0400):
>
>   This is just a linux kernel bug which needs to be fixed. They have a
>   number of other such bugs in x32 too. It's possible to work around it
>   in userspace on the library side (we do this in musl libc) but it's a
>   bit costly/painful and glibc does not do so yet. There's an open bug
>   for it which I filed:
>
> and I don't see any other responses in that discussion.

This came up again during the time64 work.  The kernel was eventually
changed to ignore the padding, so userspace can use a long int type.
x32 wasn't changed for backwards compatibility reasons.

Thanks,
Florian