Add default exception handlers and simplify default handler #16

robamu · 2025-03-25T08:59:21Z

Add three new default assembly exception handlers for data abort, prefetch abort and undefined exception. These handlers implement the ARM exception return model.
The new ASM handlers will call the Rust _default_handler by default, but the Rust handler can be overriden with _undefined_handler, _prefetch_handler and _abort_handler
The default handler is now a simple empty permanent loop

jonathanpallant · 2025-03-25T09:11:24Z

cortex-a-rt/src/lib.rs

+        // state save from compiled code
+        srsfd   sp!, {und_mode}
+    "#,
+    save_context_ignore_fpu!(),


I think it's worth a comment explaining why FPU regs don't have to be saved here.

Hmm.. Maybe we should save them? I have not put too much thought in this. I searched the code for various exception handlers (did not find much), none of them really saved the FPU regs. But I guess if the exception return works, it would be correct to safe them?

The handler could touch them, so I think they should be saved when you have the FPU feature on or are using the eabihf ABI.

jonathanpallant · 2025-03-25T09:14:18Z

cortex-r-rt/src/lib.rs

+//!
+//!   ```rust
+//!   #[unsafe(no_mangle)]
+//!   extern "C" fn boot_core(cpu_id: u32) -> !;


Is this the right function name? I'm fine with harmonising A and R run-times here.

no, copy & paste error.. Harmonizing them should not be too difficult actually. If I try this, should I do it in a separate PR?

yes, a separate PR I think

jonathanpallant · 2025-03-25T09:14:41Z

examples/mps3-an536/src/bin/svc.rs

@@ -30,7 +30,7 @@ pub fn main() -> ! {
 }

 /// This is our SVC exception handler
-#[no_mangle]
+#[unsafe(no_mangle)]


Is this OK with our MRSV?

Would CI not catch this? This is edition 2024 related I think.

https://doc.rust-lang.org/edition-guide/rust-2024/unsafe-attributes.html

To reflect the fact that these attributes can undermine Rust's safety guarantees, they are now considered "unsafe" and should be written as follows:

So it was added in Rust 1.82, and works in any edition. Edition 2024 just makes the new style mandatory.

jonathanpallant · 2025-03-25T09:15:36Z

This looks amazing. Just a couple of minor notes.

robamu · 2025-03-25T09:16:45Z

cortex-a-rt/src/lib.rs

+        // state save from compiled code
+        srsfd   sp!, {und_mode}
+    "#,
+    save_context_ignore_fpu!(),


Hmm.. Maybe we should save them? I have not put too much thought in this. I searched the code for various exception handlers (did not find much), none of them really saved the FPU regs. But I guess if the exception return works, it would be correct to safe them?

robamu · 2025-03-25T09:19:17Z

examples/mps3-an536/src/bin/svc.rs

@@ -30,7 +30,7 @@ pub fn main() -> ! {
 }

 /// This is our SVC exception handler
-#[no_mangle]
+#[unsafe(no_mangle)]


Would CI not catch this? This is edition 2024 related I think.

robamu · 2025-03-25T09:19:56Z

cortex-a-rt/src/lib.rs

 //! * `_asm_fiq_handler` - a naked function to call when a Fast Interrupt
 //!   Request (FIQ) occurs. Our linker script PROVIDEs a default function at
 //!   `_asm_default_fiq_handler` but you can override it.
 //! * `_asm_undefined_handler` - a naked function to call when an Undefined
 //!   Exception occurs. Our linker script PROVIDEs a default function at
-//!   `_asm_default_handler` but you can override it.
+//!   `_asm_defeault_undefined_handler` but you can override it.


robamu · 2025-03-25T09:21:58Z

cortex-r-rt/src/lib.rs

+//!
+//!   ```rust
+//!   #[unsafe(no_mangle)]
+//!   extern "C" fn boot_core(cpu_id: u32) -> !;


no, copy & paste error.. Harmonizing them should not be too difficult actually. If I try this, should I do it in a separate PR?

robamu · 2025-03-25T09:31:54Z

I now use the regular context saving/ restoration, tests still all run without issues.

robamu · 2025-03-25T09:33:51Z

By the way, I manually checked the faulting address argument against the binary disassembly when I wrote the tests, but I do not check them in the tests anymore. I think this would lead to extremely brittle tests, if a new version of the nightly compiler generates code with different layout.

jonathanpallant · 2025-03-25T10:05:36Z

By the way, I manually checked the faulting address argument against the binary disassembly when I wrote the tests, but I do not check them in the tests anymore. I think this would lead to extremely brittle tests, if a new version of the nightly compiler generates code with different layout.

I think that's fine. Another PR could try and run objdump or nm or something and check that the printed address matches what is in the ELF file.

jonathanpallant · 2025-03-25T10:11:24Z

cortex-a-rt/src/lib.rs

+        tst      r4, {t_bit}
+        // If not in Thumb mode, branch to not_thumb
+        beq     not_thumb
+	      subs	lr, lr, #2


I think a sneaky tab snuck in.

cortex-a-rt/src/lib.rs

cortex-ar/src/register/dfsr.rs

jonathanpallant · 2025-03-25T10:14:58Z

cortex-r-rt/src/lib.rs

+        tst      r4, {t_bit}
+        // If not in Thumb mode, branch to not_thumb
+        beq     not_thumb
+	      subs	lr, lr, #2


Suggested change

subs lr, lr, #2

subs lr, lr, #2

cortex-r-rt/src/lib.rs

jonathanpallant · 2025-03-25T10:15:48Z

cortex-r-rt/src/lib.rs

+    .type _asm_default_prefetch_handler, %function
+    _asm_default_prefetch_handler:
+        // Subtract 4 from the stored LR, see p.1212 of the ARMv7-A architecture manual.
+	      subs	lr, lr, #4


Suggested change

subs lr, lr, #4

subs lr, lr, #4

jonathanpallant · 2025-03-25T10:16:05Z

cortex-r-rt/src/lib.rs

+    .type _asm_default_abort_handler, %function
+    _asm_default_abort_handler:
+        // Subtract 8 from the stored LR, see p.1214 of the ARMv7-A architecture manual.
+	      subs	lr, lr, #8


Suggested change

subs lr, lr, #8

subs lr, lr, #8

cortex-r-rt/src/lib.rs

jonathanpallant · 2025-03-25T10:17:41Z

examples/versatileab/src/bin/undef-exception.rs

+
+#[no_mangle]
+unsafe extern "C" fn _undefined_handler(_faulting_instruction: u32) {
+    static mut COUNTER: u32 = 0;


Re-entering this function using a static mut is UB. It should probably be at AtomicU32 using fetch_add.

examples/versatileab/src/bin/prefetch-exception.rs

jonathanpallant · 2025-03-25T10:18:55Z

examples/versatileab/src/bin/prefetch-exception.rs

+
+#[unsafe(no_mangle)]
+unsafe extern "C" fn _prefetch_handler(_faulting_instruction: u32) {
+    static mut COUNTER: u32 = 0;


Re-entering this function using a static mut is UB. It should probably be at AtomicU32 using fetch_add.

jonathanpallant · 2025-03-25T10:19:24Z

examples/versatileab/src/bin/abt-exception.rs

+
+#[unsafe(no_mangle)]
+unsafe extern "C" fn _abort_handler(_faulting_instruction: u32) {
+    static mut COUNTER: u32 = 0;


Re-entering this function using a static mut is UB. It should probably be at AtomicU32 using fetch_add.

Right, replaced those with Atomics.

jonathanpallant · 2025-03-25T10:20:19Z

I did a second pass on my desktop rather than my phone, and I observed some misalignment in the assembly code.

- Add three new default assembly exception handlers for data abort, prefetch abort and undefined exception. These handlers implement the ARM exception return model. - The new ASM handlers will call the Rust _default_handler by default, but the Rust handler can be overriden with _undefined_handler, _prefetch_handler and _abort_handler - The default handler is now a simple empty permanent loop

jonathanpallant · 2025-03-25T14:44:39Z

One thing that would be good is tests for the mps3-an536, but we can't run those in CI, I'll just open a ticket.