Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pointer alignmnet issues on 32 bit ARM #93

Closed
Jkillelea opened this issue May 22, 2020 · 5 comments · Fixed by #130 or #133
Closed

Pointer alignmnet issues on 32 bit ARM #93

Jkillelea opened this issue May 22, 2020 · 5 comments · Fixed by #130 or #133
Labels
bug Something isn't working
Milestone

Comments

@Jkillelea
Copy link

It looks like some struct has changed size and it no longer aligns nicely with 32 bit ARM. It's possible that this software simply shouldn't be compiled on this platform and instead it should be reserved for an x86/x86_64 based ground station.

Steps to reproduce the behavior:

  1. Clone cFS from https://github.com/nasa/cFS
  2. Compile

Expected behavior:
Structs sizes should align so that casts can be made on platforms that do not allow unaligned access (such as ARM).

Code snips:

Scanning dependencies of target mission-all
Scanning dependencies of target cmdUtil
[ 14%] Building C object tools/cFS-GroundSystem/Subsystems/cmdUtil/CMakeFiles/cmdUtil.dir/cmdUtil.c.o
[ 28%] Building C object tools/cFS-GroundSystem/Subsystems/cmdUtil/CMakeFiles/cmdUtil.dir/SendUdp.c.o
/home/pi/upstream-cfs/tools/cFS-GroundSystem/Subsystems/cmdUtil/SendUdp.c: In function ‘SendUdp’:
/home/pi/upstream-cfs/tools/cFS-GroundSystem/Subsystems/cmdUtil/SendUdp.c:90:20: error: cast increases required alignment of target type [-Werror=cast-align]
         inet_ntoa(((struct sockaddr_in*)result->ai_addr)->sin_addr), port);
                    ^
cc1: all warnings being treated as errors
tools/cFS-GroundSystem/Subsystems/cmdUtil/CMakeFiles/cmdUtil.dir/build.make:86: recipe for target 'tools/cFS-GroundSystem/Subsystems/cmdUtil/CMakeFiles/cmdUtil.dir/SendUdp.c.o' failed
make[7]: *** [tools/cFS-GroundSystem/Subsystems/cmdUtil/CMakeFiles/cmdUtil.dir/SendUdp.c.o] Error 1
CMakeFiles/Makefile2:462: recipe for target 'tools/cFS-GroundSystem/Subsystems/cmdUtil/CMakeFiles/cmdUtil.dir/all' failed
make[6]: *** [tools/cFS-GroundSystem/Subsystems/cmdUtil/CMakeFiles/cmdUtil.dir/all] Error 2
Makefile:138: recipe for target 'all' failed
make[5]: *** [all] Error 2
CMakeFiles/mission-all.dir/build.make:57: recipe for target 'CMakeFiles/mission-all' failed
make[4]: *** [CMakeFiles/mission-all] Error 2
CMakeFiles/Makefile2:164: recipe for target 'CMakeFiles/mission-all.dir/all' failed
make[3]: *** [CMakeFiles/mission-all.dir/all] Error 2
CMakeFiles/Makefile2:171: recipe for target 'CMakeFiles/mission-all.dir/rule' failed
make[2]: *** [CMakeFiles/mission-all.dir/rule] Error 2
Makefile:212: recipe for target 'mission-all' failed
make[1]: *** [mission-all] Error 2
Makefile:120: recipe for target 'all' failed
make: *** [all] Error 2
pi@raspberrypi:~/upstream-cfs $ uname -a
Linux raspberrypi 4.19.66+ #1253 Thu Aug 15 11:37:30 BST 2019 armv6l GNU/Linux

System observed on:

  • Raspberry Pi Zero
  • OS: Linux 4.19
  • cFS git commit a4345d87c01b1818bdb217ffd4dcce8bf86e33fe, cFS-GroundSystem git commit 1c31f94f3b63d3ff9c5da35ca9f8825a89ef3258
@Jkillelea
Copy link
Author

Jkillelea commented May 22, 2020

On closer inspection, the offending code seems to be a printf, casting from struct addrinfo* to struct sockaddr_in*.

https://github.com/nasa/cFS-GroundSystem/blob/master/Subsystems/cmdUtil/SendUdp.c#L89

Weird. Both pointer types appear to have the same size and alignment according to sizeof and alignof.

@jphickey
Copy link
Contributor

The problem is that it's casting a struct sockaddr* to a struct sockaddr_in * ... which is generally the way its supposed to be done except that struct sockaddr probably is what's missing the alignment.

Roundabout fix would be to create a union with both a struct sockaddr and struct sockaddr_in member, then copy the data.. The code should also really be checking ai_addrlen and ai_family if not already doing so.

@skliper
Copy link
Contributor

skliper commented May 22, 2020

Or use getnameinfo?

Either way that function needs some work...

@Jkillelea
Copy link
Author

Jkillelea commented May 23, 2020

In the meantime, it looks like arch_build_custom.cmake and mission_build_custom.cmakecan be edited to suppress pointer alignment errors. Probably not a good long term fix.

@skliper
Copy link
Contributor

skliper commented Aug 31, 2020

Also observed by @hukuzatuna. Recreated on gcc 8+ with cast-align=strict.

skliper added a commit to skliper/cFS-GroundSystem that referenced this issue Aug 31, 2020
skliper added a commit to skliper/cFS-GroundSystem that referenced this issue Sep 1, 2020
yammajamma added a commit that referenced this issue Sep 3, 2020
Fix #93, Refactor and resolve cast align warning
@astrogeco astrogeco added this to the 2.3.0 milestone Oct 1, 2020
@astrogeco astrogeco added the bug Something isn't working label Oct 1, 2020
@astrogeco astrogeco modified the milestones: 2.3.0, 3.0.0 Jan 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
4 participants