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

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

Description User image Panos Astithas [:past] 2012-05-25 07:34:33 PDT

1) Visit
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:

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 User image 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 User image 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 User image 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]:


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 User image 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.)

Comment 6 User image Ed Morley [:emorley] 2012-05-31 05:55:43 PDT

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