Fix building tst-socket-timestamp-compat on older Linux

Message ID 20220211191638.324672-1-tuliom@linux.ibm.com
State Dropped
Headers
Series Fix building tst-socket-timestamp-compat on older Linux |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent
dj/TryBot-32bit success Build for i686

Commit Message

Tulio Magno Quites Machado Filho Feb. 11, 2022, 7:16 p.m. UTC
  Work around an error caused by not having SO_TIMESTAMP_NEW,
SO_TIMESTAMPNS_NEW, SO_TIMESTAMP_OLD and SO_TIMESTAMPNS_OLD defined.

Tested on ppc64le and x86_64 using Linux 4.16 and 4.18.
---
 .../sysv/linux/tst-socket-timestamp-compat.c    | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
  

Comments

Florian Weimer Feb. 11, 2022, 7:29 p.m. UTC | #1
* Tulio Magno Quites Machado Filho via Libc-alpha:

> Work around an error caused by not having SO_TIMESTAMP_NEW,
> SO_TIMESTAMPNS_NEW, SO_TIMESTAMP_OLD and SO_TIMESTAMPNS_OLD defined.
>
> Tested on ppc64le and x86_64 using Linux 4.16 and 4.18.
> ---
>  .../sysv/linux/tst-socket-timestamp-compat.c    | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/sysdeps/unix/sysv/linux/tst-socket-timestamp-compat.c b/sysdeps/unix/sysv/linux/tst-socket-timestamp-compat.c
> index 0ff1a214e6..68b0a0e412 100644
> --- a/sysdeps/unix/sysv/linux/tst-socket-timestamp-compat.c
> +++ b/sysdeps/unix/sysv/linux/tst-socket-timestamp-compat.c
> @@ -23,6 +23,23 @@
>  #include <support/xunistd.h>
>  #include <stdbool.h>
>  
> +/* The following macros are only available on ABIs that support 32 bit time_t.
> +   This test has a runtime check to guarantee it runs only on these ABIs.
> +   However, we need to create fake values to guarantee this test is built
> +   for all the ABIs.  */

Isn't this related to the kernel header version, and not just time32
support?

Thanks,
Florian
  
Tulio Magno Quites Machado Filho Feb. 11, 2022, 9:25 p.m. UTC | #2
Florian Weimer via Libc-alpha <libc-alpha@sourceware.org> writes:

> * Tulio Magno Quites Machado Filho via Libc-alpha:
>
>> diff --git a/sysdeps/unix/sysv/linux/tst-socket-timestamp-compat.c b/sysdeps/unix/sysv/linux/tst-socket-timestamp-compat.c
>> index 0ff1a214e6..68b0a0e412 100644
>> --- a/sysdeps/unix/sysv/linux/tst-socket-timestamp-compat.c
>> +++ b/sysdeps/unix/sysv/linux/tst-socket-timestamp-compat.c
>> @@ -23,6 +23,23 @@
>>  #include <support/xunistd.h>
>>  #include <stdbool.h>
>>  
>> +/* The following macros are only available on ABIs that support 32 bit time_t.
>> +   This test has a runtime check to guarantee it runs only on these ABIs.
>> +   However, we need to create fake values to guarantee this test is built
>> +   for all the ABIs.  */
>
> Isn't this related to the kernel header version, and not just time32
> support?

The runtime test does depend on the kernel header version because it depends
on the time_t size.
However, the definition of these macros depends only on the value of
__TIMESIZE, which doesn't depend on the kernel header [1].

[1] https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/powerpc/bits/socket-constants.h;hb=HEAD#l50
  
Florian Weimer Feb. 11, 2022, 9:40 p.m. UTC | #3
* Tulio Magno Quites Machado Filho:

> Florian Weimer via Libc-alpha <libc-alpha@sourceware.org> writes:
>
>> * Tulio Magno Quites Machado Filho via Libc-alpha:
>>
>>> diff --git a/sysdeps/unix/sysv/linux/tst-socket-timestamp-compat.c b/sysdeps/unix/sysv/linux/tst-socket-timestamp-compat.c
>>> index 0ff1a214e6..68b0a0e412 100644
>>> --- a/sysdeps/unix/sysv/linux/tst-socket-timestamp-compat.c
>>> +++ b/sysdeps/unix/sysv/linux/tst-socket-timestamp-compat.c
>>> @@ -23,6 +23,23 @@
>>>  #include <support/xunistd.h>
>>>  #include <stdbool.h>
>>>  
>>> +/* The following macros are only available on ABIs that support 32 bit time_t.
>>> +   This test has a runtime check to guarantee it runs only on these ABIs.
>>> +   However, we need to create fake values to guarantee this test is built
>>> +   for all the ABIs.  */
>>
>> Isn't this related to the kernel header version, and not just time32
>> support?
>
> The runtime test does depend on the kernel header version because it depends
> on the time_t size.
> However, the definition of these macros depends only on the value of
> __TIMESIZE, which doesn't depend on the kernel header [1].
>
> [1] https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/powerpc/bits/socket-constants.h;hb=HEAD#l50

What I meant is this: I see the same build failure on aarch64 with our
4.18-derived headers, but not with our 5.14-derived headers.  This is on
the same architecture, so it can't be glibc's time32 support state.

Hmm, or is it that ILP32 aarch64 support was upstreamed between the two
kernel releases?

Thanks,
Florian
  
Adhemerval Zanella Feb. 14, 2022, 1:08 p.m. UTC | #4
On 11/02/2022 18:40, Florian Weimer via Libc-alpha wrote:
> * Tulio Magno Quites Machado Filho:
> 
>> Florian Weimer via Libc-alpha <libc-alpha@sourceware.org> writes:
>>
>>> * Tulio Magno Quites Machado Filho via Libc-alpha:
>>>
>>>> diff --git a/sysdeps/unix/sysv/linux/tst-socket-timestamp-compat.c b/sysdeps/unix/sysv/linux/tst-socket-timestamp-compat.c
>>>> index 0ff1a214e6..68b0a0e412 100644
>>>> --- a/sysdeps/unix/sysv/linux/tst-socket-timestamp-compat.c
>>>> +++ b/sysdeps/unix/sysv/linux/tst-socket-timestamp-compat.c
>>>> @@ -23,6 +23,23 @@
>>>>  #include <support/xunistd.h>
>>>>  #include <stdbool.h>
>>>>  
>>>> +/* The following macros are only available on ABIs that support 32 bit time_t.
>>>> +   This test has a runtime check to guarantee it runs only on these ABIs.
>>>> +   However, we need to create fake values to guarantee this test is built
>>>> +   for all the ABIs.  */
>>>
>>> Isn't this related to the kernel header version, and not just time32
>>> support?
>>
>> The runtime test does depend on the kernel header version because it depends
>> on the time_t size.
>> However, the definition of these macros depends only on the value of
>> __TIMESIZE, which doesn't depend on the kernel header [1].
>>
>> [1] https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/powerpc/bits/socket-constants.h;hb=HEAD#l50
> 
> What I meant is this: I see the same build failure on aarch64 with our
> 4.18-derived headers, but not with our 5.14-derived headers.  This is on
> the same architecture, so it can't be glibc's time32 support state.

There are two issue in fact, one is that socket-constants.h is only included
for !__USE_MISC, which is then disable for _DEFAULT_SOURCE (enabled with
_GNU_SOURCE).  The another issue is socket-constants.h only defines
the *_OLD and *_NEW variant in some cases, different than kernel header.

I think we should always include socket-constants.h, only define the values
if there are not already defined by the kernel headers, and always define
the *_OLD and *_NEW constants.  Something like the below (it requires to
adapt to other arch-specific socket-constants.h as well).

> 
> Hmm, or is it that ILP32 aarch64 support was upstreamed between the two
> kernel releases?
> 
No and most likely it will never will.

--

diff --git a/sysdeps/unix/sysv/linux/bits/socket-constants.h b/sysdeps/unix/sysv/linux/bits/socket-constants.h
index cbbc2665b2..3ffc2cb9fa 100644
--- a/sysdeps/unix/sysv/linux/bits/socket-constants.h
+++ b/sysdeps/unix/sysv/linux/bits/socket-constants.h
@@ -22,60 +22,118 @@
 
 #include <bits/timesize.h>
 
-#define SOL_SOCKET 1
-#define SO_ACCEPTCONN 30
-#define SO_BROADCAST 6
-#define SO_DONTROUTE 5
-#define SO_ERROR 4
-#define SO_KEEPALIVE 9
-#define SO_LINGER 13
-#define SO_OOBINLINE 10
-#define SO_RCVBUF 8
-#define SO_RCVLOWAT 18
-#define SO_REUSEADDR 2
-#define SO_SNDBUF 7
-#define SO_SNDLOWAT 19
-#define SO_TYPE 3
+#ifndef SOL_SOCKET
+# define SOL_SOCKET 1
+#endif
+#ifndef SO_ACCEPTCONN
+# define SO_ACCEPTCONN 30
+#endif
+#ifndef SO_BROADCAST
+# define SO_BROADCAST 6
+#endif
+#ifndef SO_DONTROUTE
+# define SO_DONTROUTE 5
+#endif
+#ifdef SO_ERROR
+# define SO_ERROR 4
+#endif
+#ifndef SO_KEEPALIVE
+# define SO_KEEPALIVE 9
+#endif
+#ifndef SO_LINGER
+# define SO_LINGER 13
+#endif
+#ifndef SO_OOBINLINE
+# define SO_OOBINLINE 10
+#endif
+#ifndef SO_RCVBUF
+# define SO_RCVBUF 8
+#endif
+#ifndef SO_RCVLOWAT
+# define SO_RCVLOWAT 18
+#endif
+#ifndef SO_REUSEADDR
+# define SO_REUSEADDR 2
+#endif
+#ifndef SO_SNDBUF
+# define SO_SNDBUF 7
+#endif
+#ifndef SO_SNDLOWAT
+# define SO_SNDLOWAT 19
+#endif
+#ifndef SO_TYPE
+# define SO_TYPE 3
+#endif
+
+#ifndef SO_RCVTIMEO_OLD
+# define SO_RCVTIMEO_OLD 20
+#endif
+#ifndef SO_SNDTIMEO_OLD
+# define SO_SNDTIMEO_OLD 21
+#endif
+#ifndef SO_RCVTIMEO_NEW
+# define SO_RCVTIMEO_NEW 66
+#endif
+#ifndef SO_SNDTIMEO_NEW
+# define SO_SNDTIMEO_NEW 67
+#endif
+
+#ifndef SO_TIMESTAMP_OLD
+# define SO_TIMESTAMP_OLD 29
+#endif
+#ifndef SO_TIMESTAMPNS_OLD
+# define SO_TIMESTAMPNS_OLD 35
+#endif
+#ifndef SO_TIMESTAMPING_OLD
+# define SO_TIMESTAMPING_OLD 37
+#endif
+
+#ifndef SO_TIMESTAMP_NEW
+# define SO_TIMESTAMP_NEW 63
+#endif
+#ifndef SO_TIMESTAMPNS_NEW
+# define SO_TIMESTAMPNS_NEW 64
+#endif
+#ifndef SO_TIMESTAMPING_NEW
+# define SO_TIMESTAMPING_NEW 65
+#endif
 
 #if (__TIMESIZE == 64 && __WORDSIZE == 32 \
      && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32))
-# define SO_RCVTIMEO 66
-# define SO_SNDTIMEO 67
-# define SO_TIMESTAMP 63
-# define SO_TIMESTAMPNS 64
-# define SO_TIMESTAMPING 65
+# ifndef SO_RCVTIMEO
+#  define SO_RCVTIMEO SO_RCVTIMEO_NEW
+# endif
+# ifndef SO_SNDTIMEO
+#  define SO_SNDTIMEO SO_SNDTIMEO_NEW
+# endif
+# ifndef SO_TIMESTAMP
+#  define SO_TIMESTAMP SO_TIMESTAMP_NEW
+# endif
+# ifndef SO_TIMESTAMPNS
+#  define SO_TIMESTAMPNS SO_TIMESTAMPNS_NEW
+# endif
+# ifndef SO_TIMESTAMPING
+#  define SO_TIMESTAMPING SO_TIMESTAMPING_NEW
+# endif
 #else
-# if __TIMESIZE == 64
-#  define SO_RCVTIMEO 20
-#  define SO_SNDTIMEO 21
-#  define SO_TIMESTAMP 29
-#  define SO_TIMESTAMPNS 35
-#  define SO_TIMESTAMPING 37
-# else
-#  define SO_RCVTIMEO_OLD 20
-#  define SO_SNDTIMEO_OLD 21
-#  define SO_RCVTIMEO_NEW 66
-#  define SO_SNDTIMEO_NEW 67
-
-#  define SO_TIMESTAMP_OLD 29
-#  define SO_TIMESTAMPNS_OLD 35
-#  define SO_TIMESTAMPING_OLD 37
-#  define SO_TIMESTAMP_NEW 63
-#  define SO_TIMESTAMPNS_NEW 64
-#  define SO_TIMESTAMPING_NEW 65
-
-#  ifdef __USE_TIME_BITS64
-#   define SO_RCVTIMEO SO_RCVTIMEO_NEW
-#   define SO_SNDTIMEO SO_SNDTIMEO_NEW
-#   define SO_TIMESTAMP SO_TIMESTAMP_NEW
-#   define SO_TIMESTAMPNS SO_TIMESTAMPNS_NEW
-#   define SO_TIMESTAMPING SO_TIMESTAMPING_NEW
-#  else
-#   define SO_RCVTIMEO SO_RCVTIMEO_OLD
-#   define SO_SNDTIMEO SO_SNDTIMEO_OLD
-#   define SO_TIMESTAMP SO_TIMESTAMP_OLD
-#   define SO_TIMESTAMPNS SO_TIMESTAMPNS_OLD
-#   define SO_TIMESTAMPING SO_TIMESTAMPING_OLD
-#  endif
+# ifndef SO_RCVTIMEO
+#  define SO_RCVTIMEO (sizeof(time_t) == sizeof(__time_t) \
+		       ? SO_RCVTIMEO_OLD : SO_RCVTIMEO_NEW)
+# endif
+# ifndef SO_SNDTIMEO
+#  define SO_SNDTIMEO (sizeof(time_t) == sizeof(__time_t) \
+		       ? SO_SNDTIMEO_OLD : SO_SNDTIMEO_NEW)
+# endif
+# ifndef SO_TIMESTAMP
+#  define SO_TIMESTAMP (sizeof(time_t) == sizeof(__time_t) \
+			? SO_TIMESTAMP_OLD : SO_TIMESTAMP_NEW)
+# endif
+# ifndef SO_TIMESTAMPNS
+#  define SO_TIMESTAMPNS (sizeof(time_t) == sizeof(__time_t) \
+			  ? SO_TIMESTAMPNS_OLD : SO_TIMESTAMPNS_NEW)
+# endif
+# ifndef SO_TIMESTAMPING
+#  define SO_TIMESTAMPING (sizeof(time_t) == sizeof(__time_t) \
+			   ? SO_TIMESTAMPING_OLD : SO_TIMESTAMPING_NEW)
 # endif
 #endif
diff --git a/sysdeps/unix/sysv/linux/bits/socket.h b/sysdeps/unix/sysv/linux/bits/socket.h
index 79da9e7597..db4e0c1c98 100644
--- a/sysdeps/unix/sysv/linux/bits/socket.h
+++ b/sysdeps/unix/sysv/linux/bits/socket.h
@@ -354,10 +354,12 @@ struct ucred
 #ifdef __USE_MISC
 # include <bits/types/time_t.h>
 # include <asm/socket.h>
-#else
+#endif
+
+#ifndef SO_DEBUG
 # define SO_DEBUG 1
-# include <bits/socket-constants.h>
 #endif
+#include <bits/socket-constants.h>
 
 /* Structure used to manipulate the SO_LINGER option.  */
 struct linger
  
Adhemerval Zanella Feb. 14, 2022, 1:12 p.m. UTC | #5
On 14/02/2022 10:08, Adhemerval Zanella wrote:
> 
> 
> On 11/02/2022 18:40, Florian Weimer via Libc-alpha wrote:
>> * Tulio Magno Quites Machado Filho:
>>
>>> Florian Weimer via Libc-alpha <libc-alpha@sourceware.org> writes:
>>>
>>>> * Tulio Magno Quites Machado Filho via Libc-alpha:
>>>>
>>>>> diff --git a/sysdeps/unix/sysv/linux/tst-socket-timestamp-compat.c b/sysdeps/unix/sysv/linux/tst-socket-timestamp-compat.c
>>>>> index 0ff1a214e6..68b0a0e412 100644
>>>>> --- a/sysdeps/unix/sysv/linux/tst-socket-timestamp-compat.c
>>>>> +++ b/sysdeps/unix/sysv/linux/tst-socket-timestamp-compat.c
>>>>> @@ -23,6 +23,23 @@
>>>>>  #include <support/xunistd.h>
>>>>>  #include <stdbool.h>
>>>>>  
>>>>> +/* The following macros are only available on ABIs that support 32 bit time_t.
>>>>> +   This test has a runtime check to guarantee it runs only on these ABIs.
>>>>> +   However, we need to create fake values to guarantee this test is built
>>>>> +   for all the ABIs.  */
>>>>
>>>> Isn't this related to the kernel header version, and not just time32
>>>> support?
>>>
>>> The runtime test does depend on the kernel header version because it depends
>>> on the time_t size.
>>> However, the definition of these macros depends only on the value of
>>> __TIMESIZE, which doesn't depend on the kernel header [1].
>>>
>>> [1] https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/powerpc/bits/socket-constants.h;hb=HEAD#l50
>>
>> What I meant is this: I see the same build failure on aarch64 with our
>> 4.18-derived headers, but not with our 5.14-derived headers.  This is on
>> the same architecture, so it can't be glibc's time32 support state.
> 
> There are two issue in fact, one is that socket-constants.h is only included
> for !__USE_MISC, which is then disable for _DEFAULT_SOURCE (enabled with
> _GNU_SOURCE).  The another issue is socket-constants.h only defines
> the *_OLD and *_NEW variant in some cases, different than kernel header.
> 
> I think we should always include socket-constants.h, only define the values
> if there are not already defined by the kernel headers, and always define
> the *_OLD and *_NEW constants.  Something like the below (it requires to
> adapt to other arch-specific socket-constants.h as well).
> 

Another way is to move the test to a internal one and use the values
on socket-constants-time64.h instead.
  

Patch

diff --git a/sysdeps/unix/sysv/linux/tst-socket-timestamp-compat.c b/sysdeps/unix/sysv/linux/tst-socket-timestamp-compat.c
index 0ff1a214e6..68b0a0e412 100644
--- a/sysdeps/unix/sysv/linux/tst-socket-timestamp-compat.c
+++ b/sysdeps/unix/sysv/linux/tst-socket-timestamp-compat.c
@@ -23,6 +23,23 @@ 
 #include <support/xunistd.h>
 #include <stdbool.h>
 
+/* The following macros are only available on ABIs that support 32 bit time_t.
+   This test has a runtime check to guarantee it runs only on these ABIs.
+   However, we need to create fake values to guarantee this test is built
+   for all the ABIs.  */
+#ifndef SO_TIMESTAMP_NEW
+# define SO_TIMESTAMP_NEW 0
+#endif
+#ifndef SO_TIMESTAMPNS_NEW
+# define SO_TIMESTAMPNS_NEW 0
+#endif
+#ifndef SO_TIMESTAMP_OLD
+# define SO_TIMESTAMP_OLD 0
+#endif
+#ifndef SO_TIMESTAMPNS_OLD
+# define SO_TIMESTAMPNS_OLD 0
+#endif
+
 /* AF_INET socket and address used to receive data.  */
 static int srv;
 static struct sockaddr_in srv_addr;