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

Submitted by:  Dirk Ziegelmeier <dziegel>
Submitted on:  Fri 24 Jun 2016 07:27:54 PM UTC  
 
Category: PPPSeverity: 3 - Normal
Item Group: Compiler WarningStatus: Fixed
Privacy: PublicAssigned to: Sylvain Rochet <gradator>
Open/Closed: ClosedPlanned Release: None
lwIP version: git head

(Jump to the original submission Jump to the original submission)

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

Patch works

Dirk Ziegelmeier <dziegel>
Project Member
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>
Project MemberIn charge of this item.
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>
Project Member
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>
Project Member
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>
Project Member
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>
Project MemberIn charge of this item.
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>
Project Member
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>
Project MemberIn charge of this item.
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>
Project Member

 

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

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

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 8 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 29 Jun 2016 08:11:12 PM UTCdziegelStatusConfirmed=>Fixed
      Open/ClosedOpen=>Closed
    Tue 28 Jun 2016 08:02:01 PM UTCdziegelStatusWont Fix=>Confirmed
      Open/ClosedClosed=>Open
    Tue 28 Jun 2016 07:12:26 PM UTCgradatorStatusConfirmed=>Wont Fix
      Open/ClosedOpen=>Closed
    Sun 26 Jun 2016 04:10:55 PM UTCgradatorStatusNone=>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.1-cleanup1