Closed Bug 360969 Opened 13 years ago Closed 13 years ago

This page crashes SpiderMonkey [@ js_LookupPropertyWithFlags]

Categories

(Core :: JavaScript Engine, defect, P2, critical)

defect

Tracking

()

VERIFIED FIXED
mozilla1.9alpha1

People

(Reporter: jsarapata, Assigned: mrbkap)

References

()

Details

(4 keywords)

Crash Data

Attachments

(1 file)

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.
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
Attached patch Potential fixSplinter Review
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.
(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.
(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
Blocks: js1.7src
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+
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
Closed: 13 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
/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
Flags: in-testsuite+
Crash Signature: [@ js_LookupPropertyWithFlags]
You need to log in before you can comment on or make changes to this bug.