112974 - inheriting proto's map does not take into account JSCLASS_HAS_PRIVATE

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[John Bandhauer]

This is from the newsgroup.
news://news.mozilla.org:119/[e-mail]

I confirmed this. I'll attach a sample C file (can be put in place of js.c to
build easily). The problem does not show up if JS_InitStandardClasses is called
on the global before it is used as the proto because its slot count is changed
quite a bit from its initial state. But when using the global resolve hook stuff
the global is left in its initial state and the map is not sufficient for later
expectations as used in the second object. This same problem would occur if some
other plain JS object were used as the proto.

I'm going to wimp out and not propose a fix at this point.

Comment 1

[John Bandhauer]

Attachment: test C file

Comment 2

[Phil Schwartau]

Reassigning to Kenton; cc'ing Roger -

Comment 3

[Brendan Eich [:brendan]]

I'll help in 0.9.9.

/be

Comment 4

[Kenton Hanson (gone)]

targeting 9.9.

Comment 5

[Brendan Eich [:brendan]]

This is something I really should considef for mozilla1.0/js1.5.

/be

Comment 6

[Judson Valeski]

do we still want to go for this in 1.0.1? 

Comment 7

[Brendan Eich [:brendan]]

Moving out a bit.  The js1.5 release (after an rc5 release candidate) should
come off off the 1.0 branch on or before 1.0.2 freezing, I say.  The trunk and
branch should be in sync for js/src/js*.[chmt]* and build files.

/be

Comment 8

[Brendan Eich [:brendan]]

Attachment: proposed fix

Don't share the prototype's map if the new object's class and the prototype's
class do not have the same slot-allocating flag bits (JSCLASS_HAS_PRIVATE and
the reserved slots field).

This fixes jband's reduced testcase, and makes sense.  Looking for r= and sr=
pretty soon.  Thanks,

/be

Comment 9

[rogerl (gone)]

Comment on attachment 93330 [details] [diff] [review]
proposed fix

r=rogerl

Comment 10

[John Bandhauer]

Comment on attachment 93330 [details] [diff] [review]
proposed fix

sr=jband. A comment wouldn't hurt.

Comment 11

[Brendan Eich [:brendan]]

Fixed on the trunk.  Another one for the eventual js1.5 landing on the 1.0
branch....

/be

Comment 12

[Phil Schwartau]

Verified Fixed

On WinNT, Linux, I built the JS shell using jband's C testcase
above in place of js.c. Before the fix for this bug, I experienced:

WINNT
debug, opt shell both crashed on startup

LINUX 
debug shell crashed on startup (but opt shell did not: why?)


After this fix, the testcase passed; i.e. I got no crash on 
startup in either debug or optimized mode, on WinNT or Linux -