Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix alignment errors in EWRAM_INIT and friends when using u8, u16, etc. #5512

Merged
merged 1 commit into from
Oct 12, 2024

Conversation

aronson
Copy link

@aronson aronson commented Oct 12, 2024

Description

I opened a PR a while back to add EWRAM_INIT support to the project. The linker script modifications I made didn't take into account that users may define data sections that are not a multiple of 4 bytes in size. This leads the DMA transfer at init to copy garbage into the wrong sections, resulting in subtle errors at runtime when the feature is used. I've also aligned IWRAM_DATA to prevent the same error.

This is a one-line change, allow me to explain what's happening:

 .ewram ORIGIN(EWRAM) : AT (__ewram_lma)
     ALIGN(4)
 {
         __ewram_start = .;
         *(.ewram*);
+        . = ALIGN(4);
         __ewram_end = .;
 } > EWRAM

The . represents an address marching ahead in memory in ld scripts as each section is defined/filled. If we added a EWRAM_INIT u8 foo = 1; then this section would resolve *(.ewram*) in a way that marches the pointer forward by 1 byte. The section itself will be aligned to 4 bytes because of the original and first ALIGN(4) directive, but the __ewram_end symbol we export to the initialization assembly to tell it which sections to copy would be 1 byte ahead, not 4. This breaks assumptions in the assembly code and leads it to copy garbage into otherwise necessary sections.

By adding the new line above, we force the pointer to march ahead anywhere from 0-3 bytes to ensure there is sufficient padding for the DMA copy to function.

This should resolve the issue. To test the issue and its resolution one can make a single line change to main.c:

EWRAM_INIT u8 gTestu8 = 4;

Without the fix, a number of issues will the occur, the first being that the default option selected after the title screen on a blank save will be "Options" instead of "New Game".

Images

image

Discord contact info

@luigi___

@aronson aronson force-pushed the fix/linker-alignment branch from 1c5eb70 to 2e3c055 Compare October 12, 2024 14:23
@aronson
Copy link
Author

aronson commented Oct 12, 2024

Added the fix to AGBCC as well with a force push.

@mrgriffin mrgriffin merged commit a01f9b4 into rh-hideout:master Oct 12, 2024
1 check passed
@aronson aronson deleted the fix/linker-alignment branch October 12, 2024 20:01
@AsparagusEduardo AsparagusEduardo added bugfix Bugfixes General Doesn't fit under other labels labels Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Bugfixes General Doesn't fit under other labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants