getApiVersionFromCallStack ignores EnterCodeContext

NEW
Assigned to

Status

Tamarin
Virtual Machine
P3
normal
7 years ago
7 years ago

People

(Reporter: Stan Switzer, Assigned: Stan Switzer)

Tracking

unspecified
Q1 12 - Brannan
Bug Flags:
in-testsuite ?
flashplayer-injection +
flashplayer-qrb +
flashplayer-bug +

Details

Attachments

(1 attachment)

1.56 KB, patch
Rick Reitmaier
: review+
Edwin Smith
: superreview+
Details | Diff | Splinter Review
(Assignee)

Description

7 years ago
The function AvmCore::getApiVersionFromCallStack is supposed to determine the API version based on the invoking user by walking the MethodFrame stack, but it incorrectly ignores CodeContext entries.

The MethodFrame stack contains both MethodEnvs and raw CodeContexts that represent points where the VM was entered on behalf of user code (say, an event dispatcher).

While crawling the stack, builtin methods are ignored since the function is trying to find the nearest invoking user code. Two problems arise: first there may be no user code on the stack (in some cases builtin code is registered as an event handler), and any MethodEnvs found after a raw CodeContext represent an entirely different entry into the VM so they are beside the point.
(Assignee)

Comment 1

7 years ago
Created attachment 561812 [details] [diff] [review]
Proposed fix

This patch stops looking for non-builtin MethodEnvs at the first raw CodeContext and then determines API version corresponding to that code context. You can find the API version by locating the CodeContext's toplevel's pool's API version.

This fix also addresses the issue alluded to in the FIXME comment.
(Assignee)

Updated

7 years ago
Attachment #561812 - Flags: review?(treilly)
(Assignee)

Updated

7 years ago
Attachment #561812 - Flags: review?(rreitmai)

Comment 2

7 years ago
Comment on attachment 561812 [details] [diff] [review]
Proposed fix

Delicate code I'm not familiar with, deferring to the pros.   Should jodyer review as well?
Attachment #561812 - Flags: review?(rreitmai) → superreview?(edwsmith)

Updated

7 years ago
Attachment #561812 - Flags: review?(treilly) → review?(rreitmai)

Comment 3

7 years ago
Comment on attachment 561812 [details] [diff] [review]
Proposed fix

Seem ok.  But as we know any change related to this area requires lots and lots of testing.
Attachment #561812 - Flags: review?(rreitmai) → review+

Updated

7 years ago
Assignee: nobody → stan
Flags: flashplayer-qrb+
Flags: flashplayer-injection+
Flags: flashplayer-bug+
Priority: -- → P3
Target Milestone: --- → Q1 12 - Brannan

Comment 4

7 years ago
changeset: 6706:1018a267e123
user:      Tommy Reilly <treilly@adobe.com>
summary:   Bug 688552 - getApiVersionFromCallStack ignores EnterCodeContext (author=sswitzer,r=rreitmai,sr-pending=edwsmith)

http://hg.mozilla.org/tamarin-redux/rev/1018a267e123

Updated

7 years ago
Attachment #561812 - Flags: superreview?(edwsmith) → superreview+

Comment 5

7 years ago
desperately needs a test case.
Flags: in-testsuite?
You need to log in before you can comment on or make changes to this bug.