Bug 123788

Summary: GetById->GetByOffset and PutById->PutByOffset folding should mark haveStructures since it may result in structure transition watchpoints
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: JavaScriptCoreAssignee: Filip Pizlo <fpizlo>
Status: RESOLVED FIXED    
Severity: Normal CC: barraclough, ggaren, mark.lam, mhahnenberg, msaboff, oliver, sam
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
the patch ggaren: review+

Filip Pizlo
Reported 2013-11-04 22:37:14 PST
This is totally asymptomatic as far as I can tell even though it's a violation of IR semantics. It just sort of "works" because this only arises if you already had a transition watchpoint for that structure, which usually ends up being good enough since watchpoints invalidate the whole function.
Attachments
the patch (2.26 KB, patch)
2013-11-04 22:44 PST, Filip Pizlo 		ggaren: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Filip Pizlo
Comment 1 2013-11-04 22:44:03 PST
Created attachment 216005 [details] the patch
Geoffrey Garen
Comment 2 2013-11-05 10:37:00 PST
Comment on attachment 216005 [details] the patch r=me The phrase "have structures" does not communicate this detail to me. I think by "have" you mean "speculated about, in a way which is invalid after an operation that clobbers the world". Maybe a better name would be "hasStructureSpeculations" or "hasStructuresToClobber" or "hasClobberableStructures".
Filip Pizlo
Comment 3 2013-11-05 10:45:47 PST
(In reply to comment #2) > (From update of attachment 216005 [details]) > r=me > > The phrase "have structures" does not communicate this detail to me. I think by "have" you mean "speculated about, in a way which is invalid after an operation that clobbers the world". Maybe a better name would be "hasStructureSpeculations" or "hasStructuresToClobber" or "hasClobberableStructures". Fair point. I'll try to think of a better name.
Filip Pizlo
Comment 4 2013-11-05 10:51:17 PST
Landed in http://trac.webkit.org/changeset/158680
Note You need to log in before you can comment on or make changes to this bug.