623474 - String length PIC crash [@ js::mjit::EnterMethodJIT]

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jesse Ruderman]

./js -m
for (var j=0;j<2;++j) { (function(o){o.length})(String.prototype); }

Crash, null-deref in methodjit generated code.

The first bad revision is:
changeset:   feb28ec64b74
user:        Tom Schuster (evilpies)
date:        Thu Dec 30 13:25:03 2010 -0600
summary:     Bug 557353 - JM: PIC for object-wrapped string length.  r=dvander

Comment 1

[Tom S [:evilpie]]

Attachment: Fix

The fix itself is easy, String.prototype doesn't use inline slots, so we first need to load obj->slots and then slots[0].

But i don't like to make the general case slower, String.prototype should just use inline slots, too.

Comment 3

[David Anderson [:dvander] - inactive, e-mail if emergency]

Comment on attachment 501662 [details] [diff] [review]
Fix

The double load should be fine, thanks for fixing. I'll check this in with the testcase tomorrow.

We can optimize it better in a follow-up bug.

Comment 4

[David Mandelin [:dmandelin]]

It shouldn't hurt to set the assignee since it's already done. :-)

Comment 5

[David Anderson [:dvander] - inactive, e-mail if emergency]

http://hg.mozilla.org/tracemonkey/rev/423d37840edf

Comment 6

[Brendan Eich [:brendan]]

What's the followup bug? Best to file and cite in a FIXME: or risk of not filing goes up. rs=me on FIXME comment additions, any time.

/be

Comment 7

[Chris Leary [:cdleary] (not checking bugmail)]

http://hg.mozilla.org/mozilla-central/rev/423d37840edf

Despite resolution, note brendan's request for a followup bug in comment 6!

Comment 8

[Christian Holler (:decoder)]

A testcase for this bug was automatically identified at js/src/jit-test/tests/jaeger/bug623474.js.