This page crashes SpiderMonkey [@ js_LookupPropertyWithFlags]

VERIFIED FIXED in mozilla1.9alpha1

Status

()

P2
critical
VERIFIED FIXED
12 years ago
8 years ago

People

(Reporter: jsarapata, Assigned: mrbkap)

Tracking

(4 keywords)

Trunk
mozilla1.9alpha1
crash, testcase, verified1.8.0.9, verified1.8.1.1
Points:
---
Bug Flags:
blocking1.8.1.1 +
blocking1.8.0.9 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(crash signature, URL)

Attachments

(1 attachment)

(Reporter)

Description

12 years ago
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

12 years ago
Happens on Linux trunk as well.
Assignee: nobody → general
Status: UNCONFIRMED → NEW
Component: General → JavaScript Engine
Ever confirmed: true
Keywords: crash, testcase
Product: Firefox → Core
QA Contact: general → general
Summary: This page crashes SpiderMonkey → This page crashes SpiderMonkey [@ js_LookupPropertyWithFlags]
Version: unspecified → Trunk
OS: Linux → All
Hardware: PC → All
Created attachment 245812 [details] [diff] [review]
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.
Assignee: general → mrbkap
Status: NEW → ASSIGNED
Attachment #245812 - Flags: review?(brendan)
Do we have any testcases that actually exercise our extended-atom opcodes?
Priority: -- → P2
Target Milestone: --- → mozilla1.9alpha
(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

12 years ago
(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 ?
(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.
(Reporter)

Comment 7

12 years ago
(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

Updated

12 years ago
Blocks: 355044
Flags: blocking1.8.1.1?
Flags: blocking1.8.0.9?
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
Attachment #245812 - Flags: review?(brendan) → review+
Flags: blocking1.8.1.1?
Flags: blocking1.8.1.1+
Flags: blocking1.8.0.9?
Flags: blocking1.8.0.9+

Updated

12 years ago
Attachment #245812 - Flags: approval1.8.1.1?
Attachment #245812 - Flags: approval1.8.0.9?
Comment on attachment 245812 [details] [diff] [review]
Potential fix

approved for 1.8 branch, a=dveditz for drivers
Attachment #245812 - Flags: approval1.8.1.1?
Attachment #245812 - Flags: approval1.8.1.1+
Attachment #245812 - Flags: approval1.8.0.9?
Attachment #245812 - Flags: approval1.8.0.9+
Fixed on branch and trunk.
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Keywords: fixed1.8.1.1
Resolution: --- → FIXED
...and on the other branch too...
Keywords: fixed1.8.0.9
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.
Status: RESOLVED → VERIFIED
Keywords: fixed1.8.0.9, fixed1.8.1.1 → verified1.8.0.9, verified1.8.1.1

Comment 13

12 years ago
/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

Updated

12 years ago
Flags: in-testsuite+
Crash Signature: [@ js_LookupPropertyWithFlags]
You need to log in before you can comment on or make changes to this bug.