fun_getProperty needs a method barrier

RESOLVED FIXED in mozilla13

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: luke, Assigned: luke)

Tracking

unspecified
mozilla13
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

5 years ago
When fun_getProperty searches the call stack for a matching callee (matching 'f' in f.arguments), it needs to use the method barrier.  Otherwise, it may miss the innermost frame.  This was the *real* cause of bug 721322.  The patch in bug 721322, by noting arguments usage, I think just turned off the method optimization.  Most of that patch can actually be removed: read-only access to the arguments object via f.arguments may happen at any time and will faithfully return the correct current argument value.
(Assignee)

Comment 1

5 years ago
Created attachment 594050 [details] [diff] [review]
add the method guard
Assignee: general → luke
Status: NEW → ASSIGNED
Attachment #594050 - Flags: review?(jorendorff)
(Assignee)

Comment 2

5 years ago
Created attachment 594051 [details] [diff] [review]
remove noteArgumentsPropertyAccess

... because it isn't needed.  I verified that the tests added by bug 721322 fail with this patch applied (but not the fix patch) and are fixed when the fix patch is applied.
Attachment #594051 - Flags: review?(jorendorff)
Comment on attachment 594050 [details] [diff] [review]
add the method guard

Certainly this seems like a strict improvement...
Attachment #594050 - Flags: review?(jorendorff) → review+
Comment on attachment 594051 [details] [diff] [review]
remove noteArgumentsPropertyAccess

...and OK, this is fine.  You could back out more of the patch in bug 721322: it contains some renaming that you haven't reverted.  Your call.
Attachment #594051 - Flags: review?(jorendorff) → review+
(Assignee)

Comment 5

5 years ago
I left the renaming b/c it will be removed with a later patch.  (We already do a strictly-more powerful analysis to determine when arguments are used for optimization purposes; with that combined with eager arguments creation, strict arguments should just fall out.)
(Assignee)

Comment 6

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/06c253791ae0
https://hg.mozilla.org/integration/mozilla-inbound/rev/dd2e84f637f2
Target Milestone: --- → mozilla13
https://hg.mozilla.org/mozilla-central/rev/06c253791ae0
https://hg.mozilla.org/mozilla-central/rev/dd2e84f637f2
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.