raspberrypi / pico-sdk

BSD 3-Clause "New" or "Revised" License
3.26k stars 838 forks source link

Fix a bug with the enter/exit critical functions missing instruction barrier #1643

Closed simonbeaudoin0935 closed 1 month ago

simonbeaudoin0935 commented 4 months ago
Fixes #1644

The save_and_disable_interrupts function was buggy for two reasons. The first one, it is declared
as a static inline function; once the function's content is inlined, the compiler is free to
optimize/reorder the content as it wishes with surrounding instructions. This, combined with the
fact that the inline assembly "cpsid i" does not contain the trailing `: \"memory\" ` that acts
like an implicit instruction barrier, this allows the compiler to reorder the instructions
that disable the interrupts with whatever is arround it in the caller function.

Now, the fix is to use the already existing CMSIS functions __disable_irq() and __enable_irq()
instead of duplicating the inline assembly. The CMSIS functions DO contain the `: \"memory\" `
statements that will instruct the compiler it cannot do assumptions about the memory after
the inline assembly (aka, instruction barrier). "
error: pathspec 'i does not contain the trailing  that acts
like an implicit instruction barrier, this allows the compiler to reorder the instructions
that disable the interrupts with whatever is arround it in the caller function.

Now, the fix is to use the already existing CMSIS functions __disable_irq() and __enable_irq()
instead of duplicating the inline assembly. The CMSIS functions DO contain the
statements that will instruct the compiler it cannot do assumptions about the memory after
the inline assembly (aka, instruction barrier). '
lurch commented 4 months ago

There appears to be a large chunk of duplicated-text in your commit message?

simonbeaudoin0935 commented 4 months ago

There appears to be a large chunk of duplicated-text in your commit message?

There appears to be a large chunk of duplicated-text in your commit message?

Hmm, indeed, let me fix that

kilograham commented 1 month ago

Fixes #1644

kilograham commented 1 month ago

will fix this by adding memory