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

cortex-m-rt: initialize the frame pointer chain. #468

Closed
wants to merge 1 commit into from

Conversation

Dirbaio
Copy link
Member

@Dirbaio Dirbaio commented Feb 14, 2023

AAPCS says the starting value should be 0: https://github.com/ARM-software/abi-aa/blob/main/aapcs32/aapcs32.rst#6214the-frame-pointer

The end of the frame record chain is indicated by the address zero in the address for the previous frame

AAPCS says the starting value should be 0: https://github.com/ARM-software/abi-aa/blob/main/aapcs32/aapcs32.rst#6214the-frame-pointer

> The end of the frame record chain is indicated by the address zero in the address for the previous frame
@Dirbaio Dirbaio requested a review from a team as a code owner February 14, 2023 00:54
@adamgreig
Copy link
Member

adamgreig commented Feb 14, 2023

I'm not one to disagree with AAPCS, but I'm not sure there's any point including this extra instruction in the startup. It doesn't seem like any debugger even looks for this value, let alone does anything with a 0 value, and because no other startup code we've checked sets it, it seems very unlikely any debugger will start caring about it either. I agree in principle if every (or even most, or even any) startup reliably set FP=0 then a debugger could use this to help unwinding, but since debuggers don't do it, and startups don't do it, I'm not convinced we should be the ones to start doing it.

That said, I've not investigated debuggers besides gdb and probe-run, and only the handful of startup codes in #463 (comment), so I'd welcome feedback that suggests this would actually be useful for someone. Otherwise it seems a waste, even if a very very small waste, to include it.

@Dirbaio Dirbaio closed this Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants