buglwIP - A Lightweight TCP/IP stack - Bugs: bug #50883, struct eth_addr alignment does not...

 
 

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

bug #50883: struct eth_addr alignment does not fit with ETHADDR16_COPY

Submitter:  Daniel Elstner <danielk>
Submitted:  Wed 26 Apr 2017 02:13:16 PM UTC
   
 
Category:  ARP Severity:  3 - Normal
Item Group:  Crash Error Status:  Fixed
Privacy:  Public Assigned to:  None
Open/Closed:  Closed Planned Release:  None
lwIP version:  git head

Sun 30 Jul 2017 03:52:23 PM UTC, comment #3: 

Removed the macro. Noone advocated to keep it.

Dirk Ziegelmeier <dziegel>
Group administrator
Wed 26 Jul 2017 03:35:34 PM UTC, comment #2: 

Would it be OK to remove that micro-optimization altogether? It's hard to get it right in all cases, and is it worth the effort???

Dirk Ziegelmeier <dziegel>
Group administrator
Thu 27 Apr 2017 08:26:43 AM UTC, comment #1: 


> I think it's wrong for the ARP code to use the same packed
> type from the proto header for its own local/global variables.


Since we have a 2nd ip4_addr type, we can also have a 2nd eth_addr type. I never understood why this tiny performance optimization is necessary, but I'm not against doing that if it has decent default behaviour (i.e. don't change the current behaviour if not used)

Simon Goldschmidt <goldsimon>
Group administrator
Wed 26 Apr 2017 02:13:16 PM UTC, original submission:  

I tracked down a crash in my project to an alignment fault in etharp_raw(). In the crashing case one of the arguments points to the global constant ethzero, which is a struct eth_addr. This struct has only 8-bit members so the compiler is free to place it at any byte address. Since I define ETHADDR16_COPY() to use 16-bit reads/writes, an alignment exception is triggered.

The minimum alignment could be changed by making the type a union including some u16_t members, or a compiler attribute could be used. However, that would still not help if struct packing is enabled (it would work for me only because I happen to disable it).

I think it's wrong for the ARP code to use the same packed type from the proto header for its own local/global variables.

Daniel Elstner <danielk>

 

(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 dziegel (Posted a comment)
  • -email is unavailable- added by goldsimon (Updated the item)
  • -email is unavailable- added by danielk (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 3 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2017-07-30 dziegel StatusNone Fixed
        Open/ClosedOpen Closed
    2017-04-26 goldsimon Summarystruct eth_addr alignment error struct eth_addr alignment does not fit with ETHADDR16_COPY

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code