764401 - Assertion failure: flags_ & HAS_ARGS_OBJ, at js/src/vm/Stack-inl.h:325

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Panos Astithas (he/him) [:past] (please ni?)]

STR:
1) visit http://harthur.github.com/wwcode/
2) open the debugger and set a breakpoint in wwcode.js:35
3) reload page
4) after the execution pauses, step into/over a couple of times
5) boom


Stack trace:

0   js::StackFrame::argsObj() const + 72
1   js::DebugScopeProxy::handleUnaliasedAccess(JSContext*, js::ScopeObject&, jsid, js::DebugScopeProxy::Action, JS::Value*) + 2518
2   js::DebugScopeProxy::get(JSContext*, JSObject*, JSObject*, jsid, JS::Value*) + 131
3   _ZL16proxy_GetGenericP9JSContextN2JS6HandleIP8JSObjectEES5_NS2_I4jsidEEPNS1_5ValueE + 174
4   _ZL23DebuggerEnv_getVariableP9JSContextjPN2JS5ValueE + 663
5   js::CallJSNative(JSContext*, int (*)(JSContext*, unsigned int, JS::Value*), js::CallArgs const&) + 907
6   js::InvokeKernel(JSContext*, js::CallArgs, js::MaybeConstruct) + 285
7   js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) + 3893


Full stack:
http://past.pastebin.mozilla.org/1661860

Comment 1

[Luke Wagner [:luke]]

Oh, hah, the debugger is observing the stack frame during the prologue, before the argsobj has even been created.  Fortunately, we have a frame flag for that, so simple fix.  Thanks for the report and simple STR!

Comment 2

[Luke Wagner [:luke]]

Attachment: fix and test

Jason: jimb reviewed the patch this patch is fixing, but I think he is out for 2 weeks so hopefully the comment in Stack.h should explain what is happening enough here.

Comment 3

[Jim Blandy :jimb]

Comment on attachment 632787 [details] [diff] [review]
fix and test

Review of attachment 632787 [details] [diff] [review]:
-----------------------------------------------------------------

Some drive-by comments:

I wonder whether it wouldn't be better to simply remove JSOP_ARGUMENTS altogether, and fuse creation of the arguments and call objects with the function call operation.

This also makes me wonder what happens when one tries to evaluate "arguments" at that breakpoint...

::: js/src/vm/Debugger.cpp
@@ +3168,5 @@
>      Value arg;
>      if (unsigned(i) < fp->numActualArgs()) {
>          if (unsigned(i) < fp->numFormalArgs() && fp->script()->formalLivesInCallObject(i))
>              arg = fp->callObj().arg(i);
> +        else if (fp->script()->argsObjAliasesFormals() && fp->hasArgsObj())

I would like this to say:
// The debugger can stop programs pre-prologue, before the JSOP_ARGUMENTS.

Certainly if one has read the hasArgsObj comment, this is clear enough; but we've had lazily-created arguments objects before (right???), so I first read right through the hasArgsObj call as having nothing to do with normally-invisible pre-prologue states.

Comment 4

[Jim Blandy :jimb]

It seems to me this should go in Aurora, too; anyone willing to second the nomination?

Comment 5

[Luke Wagner [:luke]]

(In reply to Jim Blandy :jimb from comment #3)
> I wonder whether it wouldn't be better to simply remove JSOP_ARGUMENTS
> altogether, and fuse creation of the arguments and call objects with the
> function call operation.

We've been moving in the opposite direction: doing less in the prologue and more in initial bytecode.  This mostly simplifies jitting.  I think dvander will even be moving callobj creation to a new JSOP sometime soon.  Also, none of this changes the fact that the frame is still observable during the prologue (calls into C++ during call/args-obj creation).  Thus, I think the right thing to do is just accept the fact that the weird non-script ways we observe the frame (viz., error-handling and debugger) must deal with these partially-prologued states.

> This also makes me wonder what happens when one tries to evaluate
> "arguments" at that breakpoint...

You get 'undefined' (initial value of all local slots).  If we fix bug 764511, the debugger shouldn't observe the frame in this state, though.

Comment 6

[Panos Astithas (he/him) [:past] (please ni?)]

(In reply to Jim Blandy :jimb from comment #4)
> It seems to me this should go in Aurora, too; anyone willing to second the
> nomination?

Yes, please!

Comment 7

[Luke Wagner [:luke]]

Comment on attachment 632787 [details] [diff] [review]
fix and test

Oh, jimb's back!  Switching back per comment 2.

Comment 8

[Luke Wagner [:luke]]

(In reply to Panos Astithas [:past] from comment #6)
I'll flag a? once landed.

Comment 9

[Jim Blandy :jimb]

(In reply to Luke Wagner [:luke] from comment #7)
> Comment on attachment 632787 [details] [diff] [review]
> fix and test
> 
> Oh, jimb's back!  Switching back per comment 2.

Well, I'm sort of back. I'll be working 25-27 (Mon-Wed), and then back home again on July 3rd.

Comment 10

[Jim Blandy :jimb]

(In reply to Luke Wagner [:luke] from comment #5)
> > This also makes me wonder what happens when one tries to evaluate
> > "arguments" at that breakpoint...
> 
> You get 'undefined' (initial value of all local slots).  If we fix bug
> 764511, the debugger shouldn't observe the frame in this state, though.

Yeah, I agree that we should hide those intermediate states from the debugger. I'll comment there.

Comment 11

[Jim Blandy :jimb]

Comment on attachment 632787 [details] [diff] [review]
fix and test

Review of attachment 632787 [details] [diff] [review]:
-----------------------------------------------------------------

Hurrah!

Comment 12

[Luke Wagner [:luke]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/192fa572b0c4

Comment 13

[Luke Wagner [:luke]]

Oh, the bug isn't on aurora; this is in the DebugScopeProxy additions in bug 659577 which is FF16.

Comment 14

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/192fa572b0c4

Comment 15

[Panos Astithas (he/him) [:past] (please ni?)]

(In reply to Luke Wagner [:luke] from comment #13)
> Oh, the bug isn't on aurora; this is in the DebugScopeProxy additions in bug
> 659577 which is FF16.

Does that mean we won't get this in 15 then?

Comment 16

[Luke Wagner [:luke]]

yep