| Summary: | [6/7 Regression] Aliasing "struct sockaddr_storage" produces invalid code due to SRA | ||
|---|---|---|---|
| Product: | gcc | Reporter: | Vincent Bernat <bernat> |
| Component: | tree-optimization | Assignee: | Martin Jambor <jamborm> |
| Status: | RESOLVED INVALID | ||
| Severity: | normal | CC: | jakub, jamborm, tavianator |
| Priority: | P3 | ||
| Version: | 6.1.1 | ||
| Target Milestone: | 6.2 | ||
| Host: | Target: | ||
| Build: | Known to work: | 5.3.0 | |
| Known to fail: | 6.1.0 | Last reconfirmed: | 2016-05-17 00:00:00 |
Confirmed. ESRA replaces the aggregate copy
ss = ss3;
which enables FRE1 to do "interesting" things:
memset (&ss3, 0, 128);
ss3.ss_family = 2;
_6 = inet_addr ("127.0.0.1");
MEM[(struct sockaddr_in *)&ss3].sin_addr.s_addr = _6;
ss = ss3;
l_11 = calloc (1, 128);
MEM[(struct dbg_listener *)l_11] = 2;
MEM[(struct dbg_listener *)l_11 + 8B] = 0;
MEM[(struct dbg_listener *)l_11 + 16B] = 0;
MEM[(struct dbg_listener *)l_11 + 17B] = 0;
...
so it seems to reach the memset, ignoring the sin_addr.s_addr store.
Mine.
Actually the issue is that SRA decomposes the aggregate assignment in a way
leaving the "interesting" piece out. sin_addr.s_addr is at offset 4 bytes
and 4 bytes in size while the scalarization
ss$ss_family_13 = MEM[(struct sockaddr_storage *)&ss3];
ss$__ss_align_4 = MEM[(struct sockaddr_storage *)&ss3 + 8B];
fails to cover that area. It scalarizes
struct sockaddr_storage
{
sa_family_t ss_family;
unsigned long int __ss_align;
char __ss_padding[(128 - (2 * sizeof (unsigned long int)))];
};
but the actual data is of type
struct sockaddr_in
{
sa_family_t sin_family;
in_port_t sin_port;
struct in_addr sin_addr;
unsigned char sin_zero[sizeof (struct sockaddr) -
(sizeof (unsigned short int)) -
sizeof (in_port_t) -
sizeof (struct in_addr)];
};
so somehow it applies "strict aliasing" rules when fully scalarizing the
block copy. That needs to be fixed (with -fno-strict-aliasing an
aggregate copy needs to copy all padding). In this case the scalarization
is also very inefficiently using chars around the calloc call which
will force us to spill all the regs anyway.
All this worked in GCC 5 because somehow we didn't consider to scalarize
ss in the end:
Candidate (4191): ss
Rejected (4190): not aggregate: l
! Disqualifying ss - No scalar replacements to be created.
but I think we're just lucky here. Relevant accesses should be built from
the aggregate assignment
l_11->addr = ss;
which is again of sockaddr_storage type. sockaddr_storage should really have
the may_alias attribute (but in this case this won't help I think).
To recap, with -fno-strict-aliasing the code is valid and should work.
With -fstrict-aliasing the code is bogus and should use memcpy and not
aggregate assignment for both ss = *ss2 and l->addr = ss.
I will have a look Actually the bug is in the testcase as it aggregate-copies
struct sockaddr_storage
{
sa_family_t ss_family;
unsigned long int __ss_align;
char __ss_padding[(128 - (2 * sizeof (unsigned long int)))];
};
where by C rules an aggregate copy copies each field and thus leaves padding
undefined.
Thus this is a libc bug of having padding between ss_family and __ss_align.
And thus this has nothing to do with strict aliasing. It's rather a glibc
bug (or specification bug wherever sockaddr_storage is specified).
|
Hello, Starting with gcc-6, the following code (PoC found by Cyril Bonté) will produce an unexpected result: #include <stdio.h> #include <stdlib.h> #include <unistd.h> #include <string.h> #include <arpa/inet.h> struct dbg_listener { struct sockaddr_storage addr; }; int main(int argc, char **argv) { struct dbg_listener *l; struct sockaddr_storage ss, *ss2, ss3; memset(&ss3, 0, sizeof(ss3)); ss3.ss_family = AF_INET; ((struct sockaddr_in *)&ss3)->sin_addr.s_addr = inet_addr("127.0.0.1"); ss2 = &ss3; ss = *ss2; l = calloc(1, sizeof(*l)); l->addr = ss; printf("%x\n", ntohl(((struct sockaddr_in *)(&l->addr))->sin_addr.s_addr)); exit(0); } This is a common pattern when handling socket addresses in a generic way. Even when the code is compiled with -fno-strict-aliasing, the output is 0 while it should be 7f000001. $ gcc-6 -O2 -Wall -fno-strict-aliasing debug.c -o debug && ./debug 0 Using -O0 fixes the problem. Fixing the aliasing violation with an union fixes the problem too. Using -fno-tree-sra fixes the problem too. Compiling with gcc-5.3.1 fixes the problem too. Here is a good starting point for the context of this bug (in HAProxy): http://article.gmane.org/gmane.comp.web.haproxy/27924