Fix the undef handler.

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.

Author: jonathanpallant

cortex-a-rt/src/lib.rs

@@ -91,7 +91,7 @@
 //!
 //!   ```rust
 //!   #[unsafe(no_mangle)]
-//!   extern "C" fn _undefined_handler(faulting_instruction: u32);
+//!   extern "C" fn _undefined_handler(addr: usize);
 //!   ```
 //!
 //! * `_abort_handler` - an `extern "C"` function to call when a Data Abort Exception
@@ -103,7 +103,7 @@
 //!
 //!   ```rust
 //!   #[unsafe(no_mangle)]
-//!   extern "C" fn _abort_handler(faulting_instruction: u32);
+//!   extern "C" fn _abort_handler(addr: usize);
 //!   ```
 //!
 //! * `_prefetch_handler` - an `extern "C"` function to call when a Prefetch Abort Exception
@@ -115,7 +115,7 @@
 //!
 //!   ```rust
 //!   #[unsafe(no_mangle)]
-//!   extern "C" fn _prefetch_handler(faulting_instruction: u32);
+//!   extern "C" fn _prefetch_handler(addr: usize);
 //!   ```
 //!
 //! ### ASM functions
@@ -130,7 +130,7 @@
 //! * `_asm_undefined_handler` - a naked function to call when an Undefined
 //!   Exception occurs. Our linker script PROVIDEs a default function at
 //!   `_asm_default_undefined_handler` but you can override it.
-//! * `_asm_prefetch_handler` - a naked function to call when an Prefetch
+//! * `_asm_prefetch_handler` - a naked function to call when a Prefetch
 //!   Exception occurs. Our linker script PROVIDEs a default function at
 //!   `_asm_default_prefetch_handler` but you can override it. The provided default
 //!   handler will perform an exception return to the faulting address.
@@ -381,7 +381,7 @@ core::arch::global_asm!(
 
     // Called from the vector table when we have an software interrupt.
     // Saves state and calls a C-compatible handler like
-    // `extern "C" fn svc_handler(svc: u32, context: *const u32);`
+    // `extern "C" fn svc_handler(svc: u32);`
     .global _asm_svc_handler
     .type _asm_svc_handler, %function
     _asm_svc_handler:
@@ -426,31 +426,27 @@ core::arch::global_asm!(
 
     // Called from the vector table when we have an undefined exception.
     // Saves state and calls a C-compatible handler like
-    // `extern "C" fn _undefined_handler();`
+    // `extern "C" fn _undefined_handler(addr: usize);`
     .global _asm_default_undefined_handler
     .type _asm_default_undefined_handler, %function
     _asm_default_undefined_handler:
-        // First adjust LR for two purposes: Passing the faulting instruction to the C handler,
-        // and to return to the failing instruction after the C handler returns.
-        // Load processor status
-        mrs      r4, cpsr
-        // Occurred in Thumb state?
-        tst      r4, {t_bit}
-        // If not in Thumb mode, branch to not_thumb
-        beq     not_thumb
-        subs    lr, lr, #2
-        b       done
-not_thumb:
-        // Subtract 4 from LR (ARM mode)
-	      subs	lr, lr, #4
-done:
         // state save from compiled code
         srsfd   sp!, {und_mode}
     "#,
     save_context!(),
     r#"
+        // First adjust LR for two purposes: Passing the faulting instruction to the C handler,
+        // and to return to the failing instruction after the C handler returns.
+        // Load processor status for the calling code
+        mrs      r4, spsr
+        // Was the code that triggered the exception in Thumb state?
+        tst      r4, {t_bit}
+        // Subtract 2 in Thumb Mode, 4 in Arm Mode - see p.1206 of the ARMv7-A architecture manual.
+        ite     eq
+        subeq   lr, lr, #4
+        subne   lr, lr, #2
         // Pass the faulting instruction address to the handler.
-        mov r0, lr
+        mov     r0, lr
         // call C handler
         bl      _undefined_handler
     "#,
@@ -461,9 +457,9 @@ done:
     .size _asm_default_undefined_handler, . - _asm_default_undefined_handler
 
 
-    // Called from the vector table when we have an undefined exception.
+    // Called from the vector table when we have a prefetch exception.
     // Saves state and calls a C-compatible handler like
-    // `extern "C" fn _prefetch_handler();`
+    // `extern "C" fn _prefetch_handler(addr: usize);`
     .global _asm_default_prefetch_handler
     .type _asm_default_prefetch_handler, %function
     _asm_default_prefetch_handler:
@@ -488,7 +484,7 @@ done:
 
     // Called from the vector table when we have an undefined exception.
     // Saves state and calls a C-compatible handler like
-    // `extern "C" fn _abort_handler();`
+    // `extern "C" fn _abort_handler(addr: usize);`
     .global _asm_default_abort_handler
     .type _asm_default_abort_handler, %function
     _asm_default_abort_handler:
@@ -500,7 +496,7 @@ done:
     save_context!(),
     r#"
         // Pass the faulting instruction address to the handler.
-        mov r0, lr
+        mov     r0, lr
         // call C handler
         bl      _abort_handler
     "#,

cortex-r-rt/src/lib.rs

@@ -74,7 +74,7 @@
 //!
 //!   ```rust
 //!   #[unsafe(no_mangle)]
-//!   extern "C" fn _undefined_handler(faulting_instruction: u32);
+//!   extern "C" fn _undefined_handler(addr: usize);
 //!   ```
 //!
 //! * `_abort_handler` - an `extern "C"` function to call when a Data Abort Exception
@@ -86,7 +86,7 @@
 //!
 //!   ```rust
 //!   #[unsafe(no_mangle)]
-//!   extern "C" fn _abort_handler(faulting_instruction: u32);
+//!   extern "C" fn _abort_handler(addr: usize);
 //!   ```
 //!
 //! * `_prefetch_handler` - an `extern "C"` function to call when a Prefetch Abort Exception
@@ -98,7 +98,7 @@
 //!
 //!   ```rust
 //!   #[unsafe(no_mangle)]
-//!   extern "C" fn _prefetch_handler(faulting_instruction: u32);
+//!   extern "C" fn _prefetch_handler(addr: usize);
 //!   ```
 //!
 //! ### ASM functions
@@ -113,7 +113,7 @@
 //! * `_asm_undefined_handler` - a naked function to call when an Undefined
 //!   Exception occurs. Our linker script PROVIDEs a default function at
 //!   `_asm_default_undefined_handler` but you can override it.
-//! * `_asm_prefetch_handler` - a naked function to call when an Prefetch
+//! * `_asm_prefetch_handler` - a naked function to call when a Prefetch
 //!   Exception occurs. Our linker script PROVIDEs a default function at
 //!   `_asm_default_prefetch_handler` but you can override it. The provided default
 //!   handler will perform an exception return to the faulting address.
@@ -313,7 +313,7 @@ core::arch::global_asm!(
 
     // Called from the vector table when we have an software interrupt.
     // Saves state and calls a C-compatible handler like
-    // `extern "C" fn svc_handler(svc: u32, context: *const u32);`
+    // `extern "C" fn svc_handler(svc: u32);`
     .global _asm_svc_handler
     .type _asm_svc_handler, %function
     _asm_svc_handler:
@@ -335,6 +335,7 @@ core::arch::global_asm!(
         rfefd   sp!
     .size _asm_svc_handler, . - _asm_svc_handler
 
+
     // Called from the vector table when we have an interrupt.
     // Saves state and calls a C-compatible handler like
     // `extern "C" fn irq_handler();`
@@ -357,29 +358,25 @@ core::arch::global_asm!(
 
     // Called from the vector table when we have an undefined exception.
     // Saves state and calls a C-compatible handler like
-    // `extern "C" fn _undefined_handler();`
+    // `extern "C" fn _undefined_handler(addr: usize);`
     .global _asm_default_undefined_handler
     .type _asm_default_undefined_handler, %function
     _asm_default_undefined_handler:
-        // First adjust LR for two purposes: Passing the faulting instruction to the C handler,
-        // and to return to the failing instruction after the C handler returns.
-        // Load processor status
-        mrs      r4, cpsr
-        // Occurred in Thumb state?
-        tst      r4, {t_bit}
-        // If not in Thumb mode, branch to not_thumb
-        beq     not_thumb
-        subs	lr, lr, #2
-        b       done
-not_thumb:
-        // Subtract 4 from LR (ARM mode)
-        subs    lr, lr, #4
-done:
         // state save from compiled code
         srsfd   sp!, {und_mode}
     "#,
     save_context!(),
     r#"
+        // First adjust LR for two purposes: Passing the faulting instruction to the C handler,
+        // and to return to the failing instruction after the C handler returns.
+        // Load processor status for the calling code
+        mrs      r4, spsr
+        // Was the code that triggered the exception in Thumb state?
+        tst      r4, {t_bit}
+        // Subtract 2 in Thumb Mode, 4 in Arm Mode - see p.1206 of the ARMv7-A architecture manual.
+        ite     eq
+        subeq   lr, lr, #4
+        subne   lr, lr, #2
         // Pass the faulting instruction address to the handler.
         mov     r0, lr
         // call C handler
@@ -392,21 +389,21 @@ done:
     .size _asm_default_undefined_handler, . - _asm_default_undefined_handler
 
 
-    // Called from the vector table when we have an undefined exception.
+    // Called from the vector table when we have a prefetch exception.
     // Saves state and calls a C-compatible handler like
-    // `extern "C" fn _prefetch_handler();`
+    // `extern "C" fn _prefetch_handler(addr: usize);`
     .global _asm_default_prefetch_handler
     .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
+        subs    lr, lr, #4
         // state save from compiled code
         srsfd   sp!, {abt_mode}
     "#,
     save_context!(),
     r#"
         // Pass the faulting instruction address to the handler.
-        mov r0, lr
+        mov     r0, lr
         // call C handler
         bl      _prefetch_handler
     "#,
@@ -419,12 +416,12 @@ done:
 
     // Called from the vector table when we have an undefined exception.
     // Saves state and calls a C-compatible handler like
-    // `extern "C" fn _abort_handler();`
+    // `extern "C" fn _abort_handler(addr: usize);`
     .global _asm_default_abort_handler
     .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
+        subs    lr, lr, #8
         // state save from compiled code
         srsfd   sp!, {abt_mode}
     "#,

examples/versatileab/reference/prefetch-exception-armv7a-none-eabi.out

@@ -1,3 +1,3 @@
 Hello, this is an undefined exception example
-undefined exception occurred
-undefined exception occurred
+caught undef_from_t32
+caught undef_from_a32

examples/versatileab/reference/prefetch-exception-armv7r-none-eabihf.out

@@ -1,3 +1,3 @@
 Hello, this is an undefined exception example
-undefined exception occurred
-undefined exception occurred
+caught undef_from_t32
+caught undef_from_a32

examples/versatileab/reference/undef-exception-armv7a-none-eabi.out

@@ -56,17 +56,17 @@ fn disable_alignment_check() {
 }
 
 #[unsafe(no_mangle)]
-unsafe extern "C" fn _undefined_handler(_faulting_instruction: u32) {
+unsafe extern "C" fn _undefined_handler(_addr: u32) {
     panic!("unexpected undefined exception");
 }
 
 #[unsafe(no_mangle)]
-unsafe extern "C" fn _prefetch_handler(_faulting_instruction: u32) {
+unsafe extern "C" fn _prefetch_handler(_addr: u32) {
     panic!("unexpected prefetch exception");
 }
 
 #[unsafe(no_mangle)]
-unsafe extern "C" fn _abort_handler(_faulting_instruction: u32) {
+unsafe extern "C" fn _abort_handler(_addr: u32) {
     println!("data abort occurred");
     let dfsr = Dfsr::read();
     println!("DFSR (Fault Status Register): {:?}", dfsr);

examples/versatileab/reference/undef-exception-armv7r-none-eabihf.out

@@ -3,13 +3,17 @@
 #![no_std]
 #![no_main]
 
-use core::sync::atomic::AtomicU32;
-
 // pull in our start-up code
 use versatileab as _;
 
 use semihosting::println;
 
+// These functions are written in assembly
+extern "C" {
+    fn undef_from_a32();
+    fn undef_from_t32();
+}
+
 /// The entry-point to the Rust application.
 ///
 /// It is called by the start-up.
@@ -18,25 +22,55 @@ pub extern "C" fn kmain() -> ! {
     main();
 }
 
-static COUNTER: AtomicU32 = AtomicU32::new(0);
-
 /// The main function of our Rust application.
 #[export_name = "main"]
 fn main() -> ! {
     println!("Hello, this is an undefined exception example");
     unsafe {
-        core::arch::asm!("udf #0");
+        // trigger an undefined exception, from T32 (Thumb) mode
+        undef_from_t32();
+        // trigger an undefined exception, from A32 (Arm) mode
+        undef_from_a32();
     }
-    unreachable!("should never be here!");
+
+    semihosting::process::exit(0);
 }
 
+core::arch::global_asm!(
+    r#"
+    // fn undef_from_a32();
+    .arm
+    .global undef_from_a32
+    .type undef_from_a32, %function
+    undef_from_a32:
+        udf     #0
+        bx      lr
+    .size undef_from_a32, . - undef_from_a32
+
+    // fn undef_from_t32();
+    .thumb
+    .global undef_from_t32
+    .type undef_from_t32, %function
+    undef_from_t32:
+        udf     #0
+        bx      lr
+    .size undef_from_t32, . - undef_from_t32
+"#
+);
+
 #[no_mangle]
-unsafe extern "C" fn _undefined_handler(_faulting_instruction: u32) {
-    println!("undefined exception occurred");
-    // For the first iteration, we do a regular exception return, which should
-    // trigger the exception again.
-    let counter_val = COUNTER.fetch_add(1, core::sync::atomic::Ordering::Relaxed) + 1;
-    if counter_val == 2 {
-        semihosting::process::exit(0);
+unsafe extern "C" fn _undefined_handler(addr: usize) {
+    if addr == undef_from_a32 as usize {
+        println!("caught undef_from_a32");
+    } else if (addr + 1) == undef_from_t32 as usize {
+        // note that thumb functions have their LSB set, despite always being a
+        // multiple of two - that's how the CPU knows they are written in T32
+        // machine code.
+        println!("caught undef_from_t32");
+    } else {
+        println!(
+            "Bad fault address {:08x} is not {:08x} or {:08x}",
+            addr, undef_from_a32 as usize, undef_from_t32 as usize
+        );
     }
 }