Hi Bastien, On 1/21/23 14:30, Bastien Roucariès wrote: [...]
Ahh, indeed it seems to be UB. It's in the same 6.5.2.3/6: there's a requirement that the information about the union is kept in the function in which it's accessed. The standard presents an example, which is a bit ambiguous: The following is not a valid fragment (because the union type is not visible within function f):
[...]
I don't know what's the intention if the union type was global but the variable `u` was still not seen by f(). But it seems GCC's interpretation is UB, according to the test we just saw. The solution that I can see for that is making sockaddr also be a union. That way, the union is kept across all calls (since they all use sockaddr). struct sockaddr { union { struct { sa_family_t sa_family; char sa_data[14]; // why 14? } struct sockaddr_in sin; struct sockaddr_in6 sin6; struct sockaddr_un sun; }; };No the solution is to avoid sockaddr and mark as deprecated.
Declaring `sockaddr` as deprecated means deprecating also: accept(2) accept4(2) bind(2) connect(2) getpeername(2) getsockname(2) recvfrom(2) sendto(2) getnameinfo(3) which use the type in their prototype.Also, other types such as `addrinfo`, which contain `sockaddr` would also need to be deprecated, which would itself require deprecating:
getaddrinfo(3) freeaddrinfo(3)Since addrinfo is itself contained in other structures such as `gaicb`, we would also need to deprecate those, which would in turn require deprecating more APIs:
getaddrinfo_a(3) gai_error(3) gai_cancel(3)And maybe I left some. This feels like nuking the entire networking API, which I don't see happening soon.
Otherwise, we need to come up with a solution that keeps these APIs compatible with whatever new type we suggest using. Changing them to use `void*` instead of `sockaddr*` would be possible, but would decrease type safety considerably, so there should be a good reason for that.
Suggesting to use always `sockaddr_storage` but using `sockaddr` in the function parameters keeps the current not-so-nice casting issues, which are not Undefined Behavior per se, but not ideal either (in fact, I don't think `void*` is much worse than code full of casts). And it would also be error-prone, since users could get the idea that `sockaddr` can be used safely, since it's what gets passed as the parameter.
The problem it should be part of union without raising a warning each time we use a safe type...
I don't understand this; please detail.
The other solution is to render public and ABI stable the type here https://github.com/bminor/glibc/blob/ae612c45efb5e34713859a5facf92368307efb6e/socket/sys/socket.h#L78 under for instance sockaddr_ptr and sockaddr_const_ptr
[[gnu::transparent_union]] (used in that link) seems like a "the design of this interface is bad, sorry, this workaround will just make it work". I guess it just works, and probably it's the reason that so much undefined behavior hasn't exploded so far. However, if we can solve this using core language features, I'd go that way.
Moreover this are some patch arch by arch https://sourceware.org/legacy-ml/libc-alpha/2016-02/msg00340.html that should be made default Bastienstruct sockaddr_storage { union { sa_family_t ss_family; struct sockaddr sa; }; };
Hmm, this isn't still perfect. Since the APIs get the sockaddr, this union information would be lost. `sockaddr` needs to be the type that is declared. `sockaddr_storage` should just die; there's no way to make it compatible with APIs getting a `sockaddr`. The attribute `transparent_union` is the only way to use is safely.
Cheers, Alex
With this, sockaddr_storage becomes useless, but still usable. New code, could just use sockaddr and use the internal union members as necessary. Union info is kept across all function boundaries.
-- <http://www.alejandro-colomar.es/>
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature
