851673 - Assertion failure: v.isUndefined(), at jsarray.cpp

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

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

Attachment: stack

x = (/x/).exec();
y = wrapWithProto((new WeakMap), x);
y.length = 7;
Array.prototype.push.call(y, 1);
Array.prototype.reverse.call(y);

asserts js debug shell on m-c changeset 8f5b1f9f5804 without any CLI arguments at Assertion failure: v.isUndefined(),

Comment 1

[Christian Holler (:decoder)]

JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.

Comment 2

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

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   124725:5d58b6d250d5
user:        Jeff Walden
date:        Mon Mar 04 09:41:11 2013 -0800
summary:     Bug 843402 - Make all always-inlined methods just inline, and let the compiler choose how to optimize.  r=http://perf.snarkfest.net/compare-talos/index.html?oldRevs=7ad145f9b092&newRev=6be7cad95d97&submit=true is a wash

Waldo, is this bug a likely regressor?

Comment 3

[Jeff Walden [:Waldo]]

Not in the least.  :-)  I'll take a look at this anyway, tho, since you've flagged me.

Comment 4

[Jeff Walden [:Waldo]]

I can't reproduce this on linux64, on that rev.  It might be OS X-only.  Probably a compiler bug, if so, and if this wasn't a false alarm.

Comment 5

[Jeff Walden [:Waldo]]

*Can* reproduce with clang, linux64, --enable-debug --enable-optimize.  Good times.

Comment 6

[Jeff Walden [:Waldo]]

Attachment: Patch

The fickle finger of fate points at bholley!

Comment 7

[Jeff Walden [:Waldo]]

Looks like this dates to bug 778085, and the INVOKE_HANDLER_ON_PROTOTYPE stuff.  (Incidentally this is why the ES6 proxy MOP puts the prototype-walking *not* in every hook.  :-) )  Judging by the diff of the relevant revision on jsproxy.cpp, this is the only two-outparam method that needs fixing.

Comment 8

[Jeff Walden [:Waldo]]

Oh, and I overspoke when I said compiler bug.  :-)  It's undefined behavior, no compiler bug in sight.  I guess making the optimizer have to consider whether to inline the Value methods perturbed it enough, for clang, to make behavior actually go visibly wrong.

Comment 9

[Bobby Holley (:bholley)]

Comment on attachment 725669 [details] [diff] [review]
Patch

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

Should we just set *present to false at the beginning of the function? r=bholley either way.

Comment 10

[Jeff Walden [:Waldo]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/058f18f1426b

I prefer leaving on-success outparam setting til the last minute, ideally adjacent to the successful-return case that it accompanies.  And this code should go away as we improve our objectops/proxy APIs to be more spec-like.

Comment 11

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/058f18f1426b