avm2: Check prototype chain in with-scope property resolution#23511
Open
bjornwein wants to merge 2 commits intoruffle-rs:masterfrom
Open
avm2: Check prototype chain in with-scope property resolution#23511bjornwein wants to merge 2 commits intoruffle-rs:masterfrom
bjornwein wants to merge 2 commits intoruffle-rs:masterfrom
Conversation
Collaborator
|
Please add a test, thank you. |
In avmplus, with-scope resolution via hasMultinameProperty checks own properties, vtable traits, AND the prototype chain (delegate). Ruffle's search_scope_stack only checked vtable traits and own properties, missing the prototype chain entirely. This is particularly important for E4X (XML/XMLList) objects where methods like name(), children(), etc. are defined on the prototype in the public namespace, but the vtable only has them in the AS3 namespace (http://adobe.com/AS3/2006/builtin). When compiled code uses these methods inside a 'with' block (pushwith), the compiler generates QName(public, "name") lookups. Without prototype chain checking, findpropstrict falls through to the wrong scope (e.g. finding DisplayObject.name instead of XML.prototype.name), causing Error ruffle-rs#1006. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ac4f802 to
3b1b695
Compare
Author
|
Thanks for looking into it. I apologize for dropping this LLM-generated PR on you. I can confirm that the change solves a real-world problem for me. Unfortunately I don't have the knowledge to evaluate whether this is the best way to solve the specific issue. I (or mostly copilot) have updated the PR with a minimal-ish regression test. The test fails with error code #1065 without the fix. |
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.
Problem
When AS3 code uses XML methods like
name(),children(), etc. inside awithblock (pushwithopcode), the compiler generatesQName(PackageNamespace(""), "name")lookups (public namespace). However, Ruffle'ssearch_scope_stackonly checked:name()in the AS3 namespace (http://adobe.com/AS3/2006/builtin), not the public namespace → no matchhas_own_propertychecks E4X child elements → no matchThis caused
findpropstrictto skip the XML with-scope entirely and find the property on a parent scope instead (e.g.,DisplayObject.namegetter returning a string), leading toError #1006: value is not a functionwhen trying to call the string result.Root Cause
In avmplus,
hasMultinamePropertyfor with-scope resolution checks own properties, traits, and the prototype chain (delegate). Ruffle was missing the prototype chain check. This matters becauseXML.prototype.name(and other XML methods) are defined as public-namespace dynamic properties on the prototype inXML.as(lines 183-190), bridging the AS3→public namespace gap.Fix
Added
value_has_proto_property()tosearch_scope_stackthat walks the prototype chain checking for dynamic properties. This is called for with-scopes after the existing own-property check, matching avmplus behavior.Testing
Tested against a real-world Flex-based SWF (hardware appliance UI) that was completely broken by this bug — the app now loads and functions correctly past login. The
Error #1006inupdateTreeExpansion()is eliminated.Existing tests pass (
cargo test --release --package ruffle_core).