Skip to content

Add memory clobber to WITH_BACKUP_RAM_RW asm#132

Merged
dciabrin merged 1 commit into
dciabrin:masterfrom
dogruis:nullbios-backup-ram-memory-clobber
Jul 18, 2026
Merged

Add memory clobber to WITH_BACKUP_RAM_RW asm#132
dciabrin merged 1 commit into
dciabrin:masterfrom
dogruis:nullbios-backup-ram-memory-clobber

Conversation

@dogruis

@dogruis dogruis commented May 22, 2026

Copy link
Copy Markdown
Contributor

Without a memory clobber, gcc -O2 may reorder reads/writes of backup-RAM globals across the unlock/lock pair in WITH_BACKUP_RAM_RW, defeating the protection. Adding ::: "memory" to both asm volatile blocks makes them proper compiler barriers without changing emitted code for the asm itself.

Code example:

WITH_BACKUP_RAM_RW({
    high_score = new_score;       // BRAM write
    save_count = save_count + 1;  // BRAM write
});
some_other_global = 0;            // non-BRAM

Without "memory", gcc might rearrange to:

unlock_asm();
some_other_global = 0;            // moved INTO the protected section (harmless but pointless)
high_score = new_score;
lock_asm();
save_count = save_count + 1;      // moved OUT to AFTER the lock — bug!

The save_count write happens while BRAM is re-locked → hardware rejects the write → save data corrupted on the next save cycle.

Without a memory clobber, gcc -O2 may reorder reads/writes of
backup-RAM globals across the unlock/lock pair, defeating the
protection. ::: "memory" makes both __asm__ volatile blocks
act as proper compiler barriers.
@dciabrin

Copy link
Copy Markdown
Owner

LGTM
Thank you

@dciabrin
dciabrin merged commit ca710f9 into dciabrin:master Jul 18, 2026
@dogruis

dogruis commented Jul 18, 2026

Copy link
Copy Markdown
Contributor Author

Oh... That's great I have became a contributor!!!!! This made my day 😄

@dciabrin

Copy link
Copy Markdown
Owner

Oops, that commit broke the build on all arch :)
I should enable the CI runs for pull requests. I'll fix the commit later today

@dciabrin

Copy link
Copy Markdown
Owner

Fixed in 28b5276

pull Bot pushed a commit to sysfce2/ngdevkit that referenced this pull request Jul 18, 2026
Commit ca710f9 added memory fencing hints for the compiler,
but in doing so, this broke the original asm instruction due
to how we did not guard symbol % when referencing %d0

Ref dciabrin#132
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.

2 participants