search

adamgreen / mri

MRI - Monitor for Remote Inspection. The gdb compatible debug monitor for Cortex-M devices.
Apache License 2.0
155 stars 57 forks source link

Consider checking base priority in Platform_EnableSingleStep() #14

Closed PetteriAimonen closed 2 years ago

PetteriAimonen commented 2 years ago

I had a silly bug in my platform code:

NVIC_SetPriority(USART0_IRQn, 0);
mriCortexMInit(NULL, 0, LAST_IRQn);

The two lines being in wrong order, USART interrupt ends up being below DebugMon interrupt. This in turn causes very weird behavior as soon as step command is used, because the monitor ends up trying to single-step itself. Funnily enough it sometimes even worked to some extent.

I wonder if it would be worth it to add a check to verify that the currently executing interrupt is at correct priority before trying to step? Getting the interrupt priorities wrong seems like an easy mistake to make, and the consequences are very difficult to figure out. Though I'm not sure what to do if the check fails, best would be to report error to gdb somehow so developer knows to fix their platform code.

adamgreen commented 2 years ago

I see how that could be tricky to figure out. I don't know if adding code to MRI to detect a configuration error is something that we should add, especially with user readable text, as it would grow the monitor code for a scenario that is not directly related to debugging code.

Maybe MRI's dependencies on exception/interrupt priorities should be better documented?

PetteriAimonen commented 2 years ago

I agree with the reasoning.

I made a pull request that adds a comment about this in the init files and the porting document: https://github.com/adamgreen/mri/pull/15

That comment certainly would have been enough to guide me :)

adamgreen commented 2 years ago

Looks good. Thanks so much for the help.

PetteriAimonen commented 2 years ago

Thank you for the wonderful library :)

After I got the initial troubles sorted out it works great for debugging custom firmware on Owon XDM2041. The original designers used the SWD pins for keyboard..

adamgreen commented 2 years ago

Thank you for the wonderful library :)

You're welcome. I am happy to hear that you found it useful and thanks for your recent PRs.

After I got the initial troubles sorted out it works great for debugging custom firmware on Owon XDM2041. The original designers used the SWD pins for keyboard..

Cool! I had to Google that device to see what it was and found a link to your clock crystal replacement project on HaD :) Sounds like an awesome project.