Skip to content

cpu/arm7_common: Fix compilation with -Wcast-align #15100

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 2 commits into from
Sep 28, 2020
Merged

cpu/arm7_common: Fix compilation with -Wcast-align #15100

merged 2 commits into from
Sep 28, 2020

Conversation

maribu
Copy link
Member

@maribu maribu commented Sep 28, 2020

Contribution description

  • Enforced that stacks are aligned to word size in C code
    • Hopefully, this was already the case through the linker script. In this case, the actual alignment just got more explicit and visible in the C code. Otherwise, this fixes a bug
  • Enforced that ISR_STACKSIZE is a multiple of 4 (so that the end pointer is also aligned to word size)
  • With this, all casts from pointer to uint32_t pointers now don't drop alignment requirements
    • Silenced all -Wcast-align warnings

Testing procedure

Hopefully, this doesn't change binaries. But if so, stack usage should still be properly measured.

Issues/PRs references

Split out of #14955

@maribu
cpu/arm7_common: Align thread stacks to 32 bit
cc23822 
This didn't change binaries for me. Either the linker script already took care
of it through the section names of the stacks, or I just was lucky. If I was
just luck, this fixes a bug. If not, it makes the hidden alignment explicit in
the C code, so that code review is easier.
@maribu
cpu/arm7_common: Silence -Wcast-align
833afc0 
- Enforced that ISR_STACKSIZE is indeed a multiple of 4
- With this enforced, every cast that triggers a -Wcast-align warning is now
  a false positives, so those were silenced by (intermediately) casting to
  `uintptr_t`.
@maribu maribu added Platform: ARM Platform: This PR/issue effects ARM-based platforms Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Area: cpu Area: CPU/MCU ports labels Sep 28, 2020
@maribu maribu requested a review from benpicco September 28, 2020 09:06
@maribu maribu requested a review from kaspar030 as a code owner September 28, 2020 09:06
@benpicco benpicco 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 approved these changes Sep 28, 2020
View reviewed changes
Copy link
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Binaries do no change (build examples/default for msba2)

@maribu maribu merged commit c6fe777 into RIOT-OS:master Sep 28, 2020
@maribu
Copy link
Member Author

maribu commented Sep 28, 2020

Thanks! :-)

@maribu maribu deleted the arm7-cast-align branch September 28, 2020 14:35
@maribu maribu mentioned this pull request Nov 14, 2021
Merged
20 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benpicco benpicco benpicco approved these changes

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

Assignees
No one assigned
Labels
Area: cpu Area: CPU/MCU ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Platform: ARM Platform: This PR/issue effects ARM-based platforms
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@maribu @benpicco