500528 - Bug in new property cache scheme causes incorrect property lookup

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Jason Orendorff [:jorendorff]]

// a <-- b1 <-- c1
// a <-- b2 <-- c2
a = Object.prototype;
function B1(){}
function C1(){}
C1.prototype = b1 = new B1;
c1 = new C1;
function B2(){}
function C2(){}
C2.prototype = b2 = new B2;
c2 = new C2;

b2.x = 2;  // This does not change the shape of c2.
a.x = 1;

// The first pass through the loop below will generate a property cache entry
// for c1.x, pointing to a.
//
// We require, for correctness, that shapeOf(c1) != shapeOf(c2), since they
// have differently-shaped proto chains; but:
print(shapeOf(c1) == shapeOf(c2));  // this prints true
//
// The second pass through the loop erroneously uses the cache entry and thus
// fails to find the shadowing property on b2.

arr = [c1, c2];
out = [];
for (i=0; i<arr.length; i++)
    out[i] = arr[i].x;

assertEq(""+out, "1,2");

Comment 1

[Brendan Eich [:brendan]]

Thanks, I knew you had this when we were chatting on IRC ;-).

Hungry for optimization, soundness takes a step back. Need to restore balance in the Force...

/be

Comment 2

[Brendan Eich [:brendan]]

Attachment: quick fix

Not sure about this but the intuition is that different prototype objects should be uniquely shaped by the time they are used as delegates by new instances.

/be

Comment 3

[Jason Orendorff [:jorendorff]]

Contrary to our IRC discussion yesterday, it *is* possible to get empty intermediates on the prototype chain, so you can make a whole tree of objects all sharing the same scope:

function A(){}
A.prototype = {};
function B(){}
B.prototype = new A;
function C(){}
C.prototype = new B;

print(shapeOf(A.prototype));
print(shapeOf(B.prototype));
print(shapeOf(C.prototype));
print(shapeOf(new C));

// Output: 27 27 27 27

Actually, the test case in comment 0 does this too. So I'm not sure the "quick fix" will actually work. Haven't tried it.

Comment 4

[Brendan Eich [:brendan]]

Comment on attachment 385296 [details] [diff] [review]
quick fix

This is crap.

/be

Comment 5

[Brendan Eich [:brendan]]

Fixed by back-out of patch for bug 497789.

/be

Comment 6

[Mike Beltzner [:beltzner, not reading bugmail]]

Mass change: adding fixed1.9.2 keyword

(This bug was identified as a mozilla1.9.2 blocker which was fixed before the mozilla-1.9.2 repository was branched (August 13th, 2009) as per this query: http://is.gd/2ydcb - if this bug is not actually fixed on mozilla1.9.2, please remove the keyword. Apologies for the bugspam)