Last Comment Bug 758617 - Crash in js::StackIter::settleOnNewState()
: Crash in js::StackIter::settleOnNewState()
Status: RESOLVED FIXED
: crash
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86 Mac OS X
: -- critical (vote)
: mozilla15
Assigned To: Luke Wagner [:luke]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-25 07:34 PDT by Panos Astithas [:past] (away until 7/21)
Modified: 2012-05-31 05:55 PDT (History)
6 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
fix and test (4.25 KB, patch)
2012-05-29 14:48 PDT, Luke Wagner [:luke]
jorendorff: review+
Details | Diff | Splinter Review

Description Panos Astithas [:past] (away until 7/21) 2012-05-25 07:34:33 PDT
STR:

1) Visit http://htmlpad.org/debugger/
2) Open the debugger
3) Click on the 'Click me!' button
4) Edit the value of variable 'a' in the variable pane to asdf (no quotes!)
5) Boom.

Full log:
http://past.pastebin.mozilla.org/1649938

Top of the stack:

js::StackIter::settleOnNewState() + 205
js::StackIter::operator++() + 237
_ZL14InitExnPrivateP9JSContextN2JS6HandleIP8JSObjectEENS2_IP8JSStringEES8_jP13JSErrorReporti + 1265
js_ErrorToException(JSContext*, char const*, JSErrorReport*, JSErrorFormatString const* (*)(void*, char const*, unsigned int), void*) + 913
_ZL11ReportErrorP9JSContextPKcP13JSErrorReportPFPK19JSErrorFormatStringPvS2_jES8_ + 109
js_ReportErrorNumberVA(JSContext*, unsigned int, JSErrorFormatString const* (*)(void*, char const*, unsigned int), void*, unsigned int, int, __va_list_tag*) + 265
+ 200
js::NameOperation(JSContext*, unsigned char*, JS::Value*) + 1170
js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) + 4014
js::RunScript(JSContext*, JSScript*, js::StackFrame*) + 957
js::ExecuteKernel(JSContext*, JSScript*, JSObject&, JS::Value const&, js::ExecuteType, js::StackFrame*, JS::Value*) + 1298
js::EvaluateInEnv(JSContext*, JS::Handle<JSObject*>, js::StackFrame*, unsigned short const*, unsigned int, char const*, unsigned int, JS::Value*) + 675
_ZL17DebuggerFrameEvalP9JSContextjPN2JS5ValueE16EvalBindingsMode + 1030
js::CallJSNative(JSContext*, int (*)(JSContext*, unsigned int, JS::Value*), js::CallArgs const&) + 907
Comment 1 Luke Wagner [:luke] 2012-05-29 14:14:57 PDT
Ah, this seems to be fallout from allowing eval-in-frame to eval in a frame in a different context.  I'll try to whip up a fix.
Comment 2 Luke Wagner [:luke] 2012-05-29 14:48:21 PDT
Created attachment 628115 [details] [diff] [review]
fix and test

Delightfully simple fix.  I audited the rest of the uses of prevInContext and they seem to be fine.
Comment 3 Jason Orendorff [:jorendorff] 2012-05-30 14:10:07 PDT
Comment on attachment 628115 [details] [diff] [review]
fix and test

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

r=me.

StackIter's comment should point out that it's really just following the prev() chain, and in corner cases like eval-in-frame that can get weird. I think StackIter::settleOnNewState() should have a comment somewhere that tells what it's for. (In general I think API comments, documenting contracts, are very often worth it. We don't do enough of those.)

::: js/src/jit-test/tests/debug/cross-context-3.js
@@ +4,5 @@
> +var dbg = new Debugger(g);
> +var hits = 0;
> +dbg.onDebuggerStatement = function (frame1) {
> +    dbg.onDebuggerStatement = function (frame2) {
> +        print(frame1.eval("a").throw);

Assert something instead of printing it?
Comment 4 Luke Wagner [:luke] 2012-05-30 16:18:40 PDT
(In reply to Jason Orendorff [:jorendorff] from comment #3)
> (In general I think API comments, documenting
> contracts, are very often worth it. We don't do enough of those.)

Madness
Comment 6 Ed Morley [:emorley] 2012-05-31 05:55:43 PDT
https://hg.mozilla.org/mozilla-central/rev/02e49bc6ead9

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