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

Stack unwinding can now unwind beyond (optionally nested) exception handlers for selected architectures. #1665

Merged
merged 21 commits into from
Jul 13, 2023

Conversation

noppej
Copy link
Contributor

@noppej noppej commented Jul 12, 2023

This is a replacement for the proof of concept #1495.

Stack unwinding can now unwind beyond (optionally nested) exception handlers (# ).

  • ARMv6-M: Report Exception / Fault description, and Unwind the registers and next frames.
  • ARMv7-M: Also decodes details about HardFault, UsageFault, BusFault, and MemManageFault.
  • ARMv7-A, Armv8-M, Armv8-A, RISC-V: Not implemented - requires architecture specific implementations, but the module contains default trait implementations that allow the existing functionality for unwind to work as before, i.e. unwind all frames to the first signal handler encountered.

See the white arrows below for new information available:
UnwindException

@noppej
Copy link
Contributor Author

noppej commented Jul 12, 2023

Note for reviewer: I don't understand why Run CI / Check docs is failing ... the error message doesn't make sense. Can you point me in the right direction please?

@Yatekii
Copy link
Member

Yatekii commented Jul 12, 2023

Note for reviewer: I don't understand why Run CI / Check docs is failing ... the error message doesn't make sense. Can you point me in the right direction please?

Well, RegisterRole is private and you are linking to it in public docs :) make RegisterRole public and you're good :)

CHANGELOG.md Show resolved Hide resolved
@noppej
Copy link
Contributor Author

noppej commented Jul 12, 2023

Note for reviewer: I don't understand why Run CI / Check docs is failing ... the error message doesn't make sense. Can you point me in the right direction please?

Well, RegisterRole is private and you are linking to it in public docs :) make RegisterRole public and you're good :)

@Yatekii But it is a pub enum ...???
image

core: &mut T,
stackframe_registers: &crate::debug::DebugRegisters,
) -> Result<crate::debug::DebugRegisters, crate::Error> {
let mut calling_stack_registers = vec![0u32; EXCEPTION_STACK_REGISTERS.len()];
Copy link
Member

Choose a reason for hiding this comment

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

This code makes little sense to me. I am not sure if it is bogus or if it should be restructured. Some function description would be nice :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've described the logic and why this is needed in the function docs :)

Copy link
Member

@Yatekii Yatekii left a comment

Choose a reason for hiding this comment

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

Very nice, thank you!

I added - as is customary :D - a lot of comments :)

@Yatekii
Copy link
Member

Yatekii commented Jul 12, 2023

@Yatekii But it is a pub enum ...???

That does not mean it's parent module is too =)

@noppej
Copy link
Contributor Author

noppej commented Jul 13, 2023

@Yatekii But it is a pub enum ...???

That does not mean it's parent module is too =)

@Yatekii Oooh ... good catch. That gives me 3 possible solutions:

  1. Make the parent module core public (that seems a bit extreme).
  2. Modify the CI as per the error suggestion "note: this link will resolve properly if you pass --document-private-items"
  3. Modify the doc comment to be plain text, vs. a doc link.

Do you have preference?

@Yatekii
Copy link
Member

Yatekii commented Jul 13, 2023

For sure 1. should be the chosen solution. But you do not necessarily have to make the entire module public. It suffices if you publicly re-export the private item :)

@noppej noppej requested a review from Yatekii July 13, 2023 16:31
@Yatekii Yatekii added this pull request to the merge queue Jul 13, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Jul 13, 2023
@Yatekii Yatekii enabled auto-merge July 13, 2023 21:22
@Yatekii Yatekii added this pull request to the merge queue Jul 13, 2023
Merged via the queue into master with commit 51fa324 Jul 13, 2023
10 checks passed
@Yatekii Yatekii deleted the register_preserves branch July 13, 2023 21:29
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.

None yet

2 participants