787709 - Crash [@ js::ArrayBufferObject::removeFinalizedView] or "Assertion failure: linkObj,"

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: stack

for each(var x in DataView(eval("y = ArrayBuffer();"))) {};
function f() {
	gc();
}
f();

asserts js debug shell on m-c changeset c64a9f342156 without any CLI arguments at Assertion failure: linkObj,

The testcase crashes js opt shell as well, at js::ArrayBufferObject::removeFinalizedView when pasted into the shell.

s-s because gc is involved and also because 0xa0 is being accessed.

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   104015:804d74e217e3
user:        Steve Fink
date:        Thu Aug 23 21:29:42 2012 -0700
summary:     Bug 720949 - Add JSAPI for transferring ArrayBuffer contents

Comment 1

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

I wonder why there was a tab-character in the testcase... Anyway here's an untabbed version:

for each(var x in DataView(eval("y = ArrayBuffer();"))) {};
function f() {
    gc();
}
f();

Comment 2

[Christian Holler (:decoder)]

JSBugMon: The testcase found in this bug no longer reproduces (tried revision 5f7e3a8d6640).

Comment 3

[Christian Holler (:decoder)]

JSBugMon: Fix Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first good revision is:
changeset:   104414:22e9cbb8bb92
user:        Steve Fink
date:        Wed Sep 05 15:00:39 2012 -0700
summary:     Bug 787775 - Register DataViews with their ArrayBuffers upon creation. r=luke

Comment 4

[Christian Holler (:decoder)]

Steve, is this a dup of 787775? If so, can we add the testcase here to the test suite at least?

Comment 5

[Steve Fink [:sfink] [:s:]]

(In reply to Christian Holler (:decoder) from comment #4)
> Steve, is this a dup of 787775? If so, can we add the testcase here to the
> test suite at least?

Yes, it is. I'll add something similar to the test suite. This works:

./js -e 'var ab = new ArrayBuffer(4); var dv = new DataView(ab); dv = 1; gc()'

(All you need to do is create a DataView on an ArrayBuffer, and gc the DataView without gcing the ArrayBuffer.)

Comment 6

[Steve Fink [:sfink] [:s:]]

Attachment: Crash test for bug 787775

I verified that this crashes before, does not crash after.

Comment 7

[Steve Fink [:sfink] [:s:]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/e2fe8fa1f29b

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e2fe8fa1f29b

Comment 9

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Marking VERIFIED as testcase has landed in the testsuite.

Comment 10

[Daniel Veditz [:dveditz]]

If this is a duplicate of 787775 and like that one a regression from bug 720949 then is Firefox 17 really "affected"? If so is it too late to uplift the patch in bug 787775 to Firefox 17 Beta?

Comment 11

[Lukas Blakk [:lsblakk] use ?needinfo]

(In reply to Daniel Veditz [:dveditz] from comment #10)
> If this is a duplicate of 787775 and like that one a regression from bug
> 720949 then is Firefox 17 really "affected"? If so is it too late to uplift
> the patch in bug 787775 to Firefox 17 Beta?

It doesn't look like Firefox 17 could be affected here since both bug 787775 and 720949 appear to only affect 18. Minusing for tracking unless someone has a different read on this.

Comment 12

[Steve Fink [:sfink] [:s:]]

That is correct; 17 is not affected.

Comment 13

[Daniel Veditz [:dveditz]]

Since the bug this was dupe'd to was not a security bug I'd rather mark this issue "fixed" and "depends on" the dupe.

Doesn't need to remain hidden because it never even made it as far as Beta releases.

Comment 14

[Ioana (away)]

Marking as qa- per comment 9.