fix(gum): create the VFPU context in whichever gum function runs first - #207
Open
ItsNoHax wants to merge 1 commit into
Open
fix(gum): create the VFPU context in whichever gum function runs first#207ItsNoHax wants to merge 1 commit into
ItsNoHax wants to merge 1 commit into
Conversation
Only `sceGumLoadIdentity` and `sceGumLoadMatrix` created `VFPU_CONTEXT`. Every
other gum function went through `get_context_unchecked`, which resolved a `None`
context with `core::intrinsics::unreachable`. Opening with any of them -- most
naturally `sceGumMatrixMode`, which is the first call in most setup code -- is
therefore undefined behaviour, and in practice traps:
E CPU: CPU exception: break instruction hit at 0881ad9c
psp::sys::gum::get_context_unchecked
sceGumMatrixMode
Replace the helper with `get_context`, which creates the context on demand, and
use it everywhere including the two functions that previously did this inline.
The context is a zeroed matrix set with no saved registers, so creating it from
any entry point is the same work `sceGumLoadIdentity` was already doing.
Adds a regression test to ci/tests that opens with `sceGumMatrixMode` and reads
the resulting matrix back. It has to run first, since only the first gum call in
a process is cold. Against master the run stops after STARTING_TESTS with no
final token; with this change the suite ends FINAL_SUCCESS.
Fixes overdrivenpotato#189.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #189.
The bug
VFPU_CONTEXTis created bysceGumLoadIdentityandsceGumLoadMatrixonly. Every other gumfunction reaches it through
get_context_unchecked, which resolves aNonecontext withcore::intrinsics::unreachable. Opening with any of them is undefined behaviour, and on adebug build it traps:
That is the report's
sceGumMatrixModefailure exactly. It is easy to hit becausesceGumMatrixModeis what most setup code calls first — as in the issue's tutorial, which selectsa matrix mode before loading anything into it.
The fix
get_context_uncheckedbecomesget_context, which creates the context on demand, and the twofunctions that were doing that inline now go through it too. Creating a
Contextis a zeroedmatrix set with an empty
savedmask, so doing it from any entry point is the same worksceGumLoadIdentityalready did — the choice of which function got to do it was arbitrary.Tests
One case in
ci/tests: open withsceGumMatrixModeon a cold context, then build a translationand read it back with
sceGumStoreMatrixto show the context it created is usable. No rendering,so it needs no GU.
Only the first gum call in a process is cold, so there is one case to be had and it has to run
before the rest of the suite — hence its slot at the head of the list in
main.rs.Before (master, a8be764) the run stops at the cold call and never reaches a final token, so
run-tests.shfails on the missingFINAL_SUCCESS:After:
Run with
PPSSPPHeadless ... --timeout=40 -r .asci/concourse/run-tests.shdoes. The rest ofthe suite is unaffected and
cargo fmtis clean for bothpspandci/tests.Note
This is independent of #206, which is a separate bug in
sceGumPushMatrix. Both add a test module,so if one lands first the other needs a trivial rebase on the
modand array lines inci/tests/src/main.rs. The test here deliberately avoids push/pop so it passes on master's stackbehaviour.