Bug 104623 – Named lookups on HTML documents produce inconsistent results in JavaScriptCore bindings

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Boris Zbarsky]

BUILD: Current WebKit nightly, though Safari 6 has the same behavior

STEPS TO REPRODUCE: Load attached testcase

EXPECTED RESULTS: The output does not depend on the iteration count

ACTUAL RESULTS: Once the iteration counts get high enough, the output changes. Looks like an inline cache that's not being invalidated when shadowing properties are introduced on the document via the name getter or something.

I have no idea whether this belongs in "HTML DOM" or "JavaScriptCore"; it's sort of about the interaction between the two.  Please feel free to move as needed.

As a note, Gecko+SpiderMonkey has all sorts of weird around this too.  I was investigating what other UAs did when I ran into this behavior in WebKit+JavaScriptCore.

[Boris Zbarsky]

Created attachment 178683 [details]
Testcase

[Alexey Proskuryakov]

Ick.

Phil, do you think that it's yours?

[Filip Pizlo]

(In reply to comment #2)
> Ick.
> 
> Phil, do you think that it's yours?

Yup. That's probably a DFG bug.

[Filip Pizlo]

(In reply to comment #3)
> (In reply to comment #2)
> > Ick.
> > 
> > Phil, do you think that it's yours?
> 
> Yup. That's probably a DFG bug.

Definitely a DFG or JIT bug.  Disabling the JIT makes this produce same results regardless of iteration count.

[Filip Pizlo]

(In reply to comment #4)
> (In reply to comment #3)
> > (In reply to comment #2)
> > > Ick.
> > > 
> > > Phil, do you think that it's yours?
> > 
> > Yup. That's probably a DFG bug.
> 
> Definitely a DFG or JIT bug.  Disabling the JIT makes this produce same results regardless of iteration count.

It's a JIT prototype chain inline caching bug.  We were completely ignoring the possibility of a prototype having a GetOwnPropertySlot trap.  Boo.

[Filip Pizlo]

Created attachment 179396 [details]
the patch

[Geoffrey Garen]

Comment on attachment 179396 [details]
the patch

r=me

Please land with the test attached here.

[Filip Pizlo]

Landed in http://trac.webkit.org/changeset/137700

[Kentaro Hara]

Rebaselined run-bindings-tests in r137704.

[Filip Pizlo]

This caused massive crashes because I only tested in release mode and failed to catch a bad assert.  That's now fixed by http://trac.webkit.org/changeset/137705.

[Antti Koivisto]

This seems to have caused massive (>50%) performance regressions in a number of Bindings microbenchmarks:

Bindings/get-element-by-id
Bindings/get-elements-by-tag-name
Bindings/create-element

http://webkit-perf.appspot.com/graph.html#tests=[[3030063,2001,32196],[2863935,2001,32196],[2971082,2001,32196]]&sel=1355398469646,1355548767856.7356&displayrange=7&datatype=running

[Antti Koivisto]

Bug 105526