Skip to content

Conversation

@jonathanpallant
Copy link
Contributor

@jonathanpallant jonathanpallant commented Apr 9, 2025

It was looking at CPSR's Thumb bit, which tells you if the handler is in thumb mode, not the code that threw the fault.

I changed the undef-exception test to validate the address of the failing function, to verify that we've got this right. I also split the prefetch-exception test into separate T32 and A32 versions, to check that we return back into the correct state. I didn't bother splitting the data abort test.

I also fixed the issue of _asm_default_undefined_handler damaging r4. You have to save all the state first, then you can touch registers.

@jonathanpallant
Copy link
Contributor Author

cc @robamu

@jonathanpallant jonathanpallant changed the title Fix the undef handler. Draft: Fix the exception handlers. Apr 9, 2025
@jonathanpallant jonathanpallant changed the title Draft: Fix the exception handlers. Fix the exception handlers. Apr 9, 2025
@jonathanpallant
Copy link
Contributor Author

OK, I think this is good to go. I learned a lot about exception handling today 🙃

@jonathanpallant jonathanpallant changed the title Fix the exception handlers. Draft: Fix the exception handlers. Apr 9, 2025
@jonathanpallant
Copy link
Contributor Author

No, it's still wrong 😭

The undefined exception handler is supposed to also return to the faulting instruction, and we currently don't. This is because we push LR before we work out which mode we're in, and then pop the original LR of the stack with RFEFD. We need to save our state manually.

Comment on lines +40 to +51
core::arch::global_asm!(
r#"
// fn bkpt_from_a32();
.arm
.global bkpt_from_a32
.type bkpt_from_a32, %function
bkpt_from_a32:
bkpt #0
bx lr
.size bkpt_from_a32, . - bkpt_from_a32
"#
);

Choose a reason for hiding this comment

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

unrelated, but this looks like it could fit cortex-a::asm/cortex-r::asm. Perhaps as an unsafe function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but it doesn't help here because I need to know the precise address of the instruction that does the breakpoint.

It was looking at CPSR's Thumb bit, which tells you if the *handler* is in thumb mode, not the code that threw the fault.

Change the test to validate the address of the failing function, to verify that we've got this right.

Also fixes the issue of _asm_default_undefined_handler damaging r4. You have to save all the state first, then you can touch registers.
One for T32 and one for A32. We have to do this because the exception is non-recoverable (we return to the faulting instruction, which faults again).
I had to move the DFSR read to after the alignment check was disabled, otherwise it double-faulted.

Also it turns out the versatileab tests were running with the Cortex-A linker script not the Cortex-R linker script, and the Cortex-R linker script had a bug in it that we previously missed (it wasn't calling the new exception handlers).
@jonathanpallant
Copy link
Contributor Author

😭 😭 😭 😭

OK, I think I now work out which mode (A32 or T32) tripped the undefined abort, without damaging any registers. This took a lot of work to debug, but it seems to work now?

@jonathanpallant jonathanpallant changed the title Draft: Fix the exception handlers. Fix the exception handlers. Apr 9, 2025
@jonathanpallant
Copy link
Contributor Author

Tentatively claiming that I am finished here.

thejpster and others added 2 commits April 9, 2025 22:08
- The SVC asm trampoline can now be over-ridden
- The Undefined, Prefetch and Abort handlers can either return never, or can return a new address to continue executing from when the handler is over
- I tried to ensure the all the handlers are listed in the same order as the vector table, for consistency
- Put every function in its own section in the hope that unused ones will be GC'd by the linker
More exception handling updates.
@jonathanpallant
Copy link
Contributor Author

jonathanpallant commented Apr 9, 2025

Reader, I was not finished here.

But now I am. I swear.

@@ -1,22 +1,25 @@
//! # Run-time support for Arm Cortex-A
Copy link
Contributor

Choose a reason for hiding this comment

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

Cortex-A

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤦

@robamu
Copy link
Contributor

robamu commented Apr 10, 2025

really nice work!

Also clarify that both only apply to AArch32 systems. The terms 'Cortex-R'
and 'Cortex-A' are particular brands of processor made by Arm, and there
are processors under each of those brands that run in (or only run in)
AArch64 mode.
@jonathanpallant
Copy link
Contributor Author

Let's go! Before I fix something else.

@jonathanpallant jonathanpallant merged commit b493320 into main Apr 10, 2025
57 checks passed
restore_context!(),
r#"
// get our saved LR
pop {{lr}}
Copy link

@chrisnc chrisnc Apr 15, 2025

Choose a reason for hiding this comment

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

This will clobber the lr that was returned by _undefined_handler, right? The advertised API suggests the callee can cause the handler to return to an arbitrary new instruction, and mov lr, r0 and associated comment appear to be trying to support that, but that register move is then discarded here and replaced with the value that was pushed before save_context.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's a bug. I added support for returning a new location because I realised that sometimes you want to emulate the faulting instruction and return to the one after. But I didn't write a test for it and this is what happens when you don't test things. Thank you for checking so diligently!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See #29

@jonathanpallant jonathanpallant deleted the fix-undef-handler branch September 20, 2025 09:18
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.

5 participants