Skip to content

fix: align .text section start to 8 bytes - #614

Merged
ArthurGibert merged 3 commits into
ATOVproject:mainfrom
kosmar:fix/text-section-alignment
Aug 14, 2026
Merged

fix: align .text section start to 8 bytes#614
ArthurGibert merged 3 commits into
ATOVproject:mainfrom
kosmar:fix/text-section-alignment

Conversation

@kosmar

@kosmar kosmar commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Summary

  • _stext = ADDR(.start_block) + SIZEOF(.start_block) places .text at 0x1000013c, which is not a multiple of the section’s current 8-byte alignment requirement. rust-lld has been warning about this on every build (“address of section .text is not a multiple of alignment”).
  • Fix: ALIGN(..., 8) pads 4 bytes after .start_block so .text starts at 0x10000140 and satisfies the ELF output-section alignment invariant.
  • Note: this corrects the aggregate section-header alignment; it does not by itself prove that the earlier flaky app-task spawn crashes were caused by that misalignment (shifting code addresses can also mask a separate layout-sensitive bug).

Found while debugging dense preset pushes with the preset editor (#602). Host-side pacing there reduces USB pressure; this change clears the linker alignment warning and keeps .text properly aligned.

Test plan

  • Build firmware: rust-lld alignment warning is gone
  • Flash to hardware and push a dense layout (10+ apps incl. multiple instances of the same app) via the configurator

@chmanie

chmanie commented Aug 7, 2026

Copy link
Copy Markdown
Member

The linker-script change itself looks correct: ALIGN(..., 8) moves .text from 0x1000013c to 0x10000140 and satisfies its current 8-byte output-section alignment requirement. One requested wording change: please remove or soften the claim in memory.x and the PR description that this alignment issue caused the app-task spawn crashes. The original linker output still aligned individual input sections as needed; this patch fixes the aggregate ELF section-header invariant, but shifting code addresses can also mask a separate layout-sensitive bug. Suggested comment:

/* Move .text after the boot info and satisfy its current 8-byte
 * output-section alignment requirement. */
_stext = ALIGN(ADDR(.start_block) + SIZEOF(.start_block), 8);

With that wording adjustment, I have no issue with the alignment implementation.

@chmanie

chmanie commented Aug 7, 2026

Copy link
Copy Markdown
Member

Thank you for the fix @kosmar! If you allow edits by maintainers for this PR, we can also take care of rebasing and testing this properly.

@kosmar

kosmar commented Aug 8, 2026

Copy link
Copy Markdown
Contributor Author

@chmanie Thanks — updated the memory.x comment and softened the PR description accordingly. Maintainer edits are enabled, so feel free to rebase and test whenever you have a chance.

kosmar and others added 2 commits August 8, 2026 14:17
Co-authored-by: Cursor <cursoragent@cursor.com>
Authored by an AI coding agent on behalf of kosmar.

Co-authored-by: Cursor <cursoragent@cursor.com>
@chmanie
chmanie force-pushed the fix/text-section-alignment branch from 39d6682 to 0d93dd1 Compare August 8, 2026 12:17

@ArthurGibert ArthurGibert left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed and empirically verified. Built main pre-fix and reproduced the exact rust-lld alignment warning this PR fixes; built this branch and confirmed the warning is gone, .text lands 8-aligned (confirmed via readelf), and the resulting release ELF still converts to a valid UF2. No correctness issues.

One minor, non-blocking note: the ALIGN(..., 8) value is hardcoded with nothing asserting it matches .text's actual required alignment. If a future toolchain/dependency bump raises that requirement, the same warning would silently reappear — it's a linker warning, not a rustc one, so it isn't caught by -D warnings or any CI gate. Not worth blocking on.

@ArthurGibert

Copy link
Copy Markdown
Member

Approved on my side. @chmanie let me know

@chmanie

chmanie commented Aug 14, 2026

Copy link
Copy Markdown
Member

@ArthurGibert yeah fine by me

@ArthurGibert
ArthurGibert merged commit 2b8b1e3 into ATOVproject:main Aug 14, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants