avm2: Fix Vector.<Boolean> default value in non-script-init contexts#23527
Open
chabeck1 wants to merge 1 commit intoruffle-rs:masterfrom
Open
avm2: Fix Vector.<Boolean> default value in non-script-init contexts#23527chabeck1 wants to merge 1 commit intoruffle-rs:masterfrom
chabeck1 wants to merge 1 commit intoruffle-rs:masterfrom
Conversation
Makes VectorStorage::default() for Boolean walk the AVM2 call stack to distinguish script-init callers (return null) from other callers (return false on SWF >= 14, null below). Matches Flash Player behavior for the non-script-init case while preserving the existing vector_holes semantics. Adds two regression tests covering SWF versions 14 and 13. Ground truth captured from Adobe's debug Flash Player. Fixes ruffle-rs#23317.
Lord-McSweeney
requested changes
Apr 24, 2026
Collaborator
There was a problem hiding this comment.
This fix is incorrect. The Vector is not filled as false or null depending on whether the activation is in interpreter mode or not; rather, the value is read as false or null depending on whether the activation is in interpreter mode or not. A boolean Vector is always filled with null when initialized.
Collaborator
|
Was an LLM used to write any of this code? |
Author
|
Yes, mostly LLM-generated. Sorry for not flagging it upfront. Your review also makes clear I was on the wrong code path (fill vs read). Close or iterate, whichever you prefer. |
Collaborator
|
You can close this PR if you'd like, but if you want to implement the correct fix it's fine to do so on this PR |
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.
Summary
Fixes #23317.
In Flash Player on SWF version >= 14,
new Vector.<Boolean>(n, true)fills the new slots withfalsewhen called from inside a class method or constructor, but withnullwhen called from a script initializer. Ruffle was returningnullin both cases. This PR makes the non-script-init case match Flash on SWF >= 14 while leaving SWF <= 13 and the script-init case unchanged.Approach
VectorStorage::default()forBooleannow delegates to abool_default()helper incore/src/avm2/vector.rsthat walks the AVM2 call stack from the top, finds the first bytecode method, and checks whether it is the instance initializer of a script'sglobalclass (Class::is_script_traits()). If so, the default isValue::Null(preserving the existingvector_holesbehavior). Otherwise it isValue::Bool(false)for SWF >= 14 andValue::Nullbelow.To enable the call-stack walk inside
VectorStorage,VectorStorage::newnow takes&mut Activation; the change is threaded through every call site (amf.rs,object/vector_object.rs,globals/vector.rs, twoglobals/flash/*files). A smalliter_top_down()helper was added toCallStack.Acknowledgement of maintainer feedback
@Lord-McSweeney and @kjarosh noted on the issue that the Flash behavior here is a JIT specialization artifact, not a Vector semantic rule, and that any fix without a JIT would likely cost
Vectorconstruction performance. I understand this PR takes the "encode the JIT behavior as a runtime check" approach, which the issue thread explicitly cautioned against. I'm opening it anyway so the approach is on the record and reviewable; I fully understand if the preferred resolution is to close without merging. If there are changes that would make a variant of this acceptable (e.g. restricting the call-stack walk to SWF >= 14 paths only, or gating behind a config flag), I'm happy to iterate.Tests
tests/tests/swfs/avm2/vector_boolean_default_constructor/-- SWF v14, the bug case. Expected outputnullthenfalsecaptured from Adobe debug Flash Player.tests/tests/swfs/avm2/vector_boolean_default_constructor_v13/-- SWF v13 sanity check, expected outputnullthennull. Pins the pre-existing behavior below the version threshold.4016 passed; 0 failed. Pre-existingvector_holestest continues to pass.Out of scope
The Haxe-generated static-method case from the issue (where Flash returns
falseregardless of SWF version) is not handled here; the discriminator appears to be Haxe's bytecode pattern rather than the Vector default rule, which is a separate investigation. Noted in the issue thread.