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

Closed

simonbeaudoin0935 wants to merge 1 commit from master into develop

simonbeaudoin0935 commented

2024-02-20 14:15:33 -06:00

(Migrated from github.com)

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). '

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

2024-02-21 03:15:58 -06:00

(Migrated from github.com)

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

simonbeaudoin0935 commented

2024-02-21 13:46:43 -06:00

(Migrated from github.com)

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

Hmm, indeed, let me fix that

> 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

Rows
Columns

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

Pull request closed