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

Submitted by:  Daniel Elstner <danielk>
Submitted on:  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>
Project 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>
Project 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>
Project 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>

 

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)
  •  

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

    Only logged-in users can vote.

     

     

     

    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.5