1095870 - Assertion failure: setPropStub->holderShape() != holder->lastProperty() || setPropStub->shape() != receiverShape (Why didn't we end up using this stub?), at jit/BaselineIC.cpp

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

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

for (var j = 0; j < 2; j++) {
    isPrototypeOf().__proto__ = 0
}

asserts js debug shell on m-c changeset eb0d3b3c0b22 with --ion-eager --no-threads at Assertion failure: setPropStub->holderShape() != holder->lastProperty() || setPropStub->shape() != receiverShape (Why didn't we end up using this stub?), at jit/BaselineIC.cpp.

Debug configure options:

CC="clang -Qunused-arguments" CXX="clang++ -Qunused-arguments" AR=ar AUTOCONF=/usr/local/Cellar/autoconf213/2.13/bin/autoconf213 sh /Users/skywalker/trees/mozilla-central/js/src/configure --target=x86_64-apple-darwin12.5.0 --enable-debug --enable-optimize --enable-nspr-build --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests

=== Tinderbox Build Bisection Results by autoBisect ===

The "good" changeset has the timestamp "20141106201809" and the hash "e70ad67d1c7b".
The "bad" changeset has the timestamp "20141106204315" and the hash "7bf195ca830a".

Likely regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=e70ad67d1c7b&tochange=7bf195ca830a

Boris, is bug 1089050 a possible regressor?

Comment 1

[Boris Zbarsky [:bzbarsky]]

> Boris, is bug 1089050 a possible regressor?

Sure, since it added the assert.

Here's a simpler testcase:

for (var j = 0; j < 2; j++) {
    (false).__proto__ = 0
}

Comment 2

[Boris Zbarsky [:bzbarsky]]

So I bet the reason we didn't end up using the stub is that the stub sees the primitive value.  Then we skip the stub, land in DoSetPropFallback, try to generate a new stub, and end up in this code.

I can loosen up the assert for objects that are wrappers around primitives, but it seems more correct to me to not generate setter stubs at all when lhs is a primitive....

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Don't generate baseline set IC stubs for primitive 'this' values, since all the stubs only work on objects

Comment 4

[Jan de Mooij [:jandem]]

Comment on attachment 8519382 [details] [diff] [review]
Don't generate baseline set IC stubs for primitive 'this' values, since all the stubs only work on objects

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

So in this case we attached a stub that will never hit? Nice find.

Comment 5

[Boris Zbarsky [:bzbarsky]]

> So in this case we attached a stub that will never hit?

Well, it'll hit if an actual Boolean object comes through.  But in practice, that's never, yes.

Comment 6

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

Attachment: stack for testcase in comment 1

Add missing stack.

Comment 7

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ca8e506e5be0

Comment 8

[Ryan VanderMeulen [:RyanVM]]

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