public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] socket: Use may_alias on sockaddr structs (bug 19622)
@ 2024-05-07  9:29 Florian Weimer
  2024-05-07 11:38 ` Sam James
  0 siblings, 1 reply; 4+ messages in thread
From: Florian Weimer @ 2024-05-07  9:29 UTC (permalink / raw)
  To: libc-alpha

This supports common coding paddings.  GCC before version 8
rejects the may_alias attribute on a struct definition if it was
not present in a previous forward declaration, so this attribute
can only be conditionally applied.

Tested on i686-linux-gnu, x86_64-linux-gnu.  Built with
build-many-glibcs.py.

Suggested-by: Marek Polacek <polacek@redhat.com>
Suggested-by: Jakub Jelinek <jakub@redhat.com>

---
 bits/socket.h                           | 4 ++--
 inet/netinet/in.h                       | 8 +++++---
 misc/sys/cdefs.h                        | 9 +++++++++
 socket/sys/un.h                         | 2 +-
 sysdeps/mach/hurd/bits/socket.h         | 4 ++--
 sysdeps/unix/sysv/linux/bits/socket.h   | 4 ++--
 sysdeps/unix/sysv/linux/net/if_packet.h | 2 +-
 sysdeps/unix/sysv/linux/netash/ash.h    | 2 +-
 sysdeps/unix/sysv/linux/neteconet/ec.h  | 2 +-
 sysdeps/unix/sysv/linux/netiucv/iucv.h  | 2 +-
 10 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/bits/socket.h b/bits/socket.h
index 13de124bac..772074d52a 100644
--- a/bits/socket.h
+++ b/bits/socket.h
@@ -149,7 +149,7 @@ enum __socket_type
 #include <bits/sockaddr.h>
 
 /* Structure describing a generic socket address.  */
-struct sockaddr
+struct __attribute_struct_may_alias__ sockaddr
   {
     __SOCKADDR_COMMON (sa_);	/* Common data: address family and length.  */
     char sa_data[14];		/* Address data.  */
@@ -166,7 +166,7 @@ struct sockaddr
 #define _SS_PADSIZE \
   (_SS_SIZE - __SOCKADDR_COMMON_SIZE - sizeof (__ss_aligntype))
 
-struct sockaddr_storage
+struct __attribute_struct_may_alias__ sockaddr_storage
   {
     __SOCKADDR_COMMON (ss_);	/* Address family, etc.  */
     char __ss_padding[_SS_PADSIZE];
diff --git a/inet/netinet/in.h b/inet/netinet/in.h
index fa57b61079..f684be5beb 100644
--- a/inet/netinet/in.h
+++ b/inet/netinet/in.h
@@ -244,7 +244,7 @@ extern const struct in6_addr in6addr_loopback;   /* ::1 */
 
 
 /* Structure describing an Internet socket address.  */
-struct sockaddr_in
+struct __attribute_struct_may_alias__ sockaddr_in
   {
     __SOCKADDR_COMMON (sin_);
     in_port_t sin_port;			/* Port number.  */
@@ -257,9 +257,11 @@ struct sockaddr_in
 			   - sizeof (struct in_addr)];
   };
 
-#if !__USE_KERNEL_IPV6_DEFS
+#if __USE_KERNEL_IPV6_DEFS
+struct __attribute_struct_may_alias__ sockaddr_in6;
+#else
 /* Ditto, for IPv6.  */
-struct sockaddr_in6
+struct __attribute_struct_may_alias__ sockaddr_in6
   {
     __SOCKADDR_COMMON (sin6_);
     in_port_t sin6_port;	/* Transport layer port # */
diff --git a/misc/sys/cdefs.h b/misc/sys/cdefs.h
index 2e8279a2c7..e21a2a7080 100644
--- a/misc/sys/cdefs.h
+++ b/misc/sys/cdefs.h
@@ -874,4 +874,13 @@ _Static_assert (0, "IEEE 128-bits long double requires redirection on this platf
 # define __attribute_returns_twice__ /* Ignore.  */
 #endif
 
+/* Mark struct types as aliasable.  Restricted to compilers that
+   support forward declarations of structs in the presence of the
+   attribute.  */
+#if __GNUC_PREREQ (8, 0) || defined __clang__
+# define __attribute_struct_may_alias__ __attribute__ ((__may_alias__))
+#else
+# define __attribute_struct_may_alias__
+#endif
+
 #endif	 /* sys/cdefs.h */
diff --git a/socket/sys/un.h b/socket/sys/un.h
index bf03b7d6ce..ff9cbd6efa 100644
--- a/socket/sys/un.h
+++ b/socket/sys/un.h
@@ -26,7 +26,7 @@
 __BEGIN_DECLS
 
 /* Structure describing the address of an AF_LOCAL (aka AF_UNIX) socket.  */
-struct sockaddr_un
+struct __attribute_struct_may_alias__ sockaddr_un
   {
     __SOCKADDR_COMMON (sun_);
     char sun_path[108];		/* Path name.  */
diff --git a/sysdeps/mach/hurd/bits/socket.h b/sysdeps/mach/hurd/bits/socket.h
index 3e72f9fa93..b5eeac3731 100644
--- a/sysdeps/mach/hurd/bits/socket.h
+++ b/sysdeps/mach/hurd/bits/socket.h
@@ -153,7 +153,7 @@ enum __socket_type
 #include <bits/sockaddr.h>
 
 /* Structure describing a generic socket address.  */
-struct sockaddr
+struct __attribute_struct_may_alias__ sockaddr
   {
     __SOCKADDR_COMMON (sa_);	/* Common data: address family and length.  */
     char sa_data[14];		/* Address data.  */
@@ -170,7 +170,7 @@ struct sockaddr
 #define _SS_PADSIZE \
   (_SS_SIZE - __SOCKADDR_COMMON_SIZE - sizeof (__ss_aligntype))
 
-struct sockaddr_storage
+struct __attribute_struct_may_alias__ sockaddr_storage
   {
     __SOCKADDR_COMMON (ss_);	/* Address family, etc.  */
     char __ss_padding[_SS_PADSIZE];
diff --git a/sysdeps/unix/sysv/linux/bits/socket.h b/sysdeps/unix/sysv/linux/bits/socket.h
index 86444f0018..5ab19a8c08 100644
--- a/sysdeps/unix/sysv/linux/bits/socket.h
+++ b/sysdeps/unix/sysv/linux/bits/socket.h
@@ -181,7 +181,7 @@ typedef __socklen_t socklen_t;
 #include <bits/sockaddr.h>
 
 /* Structure describing a generic socket address.  */
-struct sockaddr
+struct __attribute_struct_may_alias__ sockaddr
   {
     __SOCKADDR_COMMON (sa_);	/* Common data: address family and length.  */
     char sa_data[14];		/* Address data.  */
@@ -194,7 +194,7 @@ struct sockaddr
 #define _SS_PADSIZE \
   (_SS_SIZE - __SOCKADDR_COMMON_SIZE - sizeof (__ss_aligntype))
 
-struct sockaddr_storage
+struct __attribute_struct_may_alias__ sockaddr_storage
   {
     __SOCKADDR_COMMON (ss_);	/* Address family, etc.  */
     char __ss_padding[_SS_PADSIZE];
diff --git a/sysdeps/unix/sysv/linux/net/if_packet.h b/sysdeps/unix/sysv/linux/net/if_packet.h
index 9ffb69b508..c17e1c23c5 100644
--- a/sysdeps/unix/sysv/linux/net/if_packet.h
+++ b/sysdeps/unix/sysv/linux/net/if_packet.h
@@ -26,7 +26,7 @@
    From Linux 2.1 the AF_PACKET interface is preferred and you should
    consider using it in place of this one.  */
 
-struct sockaddr_pkt
+struct __attribute_struct_may_alias__ sockaddr_pkt
   {
     __SOCKADDR_COMMON (spkt_);
     unsigned char spkt_device[14];
diff --git a/sysdeps/unix/sysv/linux/netash/ash.h b/sysdeps/unix/sysv/linux/netash/ash.h
index 7d885d17cc..7a6ff50b17 100644
--- a/sysdeps/unix/sysv/linux/netash/ash.h
+++ b/sysdeps/unix/sysv/linux/netash/ash.h
@@ -22,7 +22,7 @@
 #include <features.h>
 #include <bits/sockaddr.h>
 
-struct sockaddr_ash
+struct __attribute_struct_may_alias__ sockaddr_ash
   {
     __SOCKADDR_COMMON (sash_);		/* Common data: address family etc.  */
     int sash_ifindex;			/* Interface to use.  */
diff --git a/sysdeps/unix/sysv/linux/neteconet/ec.h b/sysdeps/unix/sysv/linux/neteconet/ec.h
index b07a107961..f3132f06ff 100644
--- a/sysdeps/unix/sysv/linux/neteconet/ec.h
+++ b/sysdeps/unix/sysv/linux/neteconet/ec.h
@@ -28,7 +28,7 @@ struct ec_addr
     unsigned char net;			/* Network number.  */
   };
 
-struct sockaddr_ec
+struct __attribute_struct_may_alias__ sockaddr_ec
   {
     __SOCKADDR_COMMON (sec_);
     unsigned char port;			/* Port number.  */
diff --git a/sysdeps/unix/sysv/linux/netiucv/iucv.h b/sysdeps/unix/sysv/linux/netiucv/iucv.h
index f5fad81751..27151e8bbe 100644
--- a/sysdeps/unix/sysv/linux/netiucv/iucv.h
+++ b/sysdeps/unix/sysv/linux/netiucv/iucv.h
@@ -23,7 +23,7 @@
 
 __BEGIN_DECLS
 
-struct sockaddr_iucv
+struct __attribute_struct_may_alias__ sockaddr_iucv
   {
     __SOCKADDR_COMMON (siucv_);
     unsigned short	siucv_port;		/* Reserved */

base-commit: 4bbca1a44691a6e9adcee5c6798a707b626bc331


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] socket: Use may_alias on sockaddr structs (bug 19622)
  2024-05-07  9:29 [PATCH] socket: Use may_alias on sockaddr structs (bug 19622) Florian Weimer
@ 2024-05-07 11:38 ` Sam James
  2024-05-07 11:47   ` Sam James
  0 siblings, 1 reply; 4+ messages in thread
From: Sam James @ 2024-05-07 11:38 UTC (permalink / raw)
  To: Florian Weimer; +Cc: libc-alpha

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

Florian Weimer <fweimer@redhat.com> writes:

> This supports common coding paddings.  GCC before version 8
> rejects the may_alias attribute on a struct definition if it was
> not present in a previous forward declaration, so this attribute
> can only be conditionally applied.
>
> Tested on i686-linux-gnu, x86_64-linux-gnu.  Built with
> build-many-glibcs.py.
>
> Suggested-by: Marek Polacek <polacek@redhat.com>
> Suggested-by: Jakub Jelinek <jakub@redhat.com>

Reviewed-by: Sam James <sam@gentoo.org>

>
> ---
>  bits/socket.h                           | 4 ++--
>  inet/netinet/in.h                       | 8 +++++---
>  misc/sys/cdefs.h                        | 9 +++++++++
>  socket/sys/un.h                         | 2 +-
>  sysdeps/mach/hurd/bits/socket.h         | 4 ++--
>  sysdeps/unix/sysv/linux/bits/socket.h   | 4 ++--
>  sysdeps/unix/sysv/linux/net/if_packet.h | 2 +-
>  sysdeps/unix/sysv/linux/netash/ash.h    | 2 +-
>  sysdeps/unix/sysv/linux/neteconet/ec.h  | 2 +-
>  sysdeps/unix/sysv/linux/netiucv/iucv.h  | 2 +-
>  10 files changed, 25 insertions(+), 14 deletions(-)
>
> diff --git a/bits/socket.h b/bits/socket.h
> index 13de124bac..772074d52a 100644
> --- a/bits/socket.h
> +++ b/bits/socket.h
> @@ -149,7 +149,7 @@ enum __socket_type
>  #include <bits/sockaddr.h>
>  
>  /* Structure describing a generic socket address.  */
> -struct sockaddr
> +struct __attribute_struct_may_alias__ sockaddr
>    {
>      __SOCKADDR_COMMON (sa_);	/* Common data: address family and length.  */
>      char sa_data[14];		/* Address data.  */
> @@ -166,7 +166,7 @@ struct sockaddr
>  #define _SS_PADSIZE \
>    (_SS_SIZE - __SOCKADDR_COMMON_SIZE - sizeof (__ss_aligntype))
>  
> -struct sockaddr_storage
> +struct __attribute_struct_may_alias__ sockaddr_storage
>    {
>      __SOCKADDR_COMMON (ss_);	/* Address family, etc.  */
>      char __ss_padding[_SS_PADSIZE];
> diff --git a/inet/netinet/in.h b/inet/netinet/in.h
> index fa57b61079..f684be5beb 100644
> --- a/inet/netinet/in.h
> +++ b/inet/netinet/in.h
> @@ -244,7 +244,7 @@ extern const struct in6_addr in6addr_loopback;   /* ::1 */
>  
>  
>  /* Structure describing an Internet socket address.  */
> -struct sockaddr_in
> +struct __attribute_struct_may_alias__ sockaddr_in
>    {
>      __SOCKADDR_COMMON (sin_);
>      in_port_t sin_port;			/* Port number.  */
> @@ -257,9 +257,11 @@ struct sockaddr_in
>  			   - sizeof (struct in_addr)];
>    };
>  
> -#if !__USE_KERNEL_IPV6_DEFS
> +#if __USE_KERNEL_IPV6_DEFS
> +struct __attribute_struct_may_alias__ sockaddr_in6;
> +#else
>  /* Ditto, for IPv6.  */
> -struct sockaddr_in6
> +struct __attribute_struct_may_alias__ sockaddr_in6
>    {
>      __SOCKADDR_COMMON (sin6_);
>      in_port_t sin6_port;	/* Transport layer port # */
> diff --git a/misc/sys/cdefs.h b/misc/sys/cdefs.h
> index 2e8279a2c7..e21a2a7080 100644
> --- a/misc/sys/cdefs.h
> +++ b/misc/sys/cdefs.h
> @@ -874,4 +874,13 @@ _Static_assert (0, "IEEE 128-bits long double requires redirection on this platf
>  # define __attribute_returns_twice__ /* Ignore.  */
>  #endif
>  
> +/* Mark struct types as aliasable.  Restricted to compilers that
> +   support forward declarations of structs in the presence of the
> +   attribute.  */
> +#if __GNUC_PREREQ (8, 0) || defined __clang__
> +# define __attribute_struct_may_alias__ __attribute__ ((__may_alias__))
> +#else
> +# define __attribute_struct_may_alias__
> +#endif

Consider doing '/* Ignore. */' for consistency, even if I think it's a
bit silly.

> +
>  #endif	 /* sys/cdefs.h */
> diff --git a/socket/sys/un.h b/socket/sys/un.h
> index bf03b7d6ce..ff9cbd6efa 100644
> --- a/socket/sys/un.h
> +++ b/socket/sys/un.h
> @@ -26,7 +26,7 @@
>  __BEGIN_DECLS
>  
>  /* Structure describing the address of an AF_LOCAL (aka AF_UNIX) socket.  */
> -struct sockaddr_un
> +struct __attribute_struct_may_alias__ sockaddr_un
>    {
>      __SOCKADDR_COMMON (sun_);
>      char sun_path[108];		/* Path name.  */
> diff --git a/sysdeps/mach/hurd/bits/socket.h b/sysdeps/mach/hurd/bits/socket.h
> index 3e72f9fa93..b5eeac3731 100644
> --- a/sysdeps/mach/hurd/bits/socket.h
> +++ b/sysdeps/mach/hurd/bits/socket.h
> @@ -153,7 +153,7 @@ enum __socket_type
>  #include <bits/sockaddr.h>
>  
>  /* Structure describing a generic socket address.  */
> -struct sockaddr
> +struct __attribute_struct_may_alias__ sockaddr
>    {
>      __SOCKADDR_COMMON (sa_);	/* Common data: address family and length.  */
>      char sa_data[14];		/* Address data.  */
> @@ -170,7 +170,7 @@ struct sockaddr
>  #define _SS_PADSIZE \
>    (_SS_SIZE - __SOCKADDR_COMMON_SIZE - sizeof (__ss_aligntype))
>  
> -struct sockaddr_storage
> +struct __attribute_struct_may_alias__ sockaddr_storage
>    {
>      __SOCKADDR_COMMON (ss_);	/* Address family, etc.  */
>      char __ss_padding[_SS_PADSIZE];
> diff --git a/sysdeps/unix/sysv/linux/bits/socket.h b/sysdeps/unix/sysv/linux/bits/socket.h
> index 86444f0018..5ab19a8c08 100644
> --- a/sysdeps/unix/sysv/linux/bits/socket.h
> +++ b/sysdeps/unix/sysv/linux/bits/socket.h
> @@ -181,7 +181,7 @@ typedef __socklen_t socklen_t;
>  #include <bits/sockaddr.h>
>  
>  /* Structure describing a generic socket address.  */
> -struct sockaddr
> +struct __attribute_struct_may_alias__ sockaddr
>    {
>      __SOCKADDR_COMMON (sa_);	/* Common data: address family and length.  */
>      char sa_data[14];		/* Address data.  */
> @@ -194,7 +194,7 @@ struct sockaddr
>  #define _SS_PADSIZE \
>    (_SS_SIZE - __SOCKADDR_COMMON_SIZE - sizeof (__ss_aligntype))
>  
> -struct sockaddr_storage
> +struct __attribute_struct_may_alias__ sockaddr_storage
>    {
>      __SOCKADDR_COMMON (ss_);	/* Address family, etc.  */
>      char __ss_padding[_SS_PADSIZE];
> diff --git a/sysdeps/unix/sysv/linux/net/if_packet.h b/sysdeps/unix/sysv/linux/net/if_packet.h
> index 9ffb69b508..c17e1c23c5 100644
> --- a/sysdeps/unix/sysv/linux/net/if_packet.h
> +++ b/sysdeps/unix/sysv/linux/net/if_packet.h
> @@ -26,7 +26,7 @@
>     From Linux 2.1 the AF_PACKET interface is preferred and you should
>     consider using it in place of this one.  */
>  
> -struct sockaddr_pkt
> +struct __attribute_struct_may_alias__ sockaddr_pkt
>    {
>      __SOCKADDR_COMMON (spkt_);
>      unsigned char spkt_device[14];
> diff --git a/sysdeps/unix/sysv/linux/netash/ash.h b/sysdeps/unix/sysv/linux/netash/ash.h
> index 7d885d17cc..7a6ff50b17 100644
> --- a/sysdeps/unix/sysv/linux/netash/ash.h
> +++ b/sysdeps/unix/sysv/linux/netash/ash.h
> @@ -22,7 +22,7 @@
>  #include <features.h>
>  #include <bits/sockaddr.h>
>  
> -struct sockaddr_ash
> +struct __attribute_struct_may_alias__ sockaddr_ash
>    {
>      __SOCKADDR_COMMON (sash_);		/* Common data: address family etc.  */
>      int sash_ifindex;			/* Interface to use.  */
> diff --git a/sysdeps/unix/sysv/linux/neteconet/ec.h b/sysdeps/unix/sysv/linux/neteconet/ec.h
> index b07a107961..f3132f06ff 100644
> --- a/sysdeps/unix/sysv/linux/neteconet/ec.h
> +++ b/sysdeps/unix/sysv/linux/neteconet/ec.h
> @@ -28,7 +28,7 @@ struct ec_addr
>      unsigned char net;			/* Network number.  */
>    };
>  
> -struct sockaddr_ec
> +struct __attribute_struct_may_alias__ sockaddr_ec
>    {
>      __SOCKADDR_COMMON (sec_);
>      unsigned char port;			/* Port number.  */
> diff --git a/sysdeps/unix/sysv/linux/netiucv/iucv.h b/sysdeps/unix/sysv/linux/netiucv/iucv.h
> index f5fad81751..27151e8bbe 100644
> --- a/sysdeps/unix/sysv/linux/netiucv/iucv.h
> +++ b/sysdeps/unix/sysv/linux/netiucv/iucv.h
> @@ -23,7 +23,7 @@
>  
>  __BEGIN_DECLS
>  
> -struct sockaddr_iucv
> +struct __attribute_struct_may_alias__ sockaddr_iucv
>    {
>      __SOCKADDR_COMMON (siucv_);
>      unsigned short	siucv_port;		/* Reserved */
>
> base-commit: 4bbca1a44691a6e9adcee5c6798a707b626bc331

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] socket: Use may_alias on sockaddr structs (bug 19622)
  2024-05-07 11:38 ` Sam James
@ 2024-05-07 11:47   ` Sam James
  2024-05-07 12:01     ` Florian Weimer
  0 siblings, 1 reply; 4+ messages in thread
From: Sam James @ 2024-05-07 11:47 UTC (permalink / raw)
  To: Florian Weimer; +Cc: libc-alpha

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

Sam James <sam@gentoo.org> writes:

> Florian Weimer <fweimer@redhat.com> writes:
>
>> This supports common coding paddings.  GCC before version 8
>> rejects the may_alias attribute on a struct definition if it was
>> not present in a previous forward declaration, so this attribute
>> can only be conditionally applied.
>>
>> Tested on i686-linux-gnu, x86_64-linux-gnu.  Built with
>> build-many-glibcs.py.
>>
>> Suggested-by: Marek Polacek <polacek@redhat.com>
>> Suggested-by: Jakub Jelinek <jakub@redhat.com>
>
> Reviewed-by: Sam James <sam@gentoo.org>

Also, on reflection, consider citing the POSIX change I mentioned on
Bugzilla in the commit message too.

I think it's useful in making clear this isn't (just) a QoI change?

(https://austingroupbugs.net/view.php?id=1641)

>
>>
>> ---
>>  bits/socket.h                           | 4 ++--
>>  inet/netinet/in.h                       | 8 +++++---
>>  misc/sys/cdefs.h                        | 9 +++++++++
>>  socket/sys/un.h                         | 2 +-
>>  sysdeps/mach/hurd/bits/socket.h         | 4 ++--
>>  sysdeps/unix/sysv/linux/bits/socket.h   | 4 ++--
>>  sysdeps/unix/sysv/linux/net/if_packet.h | 2 +-
>>  sysdeps/unix/sysv/linux/netash/ash.h    | 2 +-
>>  sysdeps/unix/sysv/linux/neteconet/ec.h  | 2 +-
>>  sysdeps/unix/sysv/linux/netiucv/iucv.h  | 2 +-
>>  10 files changed, 25 insertions(+), 14 deletions(-)
>>
>> diff --git a/bits/socket.h b/bits/socket.h
>> index 13de124bac..772074d52a 100644
>> --- a/bits/socket.h
>> +++ b/bits/socket.h
>> @@ -149,7 +149,7 @@ enum __socket_type
>>  #include <bits/sockaddr.h>
>>  
>>  /* Structure describing a generic socket address.  */
>> -struct sockaddr
>> +struct __attribute_struct_may_alias__ sockaddr
>>    {
>>      __SOCKADDR_COMMON (sa_);	/* Common data: address family and length.  */
>>      char sa_data[14];		/* Address data.  */
>> @@ -166,7 +166,7 @@ struct sockaddr
>>  #define _SS_PADSIZE \
>>    (_SS_SIZE - __SOCKADDR_COMMON_SIZE - sizeof (__ss_aligntype))
>>  
>> -struct sockaddr_storage
>> +struct __attribute_struct_may_alias__ sockaddr_storage
>>    {
>>      __SOCKADDR_COMMON (ss_);	/* Address family, etc.  */
>>      char __ss_padding[_SS_PADSIZE];
>> diff --git a/inet/netinet/in.h b/inet/netinet/in.h
>> index fa57b61079..f684be5beb 100644
>> --- a/inet/netinet/in.h
>> +++ b/inet/netinet/in.h
>> @@ -244,7 +244,7 @@ extern const struct in6_addr in6addr_loopback;   /* ::1 */
>>  
>>  
>>  /* Structure describing an Internet socket address.  */
>> -struct sockaddr_in
>> +struct __attribute_struct_may_alias__ sockaddr_in
>>    {
>>      __SOCKADDR_COMMON (sin_);
>>      in_port_t sin_port;			/* Port number.  */
>> @@ -257,9 +257,11 @@ struct sockaddr_in
>>  			   - sizeof (struct in_addr)];
>>    };
>>  
>> -#if !__USE_KERNEL_IPV6_DEFS
>> +#if __USE_KERNEL_IPV6_DEFS
>> +struct __attribute_struct_may_alias__ sockaddr_in6;
>> +#else
>>  /* Ditto, for IPv6.  */
>> -struct sockaddr_in6
>> +struct __attribute_struct_may_alias__ sockaddr_in6
>>    {
>>      __SOCKADDR_COMMON (sin6_);
>>      in_port_t sin6_port;	/* Transport layer port # */
>> diff --git a/misc/sys/cdefs.h b/misc/sys/cdefs.h
>> index 2e8279a2c7..e21a2a7080 100644
>> --- a/misc/sys/cdefs.h
>> +++ b/misc/sys/cdefs.h
>> @@ -874,4 +874,13 @@ _Static_assert (0, "IEEE 128-bits long double requires redirection on this platf
>>  # define __attribute_returns_twice__ /* Ignore.  */
>>  #endif
>>  
>> +/* Mark struct types as aliasable.  Restricted to compilers that
>> +   support forward declarations of structs in the presence of the
>> +   attribute.  */
>> +#if __GNUC_PREREQ (8, 0) || defined __clang__
>> +# define __attribute_struct_may_alias__ __attribute__ ((__may_alias__))
>> +#else
>> +# define __attribute_struct_may_alias__
>> +#endif
>
> Consider doing '/* Ignore. */' for consistency, even if I think it's a
> bit silly.
>
>> +
>>  #endif	 /* sys/cdefs.h */
>> diff --git a/socket/sys/un.h b/socket/sys/un.h
>> index bf03b7d6ce..ff9cbd6efa 100644
>> --- a/socket/sys/un.h
>> +++ b/socket/sys/un.h
>> @@ -26,7 +26,7 @@
>>  __BEGIN_DECLS
>>  
>>  /* Structure describing the address of an AF_LOCAL (aka AF_UNIX) socket.  */
>> -struct sockaddr_un
>> +struct __attribute_struct_may_alias__ sockaddr_un
>>    {
>>      __SOCKADDR_COMMON (sun_);
>>      char sun_path[108];		/* Path name.  */
>> diff --git a/sysdeps/mach/hurd/bits/socket.h b/sysdeps/mach/hurd/bits/socket.h
>> index 3e72f9fa93..b5eeac3731 100644
>> --- a/sysdeps/mach/hurd/bits/socket.h
>> +++ b/sysdeps/mach/hurd/bits/socket.h
>> @@ -153,7 +153,7 @@ enum __socket_type
>>  #include <bits/sockaddr.h>
>>  
>>  /* Structure describing a generic socket address.  */
>> -struct sockaddr
>> +struct __attribute_struct_may_alias__ sockaddr
>>    {
>>      __SOCKADDR_COMMON (sa_);	/* Common data: address family and length.  */
>>      char sa_data[14];		/* Address data.  */
>> @@ -170,7 +170,7 @@ struct sockaddr
>>  #define _SS_PADSIZE \
>>    (_SS_SIZE - __SOCKADDR_COMMON_SIZE - sizeof (__ss_aligntype))
>>  
>> -struct sockaddr_storage
>> +struct __attribute_struct_may_alias__ sockaddr_storage
>>    {
>>      __SOCKADDR_COMMON (ss_);	/* Address family, etc.  */
>>      char __ss_padding[_SS_PADSIZE];
>> diff --git a/sysdeps/unix/sysv/linux/bits/socket.h b/sysdeps/unix/sysv/linux/bits/socket.h
>> index 86444f0018..5ab19a8c08 100644
>> --- a/sysdeps/unix/sysv/linux/bits/socket.h
>> +++ b/sysdeps/unix/sysv/linux/bits/socket.h
>> @@ -181,7 +181,7 @@ typedef __socklen_t socklen_t;
>>  #include <bits/sockaddr.h>
>>  
>>  /* Structure describing a generic socket address.  */
>> -struct sockaddr
>> +struct __attribute_struct_may_alias__ sockaddr
>>    {
>>      __SOCKADDR_COMMON (sa_);	/* Common data: address family and length.  */
>>      char sa_data[14];		/* Address data.  */
>> @@ -194,7 +194,7 @@ struct sockaddr
>>  #define _SS_PADSIZE \
>>    (_SS_SIZE - __SOCKADDR_COMMON_SIZE - sizeof (__ss_aligntype))
>>  
>> -struct sockaddr_storage
>> +struct __attribute_struct_may_alias__ sockaddr_storage
>>    {
>>      __SOCKADDR_COMMON (ss_);	/* Address family, etc.  */
>>      char __ss_padding[_SS_PADSIZE];
>> diff --git a/sysdeps/unix/sysv/linux/net/if_packet.h b/sysdeps/unix/sysv/linux/net/if_packet.h
>> index 9ffb69b508..c17e1c23c5 100644
>> --- a/sysdeps/unix/sysv/linux/net/if_packet.h
>> +++ b/sysdeps/unix/sysv/linux/net/if_packet.h
>> @@ -26,7 +26,7 @@
>>     From Linux 2.1 the AF_PACKET interface is preferred and you should
>>     consider using it in place of this one.  */
>>  
>> -struct sockaddr_pkt
>> +struct __attribute_struct_may_alias__ sockaddr_pkt
>>    {
>>      __SOCKADDR_COMMON (spkt_);
>>      unsigned char spkt_device[14];
>> diff --git a/sysdeps/unix/sysv/linux/netash/ash.h b/sysdeps/unix/sysv/linux/netash/ash.h
>> index 7d885d17cc..7a6ff50b17 100644
>> --- a/sysdeps/unix/sysv/linux/netash/ash.h
>> +++ b/sysdeps/unix/sysv/linux/netash/ash.h
>> @@ -22,7 +22,7 @@
>>  #include <features.h>
>>  #include <bits/sockaddr.h>
>>  
>> -struct sockaddr_ash
>> +struct __attribute_struct_may_alias__ sockaddr_ash
>>    {
>>      __SOCKADDR_COMMON (sash_);		/* Common data: address family etc.  */
>>      int sash_ifindex;			/* Interface to use.  */
>> diff --git a/sysdeps/unix/sysv/linux/neteconet/ec.h b/sysdeps/unix/sysv/linux/neteconet/ec.h
>> index b07a107961..f3132f06ff 100644
>> --- a/sysdeps/unix/sysv/linux/neteconet/ec.h
>> +++ b/sysdeps/unix/sysv/linux/neteconet/ec.h
>> @@ -28,7 +28,7 @@ struct ec_addr
>>      unsigned char net;			/* Network number.  */
>>    };
>>  
>> -struct sockaddr_ec
>> +struct __attribute_struct_may_alias__ sockaddr_ec
>>    {
>>      __SOCKADDR_COMMON (sec_);
>>      unsigned char port;			/* Port number.  */
>> diff --git a/sysdeps/unix/sysv/linux/netiucv/iucv.h b/sysdeps/unix/sysv/linux/netiucv/iucv.h
>> index f5fad81751..27151e8bbe 100644
>> --- a/sysdeps/unix/sysv/linux/netiucv/iucv.h
>> +++ b/sysdeps/unix/sysv/linux/netiucv/iucv.h
>> @@ -23,7 +23,7 @@
>>  
>>  __BEGIN_DECLS
>>  
>> -struct sockaddr_iucv
>> +struct __attribute_struct_may_alias__ sockaddr_iucv
>>    {
>>      __SOCKADDR_COMMON (siucv_);
>>      unsigned short	siucv_port;		/* Reserved */
>>
>> base-commit: 4bbca1a44691a6e9adcee5c6798a707b626bc331

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] socket: Use may_alias on sockaddr structs (bug 19622)
  2024-05-07 11:47   ` Sam James
@ 2024-05-07 12:01     ` Florian Weimer
  0 siblings, 0 replies; 4+ messages in thread
From: Florian Weimer @ 2024-05-07 12:01 UTC (permalink / raw)
  To: Sam James; +Cc: libc-alpha

* Sam James:

> Sam James <sam@gentoo.org> writes:
>
>> Florian Weimer <fweimer@redhat.com> writes:
>>
>>> This supports common coding paddings.  GCC before version 8
>>> rejects the may_alias attribute on a struct definition if it was
>>> not present in a previous forward declaration, so this attribute
>>> can only be conditionally applied.
>>>
>>> Tested on i686-linux-gnu, x86_64-linux-gnu.  Built with
>>> build-many-glibcs.py.
>>>
>>> Suggested-by: Marek Polacek <polacek@redhat.com>
>>> Suggested-by: Jakub Jelinek <jakub@redhat.com>
>>
>> Reviewed-by: Sam James <sam@gentoo.org>

Thanks.

> Also, on reflection, consider citing the POSIX change I mentioned on
> Bugzilla in the commit message too.
>
> I think it's useful in making clear this isn't (just) a QoI change?
>
> (https://austingroupbugs.net/view.php?id=1641)

I'm not sure we implement this today, particularly this part:

| Similarly, when a pointer to a sockaddr_storage or sockaddr structure
| is converted to a pointer to a protocol-specific address structure,
| the compiler shall treat an access (using this converted pointer) to
| the stored value of any member of the protocol-specific structure as
| permissible.

This seems to suggest that accessing uninitialized data or fields not
present in the original object (after casting to larger struct) has some
defined behavior, without actually assigning any meaning to it.

POSIX could have said that the implementation should behave as if there
is a definition

union
{
  struct sockaddr sa;
  struct sockaddr_in sin;
  struct sockaddr_in6 sin6;
  …
  struct sockaddr_storage ss;
} <implementation-specified>;

in every program.  Then the desired behavior would follow from C's
impossible-to-implement rule about union members and aliasing.

I can perhaps say that it partially addresses Austin Groups issue 1641.

Thanks,
Florian


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2024-05-07 12:01 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-07  9:29 [PATCH] socket: Use may_alias on sockaddr structs (bug 19622) Florian Weimer
2024-05-07 11:38 ` Sam James
2024-05-07 11:47   ` Sam James
2024-05-07 12:01     ` Florian Weimer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).