Last Comment Bug 387591 - JS_GetScopeChain asserts because of native functions
: JS_GetScopeChain asserts because of native functions
: fixed1.8.1.12
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: x86 Linux
: -- normal (vote)
: ---
Assigned To: Blake Kaplan (:mrbkap)
: Jason Orendorff [:jorendorff]
Depends on:
Blocks: 386695
  Show dependency treegraph
Reported: 2007-07-10 14:10 PDT by Blake Kaplan (:mrbkap)
Modified: 2008-01-13 10:19 PST (History)
2 users (show)
bob: in‑testsuite-
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Proposed fix (1019 bytes, patch)
2007-07-10 14:10 PDT, Blake Kaplan (:mrbkap)
brendan: review+
Details | Diff | Splinter Review
Patch to checkin (1021 bytes, patch)
2007-07-10 14:34 PDT, Blake Kaplan (:mrbkap)
dveditz: approval1.8.1.12+
Details | Diff | Splinter Review

Description Blake Kaplan (:mrbkap) 2007-07-10 14:10:36 PDT
Created attachment 271729 [details] [diff] [review]
Proposed fix

Currently, if a user of the C API uses JS_CallFunction to call a native function, and that native function causes JS_GetScopeChain to be called, we will assert. The problem is that native functions inherit the scope chain of their caller, but if there is no caller, then they don't get a scope chain at all. js_GetScopeChain can't deal with this.

My proposed fix is to use the parent of the native function object if there was no scripted caller so we have *some* scopeChain to return in these cases.
Comment 1 Brendan Eich [:brendan] 2007-07-10 14:24:23 PDT
Why not remove the assertion and let the result be NULL?

Comment 2 Blake Kaplan (:mrbkap) 2007-07-10 14:27:48 PDT
Because that's ambiguous to the caller -- JS_GetScopeChain can fail. Also, I'm calling it so I can create a new (exception) object based on the current scope chain. If JS_GetScopeChain returns null, then my only other option is JS_GetGlobalObject on the current context, which is badly wrong (as you pointed out to me the other day).
Comment 3 Brendan Eich [:brendan] 2007-07-10 14:31:25 PDT
Comment on attachment 271729 [details] [diff] [review]
Proposed fix

Ok then -- boundary condition I should have foreseen.

>         /* If native, use caller varobj and scopeChain for eval. */
>         frame.varobj = fp->varobj;
>         frame.scopeChain = fp->scopeChain;

Blank line here, per prevailing style.

>+        /* But ensure that we have a scope chain. */
>+        if (!frame.scopeChain)
>+            frame.scopeChain = parent;

Otherwise, r=me.

Comment 4 Blake Kaplan (:mrbkap) 2007-07-10 14:34:49 PDT
Created attachment 271737 [details] [diff] [review]
Patch to checkin

I'll check this in when the tree opens.
Comment 5 Blake Kaplan (:mrbkap) 2007-07-10 17:29:37 PDT
Fix checked into trunk.
Comment 6 Blake Kaplan (:mrbkap) 2007-12-24 16:12:41 PST
Comment on attachment 271737 [details] [diff] [review]
Patch to checkin

This applies cleanly to the 1.8 branch.
Comment 7 Daniel Veditz [:dveditz] 2008-01-09 11:42:18 PST
Comment on attachment 271737 [details] [diff] [review]
Patch to checkin

approved for, a=dveditz for release-drivers
Comment 8 Blake Kaplan (:mrbkap) 2008-01-13 10:19:41 PST
Fixed on the 1.8 branch.

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