How to cast sockaddr_storage and avoid breaking strict-aliasing rules
Asked Answered
B

4

28

I'm using Beej's Guide to Networking and came across an aliasing issue. He proposes a function to return either the IPv4 or IPv6 address of a particular struct:

1  void *get_in_addr( struct sockaddr *sa )
2  {
3      if (sa->sa_family == AF_INET)
4        return &(((struct sockaddr_in*)sa)->sin_addr);
5      else
6        return &(((struct sockaddr_in6*)sa)->sin6_addr);
7  }

This causes GCC to spit out a strict-aliasing error for sa on line 3. As I understand it, it is because I call this function like so:

struct sockaddr_storage their_addr;
...
inet_ntop(their_addr.ss_family,
          get_in_addr((struct sockaddr *)&their_addr),
          connection_name,
          sizeof connection_name);

I'm guessing the aliasing has to do with the fact that the their_addr variable is of type sockaddr_storage and another pointer of a differing type points to the same memory.

Is the best way to get around this sticking sockaddr_storage, sockaddr_in, and sockaddr_in6 into a union? It seems like this should be well worn territory in networking, I just can't find any good examples with best practices.

Also, if anyone can explain exactly where the aliasing issue takes place, I'd much appreciate it.

Baggett answered 15/9, 2009 at 21:10 Comment(3)
Can you just change get_in_addr() to take a struct sockaddr_storage * and forget the cast in the call?Messner
Thank you, mark4o! I don't know why I was making this harder than it needed to be. Your suggestion works wonders.Baggett
@Messner Would that not merely move the problem to line 4 and 6 of get_in_addr?Nutshell
S
21

I tend to do this to get GCC do the right thing with type-punning, which is explicitly allowed with unions:


/*! Multi-family socket end-point address. */
typedef union address
{
    struct sockaddr sa;
    struct sockaddr_in sa_in;
    struct sockaddr_in6 sa_in6;
    struct sockaddr_storage sa_stor;
}
address_t;
Solifidian answered 16/9, 2009 at 13:28 Comment(16)
This makes sense, but for some reason I got the impression that type-punning was a no-no. I suppose in some circumstances there isn't a great way around it.Baggett
union was invented for exactly this - explicit type punning - so compilers have to recognize and handle it.Solifidian
Union may have been invented for this, and I agree that compilers should handle it. But the standard does not specify it. Support for it is an additional guarantee provided by gcc, so it could fail on another compiler and the compiler's developers would argue that they are in the right. And in the future, gcc developers could do the same thing. In the C world, the trend is to break existing programming practices to gain 0.5% in speed benchmarks, and strict aliasing itself is only one instance of this trend.Quick
The C99 standard specifically allows unions for this purpose. See open-std.org/jtc1/sc22/wg14/www/docs/n1124.pdf §6.5, paragraph 7.Claret
I seem to recall that POSIX reserves the names of types ending _t. Sorry for being pedantic.Ridenour
@NikolaiNFetissov "union was invented for exactly this" Source? It is a misuse of union!Misbehave
@AdamGoode "The C99 standard specifically allows unions for this purpose." Where? "See open-std.org/jtc1/sc22/wg14/www/docs/n1124.pdf §6.5, paragraph 7." "An object shall have its stored value": there is no permission to misuse unions.Misbehave
The C99 standard has been amended in three TCs and these have updated the union text.Stores
That use of a union is implementation-defined in C90, see 6.3.2.3, and see DR 283 for a footnote present in C11 to clarify the situation in C99 and C11.Ellata
@JonathanWakely The footnote is only valid if there is corresponding normative text, and I doubt that.Misbehave
@curiousguy, the footnote clarifies what the normative wording says. Are you suggesting the footnote is just incorrect and contradicts the normative wording? Did you read the DR? Did you read the reference in the footnote to 6.2.6, which is the corresponding normative text?Ellata
@JonathanWakely I have no idea which part of 6.2.6 validates the footnote. I am not saying that the footnote contradicts anything, just that it does not correspond to something the normative text says.Misbehave
@curiousguy, so then you're saying the C committee added a footnote to clarify something which is not true. I doubt that.Ellata
Also, 6.5.2.3/3 "The value is that of the named member", without the caveat from C90 saying it's implementation-defined if accessing a different union member from the one last used to store a value. i.e. it was implementation-defined in C90, but is not in C99 and C11. The normative wording saying you maybe couldn't do it was intentionally removed, so now you definitely can do it.Ellata
@JonathanWakely unfortunately C99 and C11 still contains the words (in 6.7.2.1): "the value of at most one of the members can be stored in a union object at any time". This is problematic for type punning when considered in combination with the wording of 6.5.2.3 which you quote above. What is "the value of the named member" if it's not the active memeber and "the value of at most one member can be stored"? I agree that it's meant to support type-punning, but the wording needs significant rework before that is literally the case.Zumstein
@JonathanWakely Does this mean that for example you store the socket address in sa_in6 and then pass to a POSIX function for example getsockname the member sa of your union ?Theressa
M
1

I tend to do this to get GCC do the right thing with type-punning, which is explicitly allowed with unions

I am pretty sure this (mis)use of union will not work (or only by accident) with GCC:

short type_pun2 (int i, int *pi, short *ps) {
    *pi = i;
    return *ps;
}

union U {
    int i;
    short s;
};

short type_pun (int i) {
    U u;
    return type_pun2 (i, &u.i, &u.s);
}

The correct way to do that is with memcpy, not union.

Misbehave answered 3/10, 2011 at 15:40 Comment(17)
But in your code you pass two pointers which do alias. Nikolai's suggestion of an alias for sockaddr types code doesn't imply that, and is a valid use of type-punning, which works with GCC by design.Ellata
@JonathanWakely "But in your code you pass two pointers which do alias." Yes, and? "and is a valid use of type-punning" says who?Misbehave
@JonathanWakely "Do you have difficulty reading?" 1) "The relevant bytes of the representation of the object are treated as an object of the type used for the access." I don't understand that: "accessed using a member" 2) "provided the memory is accessed through the union type" where is "through the union type" defined? These statements are closer to poetry than language specifications.Misbehave
Says GCC's own manual, which defines what GCC considers valid: gcc.gnu.org/onlinedocs/gcc-4.4.1/gcc/… and gcc.gnu.org/onlinedocs/gcc-4.4.1/gcc/… "The relevant bytes of the representation of the object are treated as an object of the type used for the access."Ellata
1) That is the wording of the C standard. 2) "Through the union type" mean int i = u.i as opposed to int* p = &u.i; int i = *p;, the latter accesses it through int* not the union type, and is what you do in your answer.Ellata
So, again, where is "accessed using a member" defined? And where is "through the union type" defined? What is an access to a member?Misbehave
I only have C99 here and that wording is from C90, but it has 3.1/1 access to read or modify the value of an object, 6.2.5/1 The meaning of a value stored in an object or returned by a function is determined by the type of the expression used to access it. If you access the active member of a union (i.e. read its value) through a different member (i.e. not the active member) then the value depends on the type of the member used in the expression, and GCC says the value you get is the bytes that represent the object treated as the type used in the access expression.Ellata
let us continue this discussion in chatMisbehave
It should be obvious to ANYONE that unions would not be useful if their behavior was undefined. C99 has language specifying that you can safely access any value in the union. See #11640447Chuckchuckfull
@Chuckchuckfull It should be obvious to ANYONE that unions are only useful where their behavior is well defined. The idea that you cannot do type punning with unions certainly does not imply unions are useless, just useless for type punning. What makes you think unions where invented to do type punning? "C99 has language specifying that you can safely access any value in the union" Where? By "any value" do you mean "any member"? What do you mean by "safely"?Misbehave
@Chuckchuckfull Your "It should be obvious to ANYONE" seems to imply that I am missing something extremely obvious here. Not only it is slightly insulting, but given the fact that I have been significantly involved in C++ standard work, and also (less) involved in C standard work, it would be incredible for me to miss something "obvious to ANYONE". In general, issue involving type punning are NOT "obvious" in either C or C++, even for the C and C++ committee members.Misbehave
FWIW I believe that the standard is supposed to allow type punning via a union (that is indeed what the footnote added after DR 283 suggests), but I agree that the wording of the normative text fails to reflect this; eg 6.7.2.1p14 says that “the value of at most one of the members can be stored in a union object at any time”, together with 6.5.2.3 "The value is that of the named member" implies (disregarding the footnote) that there can accessing a non-active member returns no value. I believe this issue is still not fixed in C11.Zumstein
* that <strike>there can</strike> accessing a non-active member returns no valueZumstein
@Zumstein I fear that the C committee has lost all its competent members, and no one is able to solve this delicate issue or write decent specification in general. This issue is bad bad bad. It's fundamental and essential in practice for many real world uses of C, and yet nobody can answer simple questions. Allowing type punning via unions opens a can of worms up to point where type based aliasing rules don't make sense. Instead of math all we get is feelings and fuzzy logic. The C committee managed to make things worse by even breaking "memcpy is safe for type punning". C++ is horrible too.Misbehave
@curiousguy: What's needed is for the Committee to recognize that the existence of distinct dialects is a good thing, especially if there are standard directives that mean "process according to this dialect if you can, or refuse compilation altogether if you can't". Add about three of those to the language to specify dialects with different aliasing semantics, along with a directive which means "all writes executed thus far must behave as though they were committed to physical storage here, and no reads after this can use data cached before this". On many compilers, an "asm" directive...Streusel
...could achieve the required semantics, but the exact syntax would vary from one compiler to the next. Have a dialect where all writes commit to memory, one which applies type-based aliasing only to direct accesses of named objects, and one which is even stricter than C11 and disallows any kind of aliasing (even with char*) except when using directives to notify the compiler about it. No need to worry about what the present rules "mean", since existing code could be handled using the first two dialects, and code written for the third could be optimized better than anything under...Streusel
...the existing rules, despite the fact that aliasing-barrier directives would also make the language much more powerful than the present rules.Streusel
O
0

I recently had a similar alias warning on HPUX system when trying to write code to get the MAC address of the machine

The &(((struct sockaddr_in *)addr)->sin_addr) complains about strict-aliasing rules

This is the code in some context

 char ip[INET6_ADDRSTRLEN] = {0};
 strucut sockaddr *addr

 ...
 get addr from ioctl(socket,SOCGIFCONF...) call
 ...
 inet_ntop(AF_INET, &(((struct sockaddr_in *)addr)->sin_addr),ip,sizeof ip);

I overcame the aliasing warning by doing the following

struct sockaddr_in sin;
memcpy(&sin,addr,sizeof(struct sockaddr));
inet_ntop(AF_INET, &sin.sin_addr,ip,sizeof ip);

And whilst this is potentially dangerous I added the following lines before it

 static_assert(sizeof(sockaddr)==sizeof(sockaddr_in));

I'm not sure if that is something would be considered bad practice, but it worked and was cross platform to other *Nix flavors and compilers

Ostia answered 23/8, 2017 at 7:49 Comment(0)
L
-1

The issue has nothing to do with the call to the function. Rather, it's with ((struct sockaddr_in*)sa)->sin_addr. The problem is that sa is a pointer of one type, but you're casting it to a pointer of a different type and then dereferencing it. This breaks a rule called "strict aliasing", which says that variables of different types can never alias. In your case, aliasing to a different type is exactly what you want to do.

The simple solution is to turn off this optimization, which allows aliasing in this manner. On GCC, the flag is -fno-strict-aliasing.

The better solution is to use a union, as mentioned by Nikolai.

void *get_in_addr(struct sockaddr *sa)
{
    union {
        struct sockaddr     *sa;
        struct sockaddr_in  *sa_in;
        struct sockaddr_in6 *sa_in6;
    } u;
    u.sa = sa;
    if (sa->sa_family == AF_INET)
        return &(u.sa_in->sin_addr);
    else
        return &(u.sa_in6->sin6_addr);
}

That said, I can't actually get GCC to give me a warning when using your original code, so I'm not sure if this buys you anything.

Lessard answered 30/3, 2010 at 1:2 Comment(7)
Using a union of pointers to convert between pointers to different types of objects may hide the strict aliasing violation from the compiler, but it is still a violation. Nikolai correctly suggested using a union of structures, rather than a union of pointers, because that is the only safe way to convert between structure types permitted by the standard.Eserine
You use a union for type punning, so instead of one illegal aliasing, you now have two: the original sockaddr_storage vs. sockaddr aliasing, and also sockaddr* vs. sockaddr_in* vs. sockaddr_in6* aliasing. And by hiding this in a function, you are making it more likely that the compiler will not see what's going on, and that it will generate bad code and no warning.Misbehave
All of you people who downvoted this are wrong. Using -fno-strict-aliasing is GOOD advice for 99% of projects out there. The remainder can use "restrict" on hot paths. Finally, people who say the behavior of unions is unspecified are wrong. This is language from C99: "If the member used to access the contents of a union object is not the same as the member last used to store a value in the object, the appropriate part of the object representation of the value is reinterpreted as an object representation in the new type as described in 6.2.6 (a process sometimes called "type punning")."Chuckchuckfull
@Chuckchuckfull "Using -fno-strict-aliasing is GOOD advice for 99% of projects out there." given the fact that the GCC team is extremely confused on the so called "strict aliasing" rule, to the point they said that a my_malloc function cannot be implemented in conforming C code, it may be a good idea to disable this craziness!Misbehave
@Chuckchuckfull "This is language from C99: "If the member used to access the contents of a union object is not the same as the member last used to store a value in the object" You are missing the whole point: this applies to unions only, and there is no union here. There is a sockaddr. Also, you are missing the other point that there is no indication that different objects pointer types have compatible representation.Misbehave
@curiousguy: Given that gcc effectively ignores language in the Standard whose purpose is to make the Common Initial Sequence rule meaningful, and given that enabling aliasing analysis causes version 6.2 to generate buggy code even in cases where data is always read as the type with which it was last written, I see no reason why anyone would want to target the -fstrict-aliasing dialect of gcc.Streusel
OK I am going to be explicit: I think GCC is a mess at the design level. The semantic isn't defined anywhere, esp. the issue of -fstrict-aliasing.Misbehave

© 2022 - 2024 — McMap. All rights reserved.