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 cache_adjust_FileAlignment to work with files not aligned to 0x200 #397

Merged
merged 1 commit into from
Apr 8, 2024

Conversation

asivery
Copy link
Contributor

@asivery asivery commented Mar 19, 2024

Hello,
Please let me know if this isn't a bug, but the library definitely doesn't align PE files aligned to 0x1000 correctly, and this did fix that.

Best regards.

@erocarrera erocarrera self-assigned this Apr 8, 2024
@erocarrera erocarrera merged commit ceab92e into erocarrera:master Apr 8, 2024
@erocarrera erocarrera added the bug label Apr 8, 2024
@erocarrera erocarrera removed the bug label Apr 18, 2024
@erocarrera
Copy link
Owner

This did send me down quite a rabbit hole. It turns out that, for all I can see, Windows ignores the header's FIleAlignment value. The Windows loader aligns the PointerToRawData to 512 bytes (the "sector" size) when mapping the sections (512 often turns to be the default value of FileAlignment).
But that's the only alignment that seems to be applied. I would be curious if anyone can find a working counter-example.
I will revert the patch.

erocarrera added a commit that referenced this pull request Apr 19, 2024
This reverts commit ceab92e, reversing
changes made to 75f0964.
erocarrera added a commit that referenced this pull request Apr 19, 2024
PR #397 led me to revisit the Window's loader behavior. It appears to ignore FileAlignment and always round the PointerToRawData to the sector size (512 bytes).
@asivery
Copy link
Contributor Author

asivery commented Apr 26, 2024

@erocarrera I understand what you mean. If you wish, I can provide you a copy of the file in question.

@erocarrera
Copy link
Owner

Hi @asivery , yes please, that'd be great!

@asivery
Copy link
Contributor Author

asivery commented May 19, 2024

@erocarrera Sorry for the delay, here you go: salwrap.zip

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