Skip to content

makefiles/cflags.inc.mk: Add -Wcast-align #14955

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 45 commits into from
Dec 3, 2021
Merged

makefiles/cflags.inc.mk: Add -Wcast-align #14955

merged 45 commits into from
Dec 3, 2021

Conversation

maribu
Copy link
Member

@maribu maribu commented Sep 5, 2020 •
edited
Loading

Contribution description

As the title says. This is useful to detect issues when doing things like

uint8_t foo[4];
uint32_t *bar = (uint32_t *)foo;
*bar = 0xacdcacdc;

Because in the above example foo has an alignment requirement of 1 byte, but bar has (on most 32 bit archs) an alignment requirement of 4 bytes. Unaligned writes will (depending on architecture) either result faults, writes to the next aligned address instead (usually rounded always down), or slow performance (and writes no longer being atomically).

Testing procedure

Green Murdock.

Issues/PRs references

Depends on:

@maribu maribu added Area: build system Area: Build system Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation labels Sep 5, 2020
@maribu
Copy link
Member Author

maribu commented Sep 5, 2020

Let's see how many issues this uncovers.

@maribu maribu added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Sep 5, 2020
@maribu
Copy link
Member Author

maribu commented Sep 6, 2020

Let's see how many issues this uncovers.

Oh oh :-( Sounds like some nights of bug fixing.

@fjmolinas
Copy link
Contributor

How many of these are there? Do we need to enable them gradually?

@maribu
Copy link
Member Author

maribu commented Sep 21, 2020

I'm now back from parental leave. So be prepared for a bunch of bugfix PRs addressing the issues unveiled by this PR ;-)

I hope to have RIOT's code base cleaned up by the end of the week. So I think we don't need to enable this gradually. Maybe there are only just a few issues - but in very central places -, resulting in the high number of build failures.

@maribu maribu removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Sep 22, 2020
This was referenced Sep 22, 2020
Merged
Merged
Merged
Merged
@maribu maribu force-pushed the cflags-cast-align branch 2 times, most recently from 9eb8806 to cf09a1c Compare September 28, 2020 08:45
This was referenced Sep 28, 2020
Merged
Merged
@maribu maribu added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Sep 28, 2020
@benpicco benpicco added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Nov 17, 2020
maribu added 22 commits December 3, 2021 10:12
@maribu
pkg/uwb-dw1000: add -Wno-cast-align
87c9032 
Also sort disabled CFLAG warnings alphabetically.
@maribu
pkg/paho-mqtt: add -Wno-cast-align
2cb0652
@maribu
pkg/wakaama: add -Wcast-align
d41d2b4
@maribu
pkg/cmsis-nn: add -Wno-cast-align
92d333a
@maribu
tests/pkg_cmsis-nn: add -Wno-cast-align
e4c627d 
This should be reverted and properly fixed.
@maribu
sys/shell: use container_of in sc_gnrc_netif
be42be9
@maribu
pkg/openwsn: add -Wno-cast-align
c4346f7 
Also sort flags alphabetically
@maribu
tests/pkg_openwsn_sock_udp: silence -Wcast-align
afdaa2f
@maribu
pkg/cryptoauthlib: add -Wno-cast-align
d16f536 
Also split compiler flags into individual lines and sort them
alphabetically.
@maribu
pkg/elk: add -Wno-cast-align
1e088dc
@maribu
tests/pkg_emlearn: fix compilation with -Wcast-align
5881261
@maribu
pkg/qdsa: add -Wno-cast-align
c20c0b9
@maribu
pkg/libb2: add -Wno-cast-align
d91c2e9
@maribu
pkg/cn-cbor: add -Wno-cast-align
1193785
@maribu
tests/pkg_utensor: fix alignment bug
3a07baf
@maribu
pkg/lvgl: add -Wno-cast-align
724d4f6
@maribu
pkg/spifs: silence -Wcast-align
c50c5ea
@maribu
pkg/ucglib: silence -Wcast-align
a6eaddb
@maribu
pkg/flatbuffers: add -Wno-cast-align
b005e8f
@maribu
pkg/tensorflow-lite: add -Wno-cast-align
db41e3a
@maribu
pkg/libhydrogen: add -Wno-cast-align
c67d861
@maribu
sys/usb/usbus/dfu: use container_of()
d571f15
@maribu maribu force-pushed the cflags-cast-align branch from ef757be to 2145ca0 Compare December 3, 2021 09:25
@maribu maribu force-pushed the cflags-cast-align branch from 2145ca0 to 63b54b5 Compare December 3, 2021 13:22
@fjmolinas fjmolinas merged commit 3afa47d into RIOT-OS:master Dec 3, 2021
@maribu
Copy link
Member Author

maribu commented Dec 3, 2021

Thanks for the review!

@maribu maribu deleted the cflags-cast-align branch December 3, 2021 16:55
@fjmolinas fjmolinas added this to the Release 2022.01 milestone Jan 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fjmolinas fjmolinas fjmolinas approved these changes

@benpicco benpicco Awaiting requested review from benpicco

@kaspar030 kaspar030 Awaiting requested review from kaspar030 kaspar030 is a code owner

@DipSwitch DipSwitch Awaiting requested review from DipSwitch

@haukepetersen haukepetersen Awaiting requested review from haukepetersen

@thomaseichinger thomaseichinger Awaiting requested review from thomaseichinger

@cgundogan cgundogan Awaiting requested review from cgundogan

@miri64 miri64 Awaiting requested review from miri64 miri64 is a code owner

@PeterKietzmann PeterKietzmann Awaiting requested review from PeterKietzmann PeterKietzmann is a code owner

@aabadie aabadie Awaiting requested review from aabadie aabadie is a code owner

@basilfx basilfx Awaiting requested review from basilfx basilfx is a code owner

@gschorcht gschorcht Awaiting requested review from gschorcht gschorcht is a code owner

@vincent-d vincent-d Awaiting requested review from vincent-d vincent-d is a code owner

@MrKevinWeiss MrKevinWeiss Awaiting requested review from MrKevinWeiss

@leandrolanzieri leandrolanzieri Awaiting requested review from leandrolanzieri leandrolanzieri is a code owner

@MichelRottleuthner MichelRottleuthner Awaiting requested review from MichelRottleuthner MichelRottleuthner is a code owner

@smlng smlng Awaiting requested review from smlng

@bergzand bergzand Awaiting requested review from bergzand bergzand is a code owner

@dylad dylad Awaiting requested review from dylad dylad is a code owner

Assignees
No one assigned
Labels
Area: BLE Area: Bluetooth Low Energy support Area: boards Area: Board ports Area: build system Area: Build system Area: core Area: RIOT kernel. Handle PRs marked with this with care! Area: cpu Area: CPU/MCU ports Area: drivers Area: Device drivers Area: network Area: Networking Area: pkg Area: External package ports Area: sys Area: System Area: tests Area: tests and testing framework Area: USB Area: Universal Serial Bus CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ARM Platform: This PR/issue effects ARM-based platforms Platform: ESP Platform: This PR/issue effects ESP-based platforms Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Milestone
Release 2022.01
Development

Successfully merging this pull request may close these issues.
5 participants
@maribu @fjmolinas @bergzand @benpicco @MrKevinWeiss