Bug 182549

Summary: put_to_scope/get_from_scope should not cache lexical scopes when expecting a global object
Product: WebKit Reporter: Keith Miller <keith_miller>
Component: New BugsAssignee: Keith Miller <keith_miller>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, jfbastien, mark.lam, msaboff, saam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=184001
Attachments:
Description Flags
Patch saam: review+

Keith Miller
Reported 2018-02-06 13:41:37 PST
put_to_scope/get_from_scope should not cache lexical scopes when expecting a global object
Attachments
Patch (9.97 KB, patch)
2018-02-06 13:54 PST, Keith Miller 		saam: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Keith Miller
Comment 1 2018-02-06 13:54:02 PST
Created attachment 333214 [details] Patch
Saam Barati
Comment 2 2018-02-06 14:03:43 PST
Comment on attachment 333214 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=333214&action=review > Source/JavaScriptCore/jit/JITPropertyAccess.cpp:861 > + emitLoadWithStructureCheck(scope, structureSlot); // Structure check covers var injection since we don't cache structures for anything but the GlobalObject. Also: this should say that resolve_scope does the check for var injection firing.
Mark Lam
Comment 3 2018-02-06 14:04:06 PST
<rdar://problem/36189995>
Saam Barati
Comment 4 2018-02-06 14:05:38 PST
Comment on attachment 333214 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=333214&action=review > Source/JavaScriptCore/ChangeLog:12 > + in the DFG, does not follow this same assumption so we could no comma needed > Source/JavaScriptCore/ChangeLog:17 > + GlobalPropertyWithVarInjectionChecks. or GlobalProperty
Keith Miller
Comment 5 2018-02-06 14:14:50 PST
Comment on attachment 333214 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=333214&action=review >> Source/JavaScriptCore/ChangeLog:12 >> + in the DFG, does not follow this same assumption so we could > > no comma needed Fixed. >> Source/JavaScriptCore/ChangeLog:17 >> + GlobalPropertyWithVarInjectionChecks. > > or GlobalProperty I don't think that was possible before? I can update the sentence though. >> Source/JavaScriptCore/jit/JITPropertyAccess.cpp:861 >> + emitLoadWithStructureCheck(scope, structureSlot); // Structure check covers var injection since we don't cache structures for anything but the GlobalObject. > > Also: this should say that resolve_scope does the check for var injection firing. Fixed.
Keith Miller
Comment 6 2018-02-06 14:42:31 PST
Committed r228193: <https://trac.webkit.org/changeset/228193>
JF Bastien
Comment 7 2018-02-23 16:59:43 PST
Comment on attachment 333214 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=333214&action=review > Source/JavaScriptCore/runtime/Options.h:253 > + v(bool, enableJITDebugAssetions, !ASSERT_DISABLED, Normal, nullptr) \ Typo enableJITDebugAssetions
Saam Barati
Comment 8 2018-03-25 22:35:47 PDT
(In reply to JF Bastien from comment #7) > Comment on attachment 333214 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=333214&action=review > > > Source/JavaScriptCore/runtime/Options.h:253 > > + v(bool, enableJITDebugAssetions, !ASSERT_DISABLED, Normal, nullptr) \ > > Typo enableJITDebugAssetions https://bugs.webkit.org/show_bug.cgi?id=184001
Note You need to log in before you can comment on or make changes to this bug.