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

AGOS: Fix compatibility with strict-alignment architectures #2241

Merged
merged 1 commit into from Aug 1, 2020

Conversation

@dwatteau
Copy link
Contributor

dwatteau commented May 11, 2020

Proposed fix for a bug I've reported some years ago:
https://bugs.scummvm.org/ticket/6220

SIMON2-FR would crash really early with a SIGBUS on OpenBSD/loongson, which is a mips64el system with strict alignment constraints.

Aligning to sizeof(byte *) in alignTableMem(), and calling alignTableMem() in loadTextIntoMem() seems to be enough. It just took me a bit of time to test this :)

(Maybe alignTableMem() could be optimised a bit, but I'm not sure it's worth it.)

Tested with SIMON2-FR and SIMON1-FR on OpenBSD/loongson and on macOS Mojave (x86-64).

I don't really know how to test the "running out of memory" possible case that lordhoto reported above, though. Don't know which platform or use case would be more likely to hit it.

Make sure that alignTableMem() always aligns to an 8 byte boundary
on 64-bit architectures where this is the usual size of pointers.

Also make sure that loadTextIntoMem() always calls alignTableMem().

This has been tested on OpenBSD/loongson (a mips64el architecture which requires
strict alignment), with clang++ 8.0.1 and simon2-cd-fr.

Fixes bug #6220.
@digitall
Copy link
Member

digitall commented May 12, 2020

@dwatteau : Thank you for the patch for this bug in the AGOS engine which has been open for quite a while. Hopefully an AGOS engine dev will review this shortly. If not, then I will give your patch a basic test with my copies of AGOS games to see if this seems good...

@sev-
Copy link
Member

sev- commented Aug 1, 2020

Thanks

@sev- sev- merged commit 3c3d35d into scummvm:master Aug 1, 2020
2 checks passed
2 checks passed
Codacy/PR Quality Review Up to standards. A positive pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.