360969 - This page crashes SpiderMonkey [@ js_LookupPropertyWithFlags]

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P2)

Description

[John Sarapata]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.7) Gecko/20060921 Ubuntu/dapper-security Firefox/1.5.0.7
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.7) Gecko/20060921 Ubuntu/dapper-security Firefox/1.5.0.7

I've tried this page with FireFox 1.5 both under Mac OS X and Ubuntu Linux on an HP. It crashes. It also crashes when I drive SpiderMonkey directly.

I last did a cvs up on November 8, but this bug appears to be not related to recent changes.

It appears to be some sort of memory stress test. The page has innocuous enough JavaScript, but when I delete about a third from anywhere, I can run with no crash.

The actual error occurs at the first CHECK_FOR_STRING_INDEX in js_LookupPropertyWithFlags. It looks like ATOM_TO_STRING gives a string with chars pointing to a buserror.

My stack crawl is:
PC: @  0x829f9a9  js_LookupPropertyWithFlags
    @  0x8592911  FailureSignalHandler()
    @ 0xb7fff440  (unknown)
    @  0x829f976  js_LookupProperty
    @  0x827ecdd  js_CheckRedeclaration
    @  0x828f5db  js_Interpret
    @  0x827e739  js_Execute
    @  0x824c841  JS_EvaluateUCScriptForPrincipals
    @  0x824c798  JS_EvaluateUCScript

The interesting bit that I can't figure out seems to be in jsinterp.c, around line 4706 in my copy:
          do_JSOP_DEFCONST:
          do_JSOP_DEFVAR:
            atom = js_GetAtom(cx, &script->atomMap, atomIndex);
            obj = fp->varobj;
            attrs = JSPROP_ENUMERATE;
            if (!(fp->flags & JSFRAME_EVAL))
                attrs |= JSPROP_PERMANENT;
            if (op == JSOP_DEFCONST)
                attrs |= JSPROP_READONLY;

            /* Lookup id in order to check for redeclaration problems. */
            id = ATOM_TO_JSID(atom);
            SAVE_SP_AND_PC(fp);
            ok = js_CheckRedeclaration(cx, obj, id, attrs, &obj2, &prop);
^^^^^^^ The crashing line
            if (!ok)
                goto out;



Reproducible: Always

Steps to Reproduce:
1. Open the web page. I have tried Unbunto on an HP and Mac OS X on a MacBook Pro.

Actual Results:  
Firefox disappears in a poof of smoke and a bus error.

Expected Results:  
Firefox should not crash.

Comment 1

[Adam Guthrie]

Happens on Linux trunk as well.

Comment 2

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Potential fix

This page has enough atoms to exercise our JSOP_LITOPX magic. The problem is that JSOP_DEFVAR (and JSOP_DEFCONST) both have JOF_NAME in their modemask, which confuses EmitAtomIndexOp into emitting JSOP_FINDNAME instead of JSOP_LITOPX. Therefore, I think we get into JSOP_FINDNAME with a bogus atomIndex and crash.

I don't think that JSOP_DEF{VAR,CONST} can ever be the generating pc, so this shouldn't hurt decompiling any.

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

Do we have any testcases that actually exercise our extended-atom opcodes?

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

(In reply to comment #2)
> Therefore, I think we get into JSOP_DEFVAR with a bogus
> atomIndex and crash.

Oops, rereading comment 0 shows that it's JSOP_DEFVAR that ends up with the bogus atomIndex.

Comment 5

[Bob Clary [:bc] (inactive)]

(In reply to comment #3)
> Do we have any testcases that actually exercise our extended-atom opcodes?
> 

Not that I know of. What is an extended-atom opcode ?

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

(In reply to comment #5)
> Not that I know of. What is an extended-atom opcode ?

I should have said extended-atomIndex opcodes. Since the interpreter only offers 16-bit immediates, referring to atom indexes > 2^16 in an immediate (the normal way atoms are given to opcodes) is impossible. Therefore, when we pass that boundary, we emit slightly different opcodes.

I think the only way to trigger this is to have more than 2^16 atoms (strings, function objects, /regular expressions/,  etc.), but I'm not sure.

Comment 7

[John Sarapata]

(In reply to comment #2)
> Created an attachment (id=245812) [edit]
> Potential fix

This patch fixes the problem for me. It also fixes a similar page:  http://linguistica.uchicago.edu/hmmviz/index.php?hmm=French_3x1000

Comment 8

[Brendan Eich [:brendan]]

Comment on attachment 245812 [details] [diff] [review]
Potential fix

There was at least one testcase in another bug.  It used eval to generate a bunch of x1, x2, x3, ...x65535, x65536 var names.

/be

Comment 9

[Daniel Veditz [:dveditz]]

Comment on attachment 245812 [details] [diff] [review]
Potential fix

approved for 1.8 branch, a=dveditz for drivers

Comment 10

[Blake Kaplan (:mrbkap) (inactive)]

Fixed on branch and trunk.

Comment 11

[Blake Kaplan (:mrbkap) (inactive)]

...and on the other branch too...

Comment 12

[Martijn Wargers (dead)]

Verified fixed on trunk, 1.8.1 branch and 1.8.0.x branch, using builds before the patch was checked in and builds after the patch was checked in.

Comment 13

[Bob Clary [:bc] (inactive)]

/cvsroot/mozilla/js/tests/js1_5/Regress/regress-360969-01.js,v  <--  regress-360969-01.js

/cvsroot/mozilla/js/tests/js1_5/Regress/regress-360969-02.js,v  <--  regress-360969-02.js

/cvsroot/mozilla/js/tests/js1_5/Regress/regress-360969-03.js,v  <--  regress-360969-03.js

/cvsroot/mozilla/js/tests/js1_5/Regress/regress-360969-04.js,v  <--  regress-360969-04.js

/cvsroot/mozilla/js/tests/js1_5/Regress/regress-360969-05.js,v  <--  regress-360969-05.js

/cvsroot/mozilla/js/tests/js1_5/Regress/regress-360969-06.js,v  <--  regress-360969-06.js