Bug 238349

Summary: AI should not set the structure for ObjectCreate
Product: WebKit Reporter: Justin Michaud <justin_michaud>
Component: JavaScriptCoreAssignee: Justin Michaud <justin_michaud>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, keith_miller, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: Safari 15   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
saam: review-
Patch
saam: review-
Patch
none
[fast-cq] Patch saam: review+

Justin Michaud
Reported 2022-03-24 14:51:36 PDT
The AbstractInterpreter should not set the structure for ObjectCreate because it might change by the time the constant folding phase runs if the structure cache is cleared.
Attachments
Patch (2.58 KB, patch)
2022-03-24 14:53 PDT, Justin Michaud 		saam: review-
DetailsFormatted DiffDiff
Patch (3.12 KB, patch)
2022-03-25 11:49 PDT, Justin Michaud 		saam: review-
DetailsFormatted DiffDiff
Patch (3.03 KB, patch)
2022-03-25 13:09 PDT, Justin Michaud 		no flags
DetailsFormatted DiffDiff
[fast-cq] Patch (2.95 KB, patch)
2022-03-25 13:10 PDT, Justin Michaud 		saam: review+
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Justin Michaud
Comment 1 2022-03-24 14:53:50 PDT
Created attachment 455689 [details] Patch
Yusuke Suzuki
Comment 2 2022-03-24 14:59:41 PDT
Comment on attachment 455689 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=455689&action=review > Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:-3147 > - setForNode(node, structure); We should continue setting a structure for nullPrototypeObjectStructure.
Saam Barati
Comment 3 2022-03-24 15:00:59 PDT
Comment on attachment 455689 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=455689&action=review > Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:3138 > break; You can’t break here. You need the below code to run and set our type. I also suggest reworking this patch to never even bother looking up a structure and just always mark things as shoyldTryCobstantFolding or instead to keep the code as it used to be
Radar WebKit Bug Importer
Comment 4 2022-03-25 09:53:04 PDT
<rdar://problem/90842529>
Mark Lam
Comment 5 2022-03-25 11:35:06 PDT
<rdar://problem/90838071>
Justin Michaud
Comment 6 2022-03-25 11:49:56 PDT
Created attachment 455790 [details] Patch
Yusuke Suzuki
Comment 7 2022-03-25 12:05:08 PDT
Why not moving StructureCache from VM to JSGlobalObject? This cache is used for objects' structures. Thus each structure has its tied JSGlobalObject.
Saam Barati
Comment 8 2022-03-25 12:43:33 PDT
Comment on attachment 455790 [details] Patch We're discussing on slack a better approach
Justin Michaud
Comment 9 2022-03-25 13:09:08 PDT
Created attachment 455795 [details] Patch
Saam Barati
Comment 10 2022-03-25 13:10:06 PDT
Comment on attachment 455795 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=455795&action=review > Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:3131 > - didFoldClobberWorld(); > + clobberWorld(); this shouldn't change.
Justin Michaud
Comment 11 2022-03-25 13:10:28 PDT
Created attachment 455796 [details] [fast-cq] Patch
Saam Barati
Comment 12 2022-03-25 13:13:15 PDT
Comment on attachment 455796 [details] [fast-cq] Patch r=me
Yusuke Suzuki
Comment 13 2022-03-25 13:15:04 PDT
r=me too.
Mark Lam
Comment 14 2022-03-25 14:52:34 PDT
Patch landed in r291891: <http://trac.webkit.org/r291891>.
Note You need to log in before you can comment on or make changes to this bug.