raspberrypi / pico-examples

BSD 3-Clause "New" or "Revised" License
2.85k stars 820 forks source link

SIO View While Debugging Corrupts Spinlocks #363

Open mookiedog opened 1 year ago

mookiedog commented 1 year ago

Background

The spinlock registers have action-on-read semantics: reading a spinlock hardware register allocates the spinlock if it is not already allocated.

Issue

If the SIO register group is open and visible during a VS Code debug session, when the debugger reads the state of the spinlock registers to display their contents in the debug register view, it would appear to have the unfortunate side effect of permanently allocating every unallocated spinlock.

To Replicate

Any executable will do since the effects can be demonstrated using the routine runtime_init() found in the file rp2_common/pico_runtime/runtime.c which would typically be part of every Pico build.

Make sure that the SIO register group is open for display in VS Code in the 'Cortex Peripherals' window. Set a breakpoint on the irq_init_prioities() line in the startup file runtime.c:

    spin_locks_reset();
    irq_init_priorities();   <-- set breakpoint on this sourceline
    alarm_pool_init_default();

The point of setting the breakpoint at this location is that we know for a fact that every single spinlock must be unallocated when the breakpoint hits.

Since this code runs before main(), rerun the program by clicking the 'reset device' button. It will hit the breakpoint immediately. At the moment the breakpoint hits, examining the SPINLOCK_ST register in the SIO register view will display all spinlocks as being 'unallocated' (0), which is correct:

SPINLOCK_ST@0xd000005c | (Read Only) | 0x00000000

But if you execute anything else like F10 (Step Over) or F11 (Step Into), when the VS Code SIO register display gets refreshed, it will now show every single spinlock as 'allocated' (1).

SPINLOCK_ST@0xd000005c  (Read Only) 0xffffffff

Since the spinlocks are apparently getting allocated out of band due to debugger reads, all subsequent spinlock allocation requests in the program being debugged will fail. This means that it is impossible to debug anything that requires viewing SIO registers without corrupting all the spinlocks.

mookiedog commented 1 year ago

Where would be a better place to file this issue? The SDK?

lurch commented 1 year ago

We've already logged this internally, but unfortunately we've not had time to address it yet. I believe the fix for this would be for the rp2040.svd file to use the readAction property https://arm-software.github.io/CMSIS_5/SVD/html/elem_registers.html#elem_register to indicate that "Debuggers are not expected to read this register location unless explicitly instructed by the user.". However the rp2040.svd is auto-generated, and we'd have to update a chunk of internal tooling to add support for this.

mookiedog commented 1 year ago

Nice! That looks like what is required. I haven't looked at all of the registers in an RP2040, but I would have to bet that there are a number of registers beyond the Spinlocks that might need this same treatment.

pro100andrey commented 10 months ago

Hi @lurch, any updates?

matiasilva commented 9 months ago

We've already logged this internally, but unfortunately we've not had time to address it yet. I believe the fix for this would be for the rp2040.svd file to use the readAction property https://arm-software.github.io/CMSIS_5/SVD/html/elem_registers.html#elem_register to indicate that "Debuggers are not expected to read this register location unless explicitly instructed by the user.". However the rp2040.svd is auto-generated, and we'd have to update a chunk of internal tooling to add support for this.

Internal tooling has been updated, so expect this to be fixed whenever a new release of the docs is made.