Bug 177523

Summary: Propagate hasBeenFlattenedBefore in Structure's transition constructor and fix our for-in caching to fail when the prototype chain has an object with a dictionary structure
Product: WebKit Reporter: Saam Barati <saam>
Component: JavaScriptCoreAssignee: Saam Barati <saam>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, commit-queue, fpizlo, ggaren, gskachkov, jfbastien, keith_miller, mark.lam, msaboff, rmorisset, ticaiolima, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: Safari Technology Preview   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch
none
perf results none

Saam Barati
Reported 2017-09-26 23:49:38 PDT
This will make it wrong. However, the reason I think it's correct just by the skin of its teeth today because we'll end up always flattening all structures in the prototype chain. Looking at normalizePrototypeChain, things might break with a JSProxy in the prototype chain. The reason things barely work today is I think that "hasBeenFlattenedBefore()" will always return false when you're a dictionary. The reason being, when we create a structure via its transition constructor, we don't propagate forward the "hasBeenFlattenedBefore" bit. The only way to make a dictionary structure is via a transition, hence, it'll never have that bit set. So, every time we ask a dictionary structure "hasBeenFlattenedBefore", it'll say no.
Attachments
patch (6.36 KB, patch)
2017-09-27 12:25 PDT, Saam Barati 		no flags
DetailsFormatted DiffDiff
perf results (97.41 KB, text/plain)
2017-09-27 15:13 PDT, Saam Barati 		no flags
Details
View All Add attachment proposed patch, testcase, etc.
Saam Barati
Comment 1 2017-09-26 23:50:20 PDT
I'm testing this hypothesis now by running all JSC stress tests with a CRASH() if hasBeenFlattenedBefore() ever returns true.
Saam Barati
Comment 2 2017-09-27 12:25:27 PDT
Created attachment 321996 [details] patch
Saam Barati
Comment 3 2017-09-27 12:26:17 PDT
I'm going to run benchmarks before landing
Mark Lam
Comment 4 2017-09-27 12:31:28 PDT
Comment on attachment 321996 [details] patch r=me
Saam Barati
Comment 5 2017-09-27 15:13:11 PDT
Created attachment 322027 [details] perf results Neutral or perhaps 0.5% progressed.
WebKit Commit Bot
Comment 6 2017-09-27 17:44:32 PDT
Comment on attachment 321996 [details] patch Clearing flags on attachment: 321996 Committed r222590: <http://trac.webkit.org/changeset/222590>
WebKit Commit Bot
Comment 7 2017-09-27 17:44:34 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 8 2017-09-27 17:45:55 PDT
<rdar://problem/34702967>
Note You need to log in before you can comment on or make changes to this bug.