Last Comment Bug 717494 - IonMonkey: Reference error in test case due to wrong scope chain
: IonMonkey: Reference error in test case due to wrong scope chain
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla12
Assigned To: Brian Hackett (:bhackett)
:
:
Mentors:
Depends on:
Blocks: 677337 701966
  Show dependency treegraph
 
Reported: 2012-01-11 18:50 PST by David Anderson [:dvander]
Modified: 2012-01-19 02:58 PST (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (ebc05c90d282) (9.95 KB, patch)
2012-01-17 17:04 PST, Brian Hackett (:bhackett)
dvander: review+
Details | Diff | Splinter Review

Description David Anderson [:dvander] 2012-01-11 18:50:41 PST
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.
Comment 1 Brian Hackett (:bhackett) 2012-01-11 19:13:05 PST
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.
Comment 2 David Anderson [:dvander] 2012-01-11 19:22:28 PST
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();
Comment 3 Brian Hackett (:bhackett) 2012-01-11 19:29:08 PST
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...).
Comment 4 Brian Hackett (:bhackett) 2012-01-17 17:04:21 PST
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).
Comment 5 Brian Hackett (:bhackett) 2012-01-18 17:15:36 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/96a9dffede07
Comment 6 Marco Bonardo [::mak] 2012-01-19 02:58:18 PST
https://hg.mozilla.org/mozilla-central/rev/96a9dffede07

Note You need to log in before you can comment on or make changes to this bug.