Wed 29 Jun 2016 08:11:12 PM UTC, comment #8:
Patch works
|
Tue 28 Jun 2016 09:03:46 PM UTC, comment #7:
Yeah, the first warning is easy to fix, the problem are all the others :-)
Anyway, I just pushed ecbe45bf43 which should gives enough hint for
compilers to deal properly with those potentially unaligned accesses.
|
Tue 28 Jun 2016 08:19:55 PM UTC, comment #6:
I'll send an email via the list, I'm to bored to figure out how to escape the text (the "Rich markup" link above does not work)
|
Tue 28 Jun 2016 08:18:02 PM UTC, comment #5:
Just noticed the markup messed up most of the * indirections.
../../../../../lwip/src/netif/ppp/vj.c:165:28: error: cast from 'struct ip_hdr \' to 'u32_t \' (aka 'unsigned int *') increases required alignment from 1 to 4 [-Werror,-Wcast-align]
th = (struct tcp_hdr \)&((u32_t\)ip)[ilen];
|
Tue 28 Jun 2016 08:11:44 PM UTC, comment #4:
Yes, clang does not warn about casting from u16_t* to struct xxx. It complains about the first cast:
./../../../../lwip/src/netif/ppp/vj.c:165:28: error: cast from 'struct ip_hdr ' to 'u32_t ' (aka 'unsigned int *') increases required alignment from 1 to 4 [-Werror,-Wcast-align]
th = (struct tcp_hdr )&((u32_t)ip)[ilen];
in this case the variable ip is cast (a little bit indirectly) to an u32_t* by &((u32_t)ip)
-> would be fixed by
th = (struct tcp_hdr )&((u8_t)ip)[ilen<<2];
|
Tue 28 Jun 2016 08:07:12 PM UTC, comment #3:
Correct me if I am wrong, but my analysis is that it is just pointer
arithmetic, the target struct is packed, so whatever the result of
the offset computation the compiler should take care that unaligned
access is working.
|
Tue 28 Jun 2016 08:02:01 PM UTC, comment #2:
Are you sure about your analysis? The warnings are about casting from an UNALIGNED PACKED struct (struct ip_hdr, struct tcp_hdr) to a u32_t* or u16_t* which would require proper alignment.
|
Tue 28 Jun 2016 07:12:26 PM UTC, comment #1:
After investigating this, all of these warnings are about accesses on packed struct,
so for me this is a CLANG bug, it should not warn about unaligned access on
packed struct because that's something that is expected to happen.
What e40175ef05 and maybe other related commits about this issue did is only to add
a silencer for this warning using an ugly double cast with one using void* to fool
clang. It means that it doesn't fix the source of the warning at all, and it might
reappear if clang become more clever.
Until someone convince me this warning make sense, I'm just going to tell users
of buggy compilers to stop using them or to remove -Werror to their flags or to
remove the -Wsomething that produce this warning.
This bug can of course be re-opened once we have input if this warning actually
make sense, and with a solution that is not just a silencer with a time bomb
extra flavor :-)
|
Fri 24 Jun 2016 07:27:54 PM UTC, original submission:
clang -g -Wall -DLWIP_DEBUG -pedantic -Werror -Wparentheses -Wsequence-point -Wswitch-default -Wextra -Wundef -Wshadow -Wpointer-arith -Wcast-qual -Wc++-compat -Wwrite-strings -Wold-style-definition -Wcast-align -Wmissing-prototypes -Wredundant-decls -Wnested-externs -Wno-address -Wunreachable-code -Wuninitialized -Wno-format -I. -I../../../.. -I../../../../../lwip/src/include -I../../../../ports/unix/include -c ../../../../../lwip/src/netif/ppp/vj.c
../../../../../lwip/src/netif/ppp/vj.c:165:28: error: cast from 'struct ip_hdr ' to 'u32_t ' (aka 'unsigned int *') increases required alignment from 1 to 4 [-Werror,-Wcast-align]
th = (struct tcp_hdr )&((u32_t)ip)[ilen];
^~~~~~~~~~
../../../../../lwip/src/netif/ppp/vj.c:203:11: error: cast from 'struct tcp_hdr ' to 'u32_t ' (aka 'unsigned int *') increases required alignment from 1 to 4 [-Werror,-Wcast-align]
|| (u32_t)th != ((u32_t*)&cs->cs_ip)[IPH_HL(&cs->cs_ip)]) {
^~~~~~~~~~
../../../../../lwip/src/netif/ppp/vj.c:203:26: error: cast from 'struct ip_hdr ' to 'u32_t ' (aka 'unsigned int *') increases required alignment from 1 to 4 [-Werror,-Wcast-align]
|| (u32_t)th != ((u32_t*)&cs->cs_ip)[IPH_HL(&cs->cs_ip)]) {
^~~~~~~~~~~~~~~~~~
../../../../../lwip/src/netif/ppp/vj.c:224:15: error: cast from 'struct tcp_hdr ' to 'u32_t ' (aka 'unsigned int *') increases required alignment from 1 to 4 [-Werror,-Wcast-align]
&& (u32_t)th == ((u32_t*)&cs->cs_ip)[IPH_HL(&cs->cs_ip)]) {
^~~~~~~~~~
../../../../../lwip/src/netif/ppp/vj.c:224:30: error: cast from 'struct ip_hdr ' to 'u32_t ' (aka 'unsigned int *') increases required alignment from 1 to 4 [-Werror,-Wcast-align]
&& (u32_t)th == ((u32_t*)&cs->cs_ip)[IPH_HL(&cs->cs_ip)]) {
^~~~~~~~~~~~~~~~~~
../../../../../lwip/src/netif/ppp/vj.c:254:29: error: cast from 'struct ip_hdr ' to 'u32_t ' (aka 'unsigned int *') increases required alignment from 1 to 4 [-Werror,-Wcast-align]
oth = (struct tcp_hdr )&((u32_t)&cs->cs_ip)[ilen];
^~~~~~~~~~~~~~~~~~
../../../../../lwip/src/netif/ppp/vj.c:268:8: error: cast from 'struct ip_hdr ' to 'u16_t ' (aka 'unsigned short *') increases required alignment from 1 to 2 [-Werror,-Wcast-align]
if (((u16_t)ip)[0] != ((u16_t)&cs->cs_ip)[0]
^~~~~~~~~~
../../../../../lwip/src/netif/ppp/vj.c:268:27: error: cast from 'struct ip_hdr ' to 'u16_t ' (aka 'unsigned short *') increases required alignment from 1 to 2 [-Werror,-Wcast-align]
if (((u16_t)ip)[0] != ((u16_t)&cs->cs_ip)[0]
^~~~~~~~~~~~~~~~~~
../../../../../lwip/src/netif/ppp/vj.c:269:11: error: cast from 'struct ip_hdr ' to 'u16_t ' (aka 'unsigned short *') increases required alignment from 1 to 2 [-Werror,-Wcast-align]
|| ((u16_t)ip)[3] != ((u16_t)&cs->cs_ip)[3]
^~~~~~~~~~
../../../../../lwip/src/netif/ppp/vj.c:269:30: error: cast from 'struct ip_hdr ' to 'u16_t ' (aka 'unsigned short *') increases required alignment from 1 to 2 [-Werror,-Wcast-align]
|| ((u16_t)ip)[3] != ((u16_t)&cs->cs_ip)[3]
^~~~~~~~~~~~~~~~~~
../../../../../lwip/src/netif/ppp/vj.c:270:11: error: cast from 'struct ip_hdr ' to 'u16_t ' (aka 'unsigned short *') increases required alignment from 1 to 2 [-Werror,-Wcast-align]
|| ((u16_t)ip)[4] != ((u16_t)&cs->cs_ip)[4]
^~~~~~~~~~
../../../../../lwip/src/netif/ppp/vj.c:270:30: error: cast from 'struct ip_hdr ' to 'u16_t ' (aka 'unsigned short *') increases required alignment from 1 to 2 [-Werror,-Wcast-align]
|| ((u16_t)ip)[4] != ((u16_t)&cs->cs_ip)[4]
^~~~~~~~~~~~~~~~~~
../../../../../lwip/src/netif/ppp/vj.c:591:8: error: cast from 'struct ip_hdr ' to 'u16_t ' (aka 'unsigned short *') increases required alignment from 1 to 2 [-Werror,-Wcast-align]
bp = (u16_t*) &cs->cs_ip;
^~~~~~~~~~~~~~~~~~~
13 errors generated.
|