Bug 104623

Summary: Named lookups on HTML documents produce inconsistent results in JavaScriptCore bindings
Product: WebKit Reporter: Boris Zbarsky <bzbarsky>
Component: WebCore JavaScriptAssignee: Filip Pizlo <fpizlo>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, ap, fpizlo, haraken, japhet, koivisto, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Testcase
none
the patch ggaren: review+

Boris Zbarsky
Reported 2012-12-10 18:27:53 PST
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.
Attachments
Testcase (586 bytes, text/html)
2012-12-10 18:28 PST, Boris Zbarsky 		no flags
Details
the patch (10.90 KB, patch)
2012-12-13 18:32 PST, Filip Pizlo 		ggaren: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Boris Zbarsky
Comment 1 2012-12-10 18:28:26 PST
Created attachment 178683 [details] Testcase
Alexey Proskuryakov
Comment 2 2012-12-11 10:56:43 PST
Ick. Phil, do you think that it's yours?
Filip Pizlo
Comment 3 2012-12-11 12:47:30 PST
(In reply to comment #2) > Ick. > > Phil, do you think that it's yours? Yup. That's probably a DFG bug.
Filip Pizlo
Comment 4 2012-12-12 15:17:33 PST
(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
Comment 5 2012-12-12 16:17:18 PST
(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
Comment 6 2012-12-13 18:32:33 PST
Created attachment 179396 [details] the patch
Geoffrey Garen
Comment 7 2012-12-13 18:36:19 PST
Comment on attachment 179396 [details] the patch r=me Please land with the test attached here.
Filip Pizlo
Comment 8 2012-12-13 19:28:22 PST
Landed in http://trac.webkit.org/changeset/137700
Kentaro Hara
Comment 9 2012-12-13 20:06:19 PST
Rebaselined run-bindings-tests in r137704.
Filip Pizlo
Comment 10 2012-12-13 20:20:58 PST
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
Comment 11 2012-12-20 03:43:16 PST
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
Comment 12 2012-12-20 03:48:25 PST
Bug 105526
Note You need to log in before you can comment on or make changes to this bug.