IonMonkey: Reference error in test case due to wrong scope chain

RESOLVED FIXED in mozilla12

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: dvander, Assigned: bhackett)

Tracking

unspecified
mozilla12
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
We are currently failing jit-test/tests/basic/bug532823.js. It looks like NameOperation reaches for ContextStack::currentScriptedScopeChain() which is not up to date in Ion code.

Not sure what the best fix is. Ideally, we'd pass our current scopeChain into CallGetName.
(Assignee)

Comment 1

5 years ago
What does the stack for NameOperation look like?  NAME calls from jitcode should go through js::GetScopeName which is passed the scope chain as a parameter and does not call NameOperation.
(Reporter)

Comment 2

5 years ago
4983	js::GetScopeName(JSContext *cx, JSObject *obj, PropertyName *name, Value *vp)
4984	{
4985	    JSObject *obj2;
4986	    JSProperty *prop;
4987	    if (!FindPropertyHelper(cx, name, true, false, &obj, &obj2, &prop))
4988	        return false;

The problem is that FindPropertyHelper doesn't actually use |obj| as an inparam, it's just an outparam:

    scopeChain = cx->stack.currentScriptedScopeChain();
(Assignee)

Comment 3

5 years ago
Oops, FindPropertyHelper should be changed to take the scope chain as an input.  I guess it still needs the two outparams though, for the object on the scope chain and the actual holder (want to kill these opaque obj2 vars...).
(Reporter)

Updated

5 years ago
Blocks: 677337
No longer blocks: 677377
(Assignee)

Comment 4

5 years ago
Created attachment 589363 [details] [diff] [review]
patch (ebc05c90d282)

Trunk patch to not get the scope chain from cx->stack.  There are a couple exceptions where cx->currentScriptedScopeChain are still used --- XMLNAME and friends (these will run in the interp) and Object.watch, which does tests on the principals for some reason.  The latter should be fixed in a followup (not involved with bug 701966).
Assignee: general → bhackett1024
Attachment #589363 - Flags: review?(dvander)
(Reporter)

Updated

5 years ago
Attachment #589363 - Flags: review?(dvander) → review+
(Assignee)

Comment 5

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