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

DS: Hack to work around ARM/Thumb relocation issues in the SCUMM engine #5197

Merged
merged 1 commit into from Nov 5, 2023

Conversation

ccawley2011
Copy link
Member

Having all libc functions referenced in the main executable works around the issue described in Trac #14554. This allows the SCUMM engine to run correctly, instead of crashing when launching a game.

If this PR is accepted, I plan to create the v2.7.1 release build with this change applied. Hopefully a proper fix will be available before v2.8.0, but I don't know how to do this myself.

@lephilousophe
Copy link
Member

There is the is the --whole-archive option which would include the full libc. This may not be efficient.
Instead of calling the functions, maybe just create something like that?

void *pluginHack[] = {
    atan2,
    modf,
};

I don't see something easy to force include missing symbols in libc.
Maybe we could add a post-build check for platforms using our ELF loader which checks that no symbol is missing from the main binary?

@sev-
Copy link
Member

sev- commented Aug 6, 2023

Instead of calling the functions, maybe just create something like that?

@lephilousophe what is the difference when the function is not called anyway?

@lephilousophe
Copy link
Member

It avoids the compiler to emit a function with a sequence of call instructions.
Plus, it also avoids us to write the calls taking care of the number of arguments and cheating with volatile to fight call optimization.
By creating the array, we force the function inclusion because it's referenced and the result is only a sequence of pointers and they are never optimized.

@sev-
Copy link
Member

sev- commented Aug 28, 2023

Good points. @ccawley2011 what do you think?

@bluegr
Copy link
Member

bluegr commented Oct 8, 2023

It avoids the compiler to emit a function with a sequence of call instructions. Plus, it also avoids us to write the calls taking care of the number of arguments and cheating with volatile to fight call optimization. By creating the array, we force the function inclusion because it's referenced and the result is only a sequence of pointers and they are never optimized.

@lephilousophe wait, but we want to have these functions optimized, right? Is there a benefit in not optimizing them?

@lephilousophe
Copy link
Member

lephilousophe commented Oct 15, 2023

@lephilousophe wait, but we want to have these functions optimized, right? Is there a benefit in not optimizing them?

No, we just want them to be bundled inside the main binary so that plugins can make use of them.
What I meant by optimization, is that the compiler will always try to optimize the pluginHack function to avoid by all means to issue the calls to the functions we want.
That's why @ccawley2011 had to use volatile trick to force the compiler to do the call (and so to include the function in our binary).

@sev-
Copy link
Member

sev- commented Nov 5, 2023

Okay, merging this. Thank you!

@sev- sev- merged commit 3a6566c into scummvm:master Nov 5, 2023
8 checks passed
@ccawley2011 ccawley2011 deleted the ds-thumb-hack branch November 5, 2023 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants