723791 - fun_getProperty needs a method barrier

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Luke Wagner [:luke]]

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.

Comment 1

[Luke Wagner [:luke]]

Attachment: add the method guard

Comment 2

[Luke Wagner [:luke]]

Attachment: 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.

Comment 3

[Jason Orendorff [:jorendorff]]

Comment on attachment 594050 [details] [diff] [review]
add the method guard

Certainly this seems like a strict improvement...

Comment 4

[Jason Orendorff [:jorendorff]]

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.

Comment 5

[Luke Wagner [:luke]]

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.)

Comment 6

[Luke Wagner [:luke]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/06c253791ae0
https://hg.mozilla.org/integration/mozilla-inbound/rev/dd2e84f637f2

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/06c253791ae0
https://hg.mozilla.org/mozilla-central/rev/dd2e84f637f2