Last Comment Bug 764401 - Assertion failure: flags_ & HAS_ARGS_OBJ, at js/src/vm/Stack-inl.h:325
: Assertion failure: flags_ & HAS_ARGS_OBJ, at js/src/vm/Stack-inl.h:325
Status: RESOLVED FIXED
[js:p2:fx17][js:bumped:1]
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla16
Assigned To: Luke Wagner [:luke]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-06-13 08:02 PDT by Panos Astithas [:past]
Modified: 2012-06-29 10:29 PDT (History)
5 users (show)
jimb: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
fix and test (9.39 KB, patch)
2012-06-13 11:38 PDT, Luke Wagner [:luke]
jimb: review+
Details | Diff | Review

Description Panos Astithas [:past] 2012-06-13 08:02:39 PDT
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] 2012-06-13 10:55:31 PDT
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] 2012-06-13 11:38:42 PDT
Created attachment 632787 [details] [diff] [review]
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 2012-06-16 18:36:40 PDT
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 2012-06-16 18:38:03 PDT
It seems to me this should go in Aurora, too; anyone willing to second the nomination?
Comment 5 Luke Wagner [:luke] 2012-06-17 22:00:34 PDT
(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 [:past] 2012-06-18 05:31:58 PDT
(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] 2012-06-18 09:33:58 PDT
Comment on attachment 632787 [details] [diff] [review]
fix and test

Oh, jimb's back!  Switching back per comment 2.
Comment 8 Luke Wagner [:luke] 2012-06-18 09:34:49 PDT
(In reply to Panos Astithas [:past] from comment #6)
I'll flag a? once landed.
Comment 9 Jim Blandy :jimb 2012-06-20 02:45:57 PDT
(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 2012-06-20 02:48:13 PDT
(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 2012-06-20 02:54:39 PDT
Comment on attachment 632787 [details] [diff] [review]
fix and test

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

Hurrah!
Comment 13 Luke Wagner [:luke] 2012-06-20 12:22:03 PDT
Oh, the bug isn't on aurora; this is in the DebugScopeProxy additions in bug 659577 which is FF16.
Comment 14 Ed Morley [:emorley] 2012-06-21 04:07:28 PDT
https://hg.mozilla.org/mozilla-central/rev/192fa572b0c4
Comment 15 Panos Astithas [:past] 2012-06-29 03:02:05 PDT
(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] 2012-06-29 10:29:42 PDT
yep

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