buglwIP - A Lightweight TCP/IP stack - Bugs: bug #48308, Warnings in PPP Van Jacobson code...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #48308: Warnings in PPP Van Jacobson code when compiling with clang

Submitter:  Dirk Ziegelmeier <dziegel>
Submitted:  Fri 24 Jun 2016 07:27:54 PM UTC
   
 
Category:  PPP Severity:  3 - Normal
Item Group:  Compiler Warning Status:  Fixed
Privacy:  Public Assigned to:  gradator
Open/Closed:  Closed Planned Release:  None
lwIP version:  git head

Jump to the original submission

Wed 29 Jun 2016 08:11:12 PM UTC, comment #8: 

Patch works

Dirk Ziegelmeier <dziegel>
Group administrator
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.

Sylvain Rochet <gradator>
Group Member
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)

Dirk Ziegelmeier <dziegel>
Group administrator
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];

Dirk Ziegelmeier <dziegel>
Group administrator
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];

Dirk Ziegelmeier <dziegel>
Group administrator
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.

Sylvain Rochet <gradator>
Group Member
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.

Dirk Ziegelmeier <dziegel>
Group administrator
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 :-)

Sylvain Rochet <gradator>
Group Member
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.

Dirk Ziegelmeier <dziegel>
Group administrator

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by gradator (Updated the item)
  • -email is unavailable- added by dziegel (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-06-29 dziegel StatusConfirmed Fixed
        Open/ClosedOpen Closed
    2016-06-28 dziegel StatusWont Fix Confirmed
        Open/ClosedClosed Open
    2016-06-28 gradator StatusConfirmed Wont Fix
        Open/ClosedOpen Closed
    2016-06-26 gradator StatusNone Confirmed
        SummaryWarnings in PPP van Jacobsen code when compiling with clang Warnings in PPP Van Jacobson code when compiling with clang

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code