452189 - regression (from firefox 2): prototype setters not called by inline cache for [[put]]

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P2)

Description

[Geoffrey Garen]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_5_4; en-us) AppleWebKit/525.18 (KHTML, like Gecko) Version/3.1.1 Safari/525.20
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.1) Gecko/2008070206 Firefox/3.0.1

If a setter is added to the prototype chain after a body of code has specialized for [[put]], the specialized body of code will fail to call the setter. 

This is a regression from Firefox 2.

See attached test case (coming soon!).

Reproducible: Always

Steps to Reproduce:
Load attached test case.
Actual Results:  
"FAIL"

Expected Results:  
"PASS"

Comment 1

[Geoffrey Garen]

Attachment: test case

Comment 2

[Ria Klaassen (not reading all bugmail)]

This testcase regressed between 2008020700 (works) and 2008020800 (fails).

Comment 3

[Ria Klaassen (not reading all bugmail)]

Smaller range: 
http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=2008-02-07+14%3A00&maxdate=2008-02-07+16%3A00

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Proposed fix

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: testcase for the shell

This is a simplified testcase for the shell.

Comment 6

[Brendan Eich [:brendan]]

Fixed on tm:

http://hg.mozilla.org/tracemonkey/index.cgi/rev/4714cee8d904

and on m-c:

http://hg.mozilla.org/mozilla-central/index.cgi/rev/a2ae40e570a7

/be

Comment 7

[Mike Shaver (:shaver emeritus)]

Let's get this into the branch as a regression fix, if it bakes well on trunk for a bit.

Comment 8

[Brendan Eich [:brendan]]

Sorry! Comment 6 is in the wrong bug.

/be

Comment 9

[Daniel Veditz [:dveditz]]

This bug is not fixed on trunk despite an old patch, and apparently missed "baking" in the beta. Is it realistic to block a branch release for it?

Comment 10

[Brendan Eich [:brendan]]

(In reply to comment #9)
> This bug is not fixed on trunk despite an old patch, and apparently missed
> "baking" in the beta. Is it realistic to block a branch release for it?

Certainly not. Should have a patch for the next next release.

/be

Comment 11

[Brendan Eich [:brendan]]

Comment on attachment 335591 [details] [diff] [review]
Proposed fix

I hope to fix this differently.

/be

Comment 12

[Brendan Eich [:brendan]]

Attachment: fix

Comment 13

[Brendan Eich [:brendan]]

Fixed in tm:

http://hg.mozilla.org/tracemonkey/rev/cffb2a5d4489

/be

Comment 14

[Brendan Eich [:brendan]]

Fixed in m-c:

http://hg.mozilla.org/mozilla-central/rev/8c9a6d851018

/be

Comment 15

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

This checkin is in a range suspected to have caused a TXul regression on OSX:

Regression: Txul increase from 312.789 to 326.947 (4.53%) on Sun May 10 16:20:00 2009. Mac105 1.9.2 build 20090510132032
   http://graphs.mozilla.org/#show=794371,794398,794384&sel=1241813896,1242159496
   http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=6ea54bfd7c28&tochange=a6068cf686fe

Comment 16

[Boris Zbarsky [:bzbarsky]]

Filed bug 492355 to track that regression.

Comment 17

[Igor Bukanov]

(In reply to comment #12)
> Created an attachment (id=376377) [details]
> fix

Hm, this purges just the property cache for the current thread. It does not take care about the cache embedded into the jited code or on other threads.

Comment 18

[Brendan Eich [:brendan]]

(In reply to comment #17)
> (In reply to comment #12)
> > Created an attachment (id=376377) [details] [details]
> > fix
> 
> Hm, this purges just the property cache for the current thread. It does not
> take care about the cache embedded into the jited code or on other threads.

See bug 492355.

/be

Comment 19

[Robert Sayre]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/d223c3b7f6b3