Last Comment Bug 732496 - Assertion failure: spoff == js_ReconstructStackDepth(cx_, fp_->script(), pc_), at vm/Stack.cpp:1150 or Crash [@ CrashIfInvalidSlot]
: Assertion failure: spoff == js_ReconstructStackDepth(cx_, fp_->script(), pc_)...
Status: RESOLVED FIXED
js-triage-done
: assertion, crash, testcase
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86 Linux
: -- critical (vote)
: mozilla13
Assigned To: general
:
: Jason Orendorff [:jorendorff]
Mentors:
: 733141 (view as bug list)
Depends on:
Blocks: langfuzz
  Show dependency treegraph
 
Reported: 2012-03-02 10:55 PST by Christian Holler (:decoder)
Modified: 2012-03-13 04:41 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Test case for shell (see README file inside). (3.40 KB, application/x-compressed-tar)
2012-03-02 10:55 PST, Christian Holler (:decoder)
no flags Details
fix (1.11 KB, patch)
2012-03-09 16:31 PST, Luke Wagner [:luke]
bhackett1024: review+
Details | Diff | Splinter Review

Description Christian Holler (:decoder) 2012-03-02 10:55:23 PST
Created attachment 602412 [details]
Test case for shell (see README file inside).

The attached test asserts/crashes on mozilla-central revision 343ec916dfd5 (see README for running instructions).

Crash backtrace (from opt build):

==38069== Process terminating with default action of signal 11 (SIGSEGV)
==38069==  Access not within mapped region at address 0xBAD
==38069==    at 0x8184DF3: CrashIfInvalidSlot(js::StackFrame*, JS::Value*) (Stack.cpp:1065)
==38069==    by 0x8186D2E: js::StackIter::settleOnNewState() (Stack.cpp:1153)
==38069==    by 0x80AFFBA: js::types::TypeObject::clearNewScript(JSContext*) (Stack.h:1860)
==38069==    by 0x80B3247: js::types::TypeObject::markUnknown(JSContext*) (jsinferinlines.h:1099)
==38069==    by 0x80BEFA8: js::types::TypeObject::getProperty(JSContext*, int, bool) (jsinferinlines.h:1209)
==38069==    by 0x80B457C: js::types::TypeObject::getFromPrototypes(JSContext*, int, js::types::TypeSet*, bool) (jsinfer.cpp:2774)
==38069==    by 0x80B580B: PropertyAccess(JSContext*, JSScript*, unsigned char*, js::types::TypeObject*, bool, js::types::TypeSet*, int) (jsinfer.cpp:1040)
==38069==    by 0x80BCF2F: TypeConstraintProp::newType(JSContext*, js::types::TypeSet*, js::types::Type) (jsinfer.cpp:1098)
==38069==    by 0x80763A9: js::types::TypeCompartment::resolvePending(JSContext*) (jsinferinlines.h:795)
==38069==    by 0x80B2B9F: js::types::TypeMonitorResult(JSContext*, JSScript*, unsigned char*, JS::Value const&) (jsinfer.cpp:5160)
==38069==    by 0x80C9DCD: js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) (jsinferinlines.h:574)
==38069==    by 0x81C875C: js::mjit::EnterMethodJIT(JSContext*, js::StackFrame*, void*, JS::Value*, bool) (MethodJIT.cpp:1079)
Comment 1 Luke Wagner [:luke] 2012-03-02 12:18:26 PST
The fix is pretty simple:

diff --git a/js/src/jsinterp.cpp b/js/src/jsinterp.cpp
@@ -2694,18 +2694,19 @@ BEGIN_CASE(JSOP_CALL)
-        regs.sp = args.spAfterCall();
-        TypeScript::Monitor(cx, script, regs.pc, regs.sp[-1]);
+        Value *newsp = args.spAfterCall();
+        TypeScript::Monitor(cx, script, regs.pc, newsp[-1]);
+        regs.sp = newsp;

Brian, do you know how to generate a short test-case for this?  To error condition is to get this TypeScript::Monitor call to inspect the stack with FrameIter/StackIter (in this case a call to clearNewScript).
Comment 2 Christian Holler (:decoder) 2012-03-05 15:29:09 PST
*** Bug 733141 has been marked as a duplicate of this bug. ***
Comment 3 Christian Holler (:decoder) 2012-03-08 10:23:09 PST
(In reply to Luke Wagner [:luke] from comment #1)
> The fix is pretty simple:

Tested this fix and it worked for me (solved also other OOM situations with the same assert).

Can we get this landed or is it blocked by a better testcase?

> Brian, do you know how to generate a short test-case for this?  To error
> condition is to get this TypeScript::Monitor call to inspect the stack with
> FrameIter/StackIter (in this case a call to clearNewScript).

Would it be sufficient if this was covered by the OOM testing mechanism or do we need to cover more situations?
Comment 4 Luke Wagner [:luke] 2012-03-09 16:31:05 PST
Created attachment 604562 [details] [diff] [review]
fix

Righto
Comment 5 Brian Hackett (:bhackett) 2012-03-10 06:41:35 PST
Comment on attachment 604562 [details] [diff] [review]
fix

Oh, this would be kind of tricky to write a small testcase for.  The new type needs to trigger type changes that invalidate a type's newScript info, which can only happen in narrow circumstances, e.g. calling foo.call in the constructor and either foo or foo.call is changed by the new type.
Comment 7 Marco Bonardo [::mak] 2012-03-13 04:41:12 PDT
https://hg.mozilla.org/mozilla-central/rev/dd2a4596f15c

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