914601 - Assertion failure: ok, at ../vm/GlobalObject.h:415 with OOM and ParallelArray

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase asserts on mozilla-central revision c7cc85e13f7a (run with --fuzzing-safe --ion-eager):


oomAfterAllocations(51);
var p = new ParallelArray([1,2,3,4]);

Comment 1

[Christian Holler (:decoder)]

Attachment: [crash-signature] Machine-readable crash signature

Comment 2

[Christian Holler (:decoder)]

I'm seeing this assertion very frequently, but stacks vary. I'll try to figure out if there are more tests. At least one stack contains Intl instead of ParallelArray, others contain neither one nor the other.

Comment 3

[Christian Holler (:decoder)]

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

The first bad revision is:
changeset:   http://hg.mozilla.org/mozilla-central/rev/23dda916c3d0
user:        Shu-yu Guo
date:        Sat May 11 22:39:46 2013 -0700
summary:     Bug 860965 - Part 1: Copy 1D ParallelArray operations to Array. (r=luke,nmatsakis)

This iteration took 327.077 seconds to run.

Comment 4

[Jeff Walden [:Waldo]]

The assert is that a JS_DefineProperty in GlobalObject::getSelfHostedValue or whichever never fails.  Why we're asserting this, when the method has a perfectly reasonable bool failure return that could be propagated, I'm not entirely sure.

Comment 5

[Till Schneidereit [:till]]

The best explanation I can come up with is luke's request to add an assert that *getting* a self-hosted function succeeded: https://bugzilla.mozilla.org/show_bug.cgi?id=462300#c56

This then morphed (through emergent behavior nobody's responsible for) into the current weird construct. Will attach a patch to cut back the uncontrolled growth.

Comment 6

[Terrence Cole [:terrence]]

Thanks for taking this OOM bug, Till!

Comment 7

[Till Schneidereit [:till]]

Attachment: Remove non-required, crashing-on-oom assert in GlobalObject::getIntrinsicValue.

Just removing this should be fine and all that's needed

Comment 8

[Terrence Cole [:terrence]]

Comment on attachment 802967 [details] [diff] [review]
Remove non-required, crashing-on-oom assert in GlobalObject::getIntrinsicValue.

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

Yup. r=me

Comment 9

[Till Schneidereit [:till]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/38bcba92aa14

Thanks for the quick review.

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/38bcba92aa14