309752 - JS_EvaluateUCScriptInStackFrame: JS_ASSERT(0x9a < 0) under subscriptloader

Status: VERIFIED DUPLICATE of bug 328765

(Core :: JavaScript Engine, defect)

Description

[timeless]

i think this is probably "son of bug 280086"

<shaver> which getvar is it crashing on? wonder if the script is being executed 
on a different object than it was compiled against. so the function you 
disassembled isn't relevant.
> it isn't? did i grab the wrong function?
<shaver> well. it doesn't generate the same bytecode as a top-level script 
compiled for COMPILE_N_GO. for example. you might notice that there is no 
getgvar in that stream
> oh the thing i inlined from jsshell was a guide. it definitely isn't a 
perfect match. but the thing i hand disassembled is the right crashing script, 
right? ok, so according to the pc, it's the first getvar
> defvar,defvar,push,name,toobject,forname,ifeq 33,_getvar_
> given that the code was:
> for (var iter in nest)
>  if (iter in this.prototype)
>   this.prototype[iter]=nest[iter];
> it should be one of nest[iter] or this.prototype[iter], right?
* timely goes to read the helpful hint from dis()
<shaver> first getgvar, you mean?
* timely goes back to the crashing c
<shaver> so I think the problem here is that we're compiling against one
global object. and running against another. and we told the compiler we weren't 
going to do that. because if we didn't tell the compiler that, it wouldn't have 
used the getgvar optimization.
<shaver> I see venkman's fingerprints all over this
> oh yes, yes :)
<shaver> how do you construct the script that's crashing?
> it's entered into the venkman input. and i hit <enter>
<shaver> OK. don't suppose you know what jsdgbapi call that translates to?  
here we go. hrm.
> you probably are asking about some stack frame, 02..05
<shaver> 02, yeah. can you reproduce this crash?
> i haven't tried
<shaver> ok
<shaver> can you tell me what sort of object we have as the second parameter
to js_Execute?
<shaver> you can't just grab 'chain' from frame 01?
* timely starts digging through slots[0..2]
> nslots=0x5d
> slots[-1]=0x5d
> at least they're consistent :)
<shaver> that disassembly doesn't make sense, then
<shaver> hrm
<shaver> because it's loading gvar 0
> the disassembly is mine based on my poor
<shaver> oh! what's fp->nvars?
> 0
<shaver> mmmm. yes, OK
> that's the JS_Assert that's botched
<shaver> right. it's starting to come together for me now. we're not setting 
this frame up correctly for this script
> note that i'm mixing just about every evil flavor of thing available
<shaver> normally, we'd be creating a frame for the script, and using its
+global variable info to create slots
> subscriptloader, debugger;, venkmaneval
<shaver> but in this case we're reusing a frame
> the frame from data:text/javascript,debugger. i presume
<shaver> the frame that you're stopped in
> yes, that's where i'm stopped. that's the subscript that was loaded last
<shaver> oh
> and that's how i land in venkman. a very nice, tiny, and neat frame
<shaver> yes
> very tiny
<shaver> hmm. I wonder if that faux-frame is the problem.
<shaver> so the easy fix is just to remove the option twiddling in 
JS_EvaluateUCInStackFrame. which will give us slightly slower eval-in-stack-
frame behaviour
> is JS_EvaluateUCInStackFrame used normally, or really only by venkman like
consumers?
* timely presumes only the latter
<shaver> the latter

--source lines are from our 1.8 branch
00 js3250!js_Interpret+0x219 [c:\build\chs3\build\mozilla\js\src\jsinterp.c @ 
4150]
01 js3250!js_Execute+0x1a0 [c:\build\chs3\build\mozilla\js\src\jsinterp.c @ 
1394]
02 js3250!JS_EvaluateUCInStackFrame+0x6e [c:\build\chs3
\build\mozilla\js\src\jsdbgapi.c @ 930]
03 jsd3250!jsd_EvaluateUCScriptInStackFrame+0x7f [c:\build\chs3
\build\mozilla\js\jsd\jsd_stak.c @ 457]
--
          case JSOP_GETGVAR:
            slot = GET_VARNO(pc);
/* fp->nvars = 0
   fp->vars = 0x0
   assert would botch in a debug build */
            JS_ASSERT(slot < fp->nvars);
/* crash is here */
            lval = fp->vars[slot];
            if (JSVAL_IS_NULL(lval)) {
                op = JSOP_NAME;
                goto do_op;
            }

Comment 1

[timeless]

i disabled compile and go, it didn't fix this.

i believe the problem is that the optimizer doesn't understand that the 
subscriptloader is like 'with' and 'eval'

     * We can't optimize if we're in an eval called inside a with statement,
     * or we're compiling a with statement and its body, or we're in a catch
     * block whose exception variable has the same name as pn.

there's a scopeChain, but fp->nvars is empty and somehow delegates to it.

Comment 2

[Brendan Eich [:brendan]]

Timeless: what does the subscript loader have to do with the fragment of the
stack showin in comment 0?  Can you give a full stack?

/be

Comment 3

[timeless]

Attachment: i think this should work

there are various tweaks i can make to various places that would result in this
code not crashing. the two reasonable candidates of current checks are:

LookupArgOrVar(JSContext *cx, JSTreeContext *tc, JSParseNode *pn)
     * A Script object can be used to split an eval into a compile step done
     * at construction time, and an execute step done separately, possibly in
     * a different scope altogether.  We therefore cannot do any name-to-slot
     * optimizations, but must lookup names at runtime.  Note that script_exec
     * ensures that its caller's frame has a Call object, so arg and var name
     * lookups will succeed.

     * We can't optimize if we're not compiling a function body, whether via
     * eval, or directly when compiling a function statement or expression.

i'm not sure this is reasonable:
     * We can't optimize if we're in an eval called inside a with statement,
     * or we're compiling a with statement and its body, or we're in a catch
     * block whose exception variable has the same name as pn.

unfortunately, clasp is js_Object (but it could be whatever one picked as the
second parameter to the subscriptloader), so this is a bad thing to try to
manipulate. so the question is then, who is in a good position to influence
flags.

perhaps the right thing to fix is the subscriptloader since it's the evil thing
that's manipulating the object world, except JS_EvaluateScriptForPrincipals
doesn't have any way to propose flags. 

i have tested using current nightly builds and have not managed to coerce the
subscriptloader alone to trigger this crash, although i'm not confident it
can't be done.

since i don't want my poor debugger clients to deal w/ optimizers, i'm favoring
changing the debug apis to discourage optimization.

I should note that LookupArgOrVar is perfectly well aware that fp->argc = 0 and
fp->argv = 0x0 and thus that it is suicide to select the optimization that it
is selecting (as such, it is my humble and ignorant opinion that the optimizer
is broken and should opt not to take these optimizations). that said. I really
don't feel like arguing for changing the logic in the optimizer.

i have tested by slowly crawling through the optimizer and undoing each
optimization it makes to iter, and the code indeed does not crash if the
optimization is undone.

00 js3250!LookupArgOrVar+0xb7
...
04 js3250!js_CompileTokenStream+0xa5
05 js3250!CompileTokenStream+0x7a
06 js3250!JS_CompileUCScriptForPrincipals+0x39
07 js3250!JS_EvaluateUCInStackFrame+0x48
08 jsd3250!jsd_EvaluateUCScriptInStackFrame+0x7f
09 jsd3250!JSD_AttemptUCScriptInStackFrame+0x22
0a jsd3250!jsdStackFrame::Eval+0x87
0b xpcom_core!XPTC_InvokeByIndex+0x27
0c xpc3250!XPCWrappedNative::CallMethod+0x6c4
0d xpc3250!XPC_WN_CallMethod+0x8e
0e js3250!js_Invoke+0x539
...
16 js3250!JS_CallFunctionValue+0x1e
17 gklayout!nsJSContext::CallEventHandler+0xa6
...
2f jsd3250!jsdService::EnterNestedEventLoop+0x127
30 xpcom_core!XPTC_InvokeByIndex+0x27
31 xpc3250!XPCWrappedNative::CallMethod+0x6c4
32 xpc3250!XPC_WN_CallMethod+0x8e
33 js3250!js_Invoke+0x539
...
35 js3250!js_Invoke+0x57a
36 xpc3250!nsXPCWrappedJSClass::CallMethod+0x6b1
37 xpc3250!nsXPCWrappedJS::CallMethod+0x27
38 xpcom_core!PrepareAndDispatch+0xee
39 xpcom_core!SharedStub+0x16
3a jsd3250!jsds_ExecutionHookProc+0x182
3b jsd3250!jsd_CallExecutionHook+0x56
3c jsd3250!jsd_DebuggerHandler+0x83
3d js3250!js_Interpret+0x74c0
3e js3250!js_Execute+0x1a0
3f js3250!JS_EvaluateUCScriptForPrincipals+0x56
40 js3250!JS_EvaluateScriptForPrincipals+0x37
41 xpc3250!mozJSSubScriptLoader::LoadSubScript+0x2f3

Comment 4

[Brendan Eich [:brendan]]

Comment on attachment 197503 [details] [diff] [review]
i think this should work

Not bad as a minimal patch, but not what we want for the long run.

Let me poke around a bit and get back to you here. I'll confirm the bug (you
could have done that, you know!).

/be

Comment 5

[timeless]

brendan: it'd still be nice to have something for 1.8 branch...

Comment 6

[Brendan Eich [:brendan]]

mrbkap, didn't you fix this recently?  I think so:

http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/js/src&command=DIFF_FRAMESET&file=jsemit.c&rev1=3.147&rev2=3.148&root=/cvsroot

See bug 328765.

/be

*** This bug has been marked as a duplicate of 328765 ***

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

Yeah, either bug 328765 or bug 326467.