Closed Bug 798589 Opened 8 years ago Closed 8 years ago

Crash [@ JSScript::ionScript] or "Assertion failure: hasIonScript(),"

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla18
Tracking Status
firefox15 --- unaffected
firefox16 --- unaffected
firefox17 --- unaffected
firefox18 + fixed
firefox-esr10 --- unaffected
firefox-esr17 --- unaffected

People

(Reporter: gkw, Assigned: djvj)

References

()

Details

(5 keywords, Whiteboard: [adv-main18-])

Attachments

(2 files)

Attached file stack
x = []
y = new Set
z = (function () {
    y.__proto__
})
x.push(0)
wrap(y)
x.forEach(function () {
    for (var j = 0; j < 99999; j++) {
        z()
    }
})

asserts js debug shell on m-c changeset 58f3ccaa02b8 without any CLI arguments at Assertion failure: hasIonScript(), and crashes js opt shell at JSScript::ionScript

Seems like a null deref, but setting s-s to be safe and assuming sec-moderate.

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   109194:fa3d21b84a63
user:        Kannan Vijayan
date:        Wed Oct 03 22:55:24 2012 -0400
summary:     Bug 795803 - Enable ICing of JSNative and PropertyOp getters. (r=jandem)
http://www.lemonde.fr/
reproduced on Windows 7 32bit builds on 32bit, Linux 32bit and OSX 64bit.
OS: Mac OS X → All
Hardware: x86 → All
rhel6 32bit
bp-3d382dc2-9543-46cb-ba89-2541c2121006
[@ js::ion::IonCacheGetProperty::attachCallGetter ] 

win7 32bit
bp-99ccbfa5-97ca-4e7d-8fc6-aeaaa2121006
[@ js::ion::IonCacheGetProperty::attachCallGetter(JSContext*, js::ion::IonScript*, JSObject*, JSObject*, js::Shape const*, js::ion::SafepointIndex const*, void*) 

crash seems more reliable in opt builds for me. in debug builds I had to reload several times.
Nominating for tracking due to appearance in real world sites.

Even though the testcase seems to be a null deref, it seems to be crashing the browser with JIT code on the stack, so assuming worse-case and raising severity to sec-critical pending further analysis.
Attached patch Fix.Splinter Review
Using wrong ionScript to get the frameSize().  Don't want the source script for the cache, but the jit-script containing it, which is passed in as an argument.
Attachment #668724 - Flags: review?(jdemooij)
This is trivial enough to not really require an r?, but I can't push it in anyway since m-i is closed.  Putting it up for review, but I'll be pushing it in
if/when m-i re-opens.
Comment on attachment 668724 [details] [diff] [review]
Fix.

Review of attachment 668724 [details] [diff] [review]:
-----------------------------------------------------------------

Indeed, still better with a review ;)
Attachment #668724 - Flags: review?(jdemooij) → review+
Landed on m-c so closing.
Assignee: general → kvijayan
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
Have triggered another nightly (on 9f677c2bb33d) to pick up the fix for this, seeing how close we are to uplift :-)
just for reference another crash where this could be seen during the topsite scan was http://www.joy.cn
Status: RESOLVED → VERIFIED
JSBugMon: This bug has been automatically verified fixed.
Whiteboard: [adv-main18-]
Group: core-security
You need to log in before you can comment on or make changes to this bug.