Skip to content

Align SizeOfImage to section boundaries - #5

Merged
Systemcluster merged 2 commits into
Systemcluster:mainfrom
zanieb:zb/fix-size-of-image-alignment
Aug 11, 2026
Merged

Align SizeOfImage to section boundaries#5
Systemcluster merged 2 commits into
Systemcluster:mainfrom
zanieb:zb/fix-size-of-image-alignment

Conversation

@zanieb

Copy link
Copy Markdown
Contributor

The PE specification requires SizeOfImage to describe the loaded image and be a multiple of SectionAlignment. Adding a resource section currently increases it by the section’s file-aligned size, which can leave the image misaligned. Instead, we should calculate SizeOfImage from the highest section’s virtual extent and round it to SectionAlignment.

CopilotAI lite review requested due to automatic review settings August 5, 2026 14:54

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR updates PE image writing logic so SizeOfImage is computed from the highest section’s virtual extent and rounded to SectionAlignment, matching the PE specification and avoiding misaligned image sizes after adding a resource section.

Changes:

  • Recompute SizeOfImage from the max (VirtualAddress + VirtualSize) across sections and align it to SectionAlignment.
  • Add a regression test asserting SizeOfImage is a multiple of SectionAlignment after resource transfer.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

FileDescription
tests/tests.rsAdds an assertion that SizeOfImage remains section-aligned after modifying resources.
src/image.rsReplaces incremental SizeOfImage updates with a recomputed, section-aligned value based on section virtual extents.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadsrc/image.rs
Comment on lines +567 to +572
let size_of_image = section_table
.iter()
.map(|section| section.virtual_address + section.virtual_size)
.max()
.map(|size| aligned_to(size, windows_header.section_alignment()))
.unwrap_or_else(|| windows_header.size_of_image());

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

We can't just use u64, it's a u32 field. I can look at an alternative here though 🤔

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

@Systemcluster

Copy link
Copy Markdown
Owner

Thank you!

@Systemcluster
Systemclusterforce-pushed the zb/fix-size-of-image-alignment branch from afb30b6 to d4ee4c7CompareAugust 10, 2026 16:46
@Systemcluster
Systemcluster merged commit c7ffa0b into Systemcluster:mainAug 11, 2026
11 checks passed
Sign up for freeto 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.

3 participants

@zanieb@Systemcluster