607513 - "Assertion failure: pc >= script->main && pc < script->code + script->length"

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P2)

Description

[Jesse Ruderman]

Attachment: stack trace

./js

Object.freeze(this); eval('const q;')

Assertion failure: pc >= script->main && pc < script->code + script->length, 
at js/src/jsopcode.cpp:5122

Comment 1

[Brendan Eich [:brendan]]

Attachment: proposed fix with test

Prolog ops were always excluded by these js_DecompileValueGenerator and DecompileExpression assertions because such ops don't have stack effects, but such ops can fail with errors now due to ES5 features.

/be

Comment 2

[Brendan Eich [:brendan]]

http://hg.mozilla.org/tracemonkey/rev/3b16a81d7832

/be

Comment 3

[Brendan Eich [:brendan]]

http://hg.mozilla.org/tracemonkey/rev/df00f9f13619

See bug 607535 (thanks, bc).

/be

Comment 4

[Brendan Eich [:brendan]]

http://hg.mozilla.org/mozilla-central/rev/3b16a81d7832

/be

Comment 5

[Brendan Eich [:brendan]]

Regression from bug 547851, context diff snip:

+    fp = i.fp();
     script = fp->script;
-    regs = fp->regs;
-    pc = fp->imacpc ? fp->imacpc : regs->pc;
-    if (pc < script->main || script->code + script->length <= pc) {
-        JS_NOT_REACHED("bug");
-        goto do_fallback;
-    }
+    pc = fp->imacpc ? fp->imacpc : i.pc();
+    JS_ASSERT(pc >= script->main && pc < script->code + script->length);
 
     if (spindex != JSDVG_IGNORE_STACK) {

Bill's fix is fine, but you can see that the old code (which goes way, wayyy back in time, viz

3.10         (fur%nets 14-Oct-98):     if (JS_UPTRDIFF(pc, script->code) >= (jsuword)script->length) {
3.39         (brendan% 14-Sep-00):         pc = fp->pc;
3.39         (brendan% 14-Sep-00):         if (!pc)
3.39         (brendan% 14-Sep-00):             goto do_fallback;

via an Igor patch in 2008 for bug 420399) was paranoid, as most jsopcode.cpp decompilation code is. It would check for pc being outside of both ends of the half-open interval.

The decompiler can be fall out of sync with bytecode semantics and stack state, but it should fail soft where possible. Better a mangled diagnostic or toString result than an exploitable crash.

/be

Comment 6

[Christian Holler (:decoder)]

Automatically extracted testcase for this bug was committed:

https://hg.mozilla.org/mozilla-central/rev/efaf8960a929