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

Enhance HardFault debugging #1232

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

firelizzard18
Copy link
Contributor

@firelizzard18 firelizzard18 commented Jul 11, 2020

This MR updates the HardFault handler, allowing improved debugging for Cortex-M3 and Cortex-M4 based boards, using the fault status registers defined by those cores.

Changes

  • Add cortex-m0.json, cortex-m0p.json, and cortex-m3.json to targets/, and update Cortex-M targets to inherit from these, to allow targeting of M0/M0+/M3/M4 (targets/cortex-m4.json already exists).
  • Move the definition of the system control block registers from arm.go to core-specific files (arm_cortex_mX.go) to allow for core-specific registers.
  • Generalize the Teensy 3.6/NXP MK66F18's HardFault handler to work with any Cortex-M3/M4
  • Capture PSP and MSP instead of just SP, in order to log the correct stack pointer (because SP in the ISR always refers to MSP)
  • Use stackTop-stackSize instead of a hardcoded value (0x20000000) to detect stack overflow (on the Teensy 3.6, RAM starts at 0x1fff0000)

Executable size does not increase greatly for M3/M4 cores, unless const hardFaultDebug bool is set to true. As of cd521eb, sizes for src/examples/blinky1 are:

Model Version Size
Feather M0 Unmodified 23764
Feather M0 New 23852 (+88)
Feather M4 Unmodified 20596
Feather M4 New w/o debug 20728 (+132)
Feather M4 New w/ debug 23456 (+2860)

@firelizzard18 firelizzard18 force-pushed the expanded-fault-handling branch from 2f75daf to cd521eb Compare July 11, 2020 02:24
@firelizzard18
Copy link
Contributor Author

8 bytes of the change are due to Default_Handler:

 <Default_Handler>:
 bf20      	wfe
 e7fd      	b.n	<Default_Handler>
 d4d4      	bmi.n	<tinygo_getSystemStackPointer+0x2>
+d4d4      	bmi.n	<tinygo_getSystemStackPointer+0x4>
+d4d4      	bmi.n	<tinygo_switchToScheduler>
+d4d4      	bmi.n	<tinygo_switchToScheduler+0x2>
+d4d4      	bmi.n	<tinygo_switchToScheduler+0x4>

@firelizzard18 firelizzard18 force-pushed the expanded-fault-handling branch 3 times, most recently from 8db28ce to f6217b8 Compare July 11, 2020 06:45
@deadprogram
Copy link
Member

@firelizzard18 please make sure you rebase against the dev branch, that might get rid of the build errors.

Define targets for Cortex-M0, -M0+, and -M3, in addition to the existing Cortex-M4 target to allow targeting specific architectures.
@firelizzard18 firelizzard18 force-pushed the expanded-fault-handling branch from f6217b8 to 83a501f Compare July 11, 2020 21:19
@firelizzard18
Copy link
Contributor Author

@deadprogram Rebased

@firelizzard18
Copy link
Contributor Author

firelizzard18 commented Jul 12, 2020

On interrupt, the link register is filled with an EXC_RETURN (exception return) value. Bit 2 indicates whether the interrupted code was using the PSP (bit 2 == 1) or the MSP (bit 2 == 0). I attempted to do all of this in assembly, defining handleHardFault(sp *interruptStack, system bool). But I couldn't get that to work, so I did the check in Go instead. I think doing it in assembly would result in a smaller increase in code size (when hardFaultDebug == false). Here is what I think should work, but demonstrably did not:

tst lr, #(1<<2)
itete eq
mrseq r0, PSP
mrsne r0, MSP
moveq r1, #0 ; system == false
movne r1, #1 ; system == true

I have no idea why this does not work but the Go implementation does. I thought maybe I misunderstood the conditionals, so I switched them, but that didn't help. I triggered a stack overflow (via dumb recursive fibonacci) in the scheduler and in main (separately). The Go implementation correctly reports PSP vs MSP; the assembly implementation does not.

@firelizzard18
Copy link
Contributor Author

The following works as expected, but fails to compile for the Feather M0:

//go:export HardFault_Handler
func hardFaultEntry() {
    pspStacked := arm.AsmFull("ands.w {}, lr, #(1<<2)", nil)
    sp := (*interruptStack)(unsafe.Pointer(arm.AsmFull(`
        ite ne
        mrsne {}, PSP
        mrseq {}, MSP
    `, nil)))

    arm.AsmFull(`
        movs r3, #0
        ldr r3, [r3]
        mov sp, r3
    `, nil)

    handleHardFault(sp, pspStacked)
}


func handleHardFault(sp *interruptStack, pspStacked uintptr) {
    // ...
}

The error:

<inline asm>:1:2: error: instruction requires: thumb2
        ands.w r0, lr, #(1<<2)
        ^
LLVM ERROR: Error parsing inline asm

@aykevl
Copy link
Member

aykevl commented Jul 23, 2020

Yes, the Cortex-M0 has implemented far fewer instructions. Many wide instructions (such as ands.w) and most/all conditional instructions (such as mrsne) are not supported.

This is what the error says:

 error: instruction requires: thumb2

Cortex-M0 implements thumb, not thumb2.

@@ -56,195 +19,3 @@ func Asm(asm string)
// You can use {} in the asm string (which expands to a register) to set the
// return value.
func AsmFull(asm string, regs map[string]interface{}) uintptr

// Run the following system call (SVCall) with 0 arguments.
func SVCall0(num uintptr) uintptr
Copy link
Member

Choose a reason for hiding this comment

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

Why did you move all these to arm_cortex.go? At least the svc instruction should be implemented on basically all ARM chips. Why did you split this file in arm.go and arm_cortex.go?

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.

3 participants