WebKit Bugzilla
Attachment 362758 Details for
Bug 194953
: DFGBytecodeParser should not declare that a node won't clobberExit if DFGFixupPhase can later declare it does clobberExit
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
patch194953 (text/plain), 4.45 KB, created by
Robin Morisset
on 2019-02-22 13:29:09 PST
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Robin Morisset
Created:
2019-02-22 13:29:09 PST
Size:
4.45 KB
patch
obsolete
>Index: JSTests/ChangeLog >=================================================================== >--- JSTests/ChangeLog (revision 241955) >+++ JSTests/ChangeLog (working copy) >@@ -1,3 +1,15 @@ >+2019-02-22 Robin Morisset <rmorisset@apple.com> >+ >+ DFGBytecodeParser should not declare that a node won't clobberExit if DFGFixupPhase can later declare it does clobberExit >+ https://bugs.webkit.org/show_bug.cgi?id=194953 >+ <rdar://problem/47595253> >+ >+ Reviewed by Saam Barati. >+ >+ I could not make this work without the infinite loop, so I am using a watchdog to be able to use it as a regression test. >+ >+ * stress/has-indexed-property-with-worsening-array-mode.js: Added. >+ > 2019-02-19 Joseph Pecoraro <pecoraro@apple.com> > > Web Inspector: Improve ES6 Class instances in Heap Snapshot instances view >Index: JSTests/stress/has-indexed-property-with-worsening-array-mode.js >=================================================================== >--- JSTests/stress/has-indexed-property-with-worsening-array-mode.js (nonexistent) >+++ JSTests/stress/has-indexed-property-with-worsening-array-mode.js (working copy) >@@ -0,0 +1,6 @@ >+//@ requireOptions("--watchdog=1000", "--watchdog-exception-ok", "--useMaximalFlushInsertionPhase=1") >+// This test only seems to reproduce the issue when it runs in an infinite loop. So we use the watchdog to time it out. >+for (let x in [0]) { >+ break >+} >+while(1); >Index: Source/JavaScriptCore/ChangeLog >=================================================================== >--- Source/JavaScriptCore/ChangeLog (revision 241955) >+++ Source/JavaScriptCore/ChangeLog (working copy) >@@ -1,3 +1,40 @@ >+2019-02-22 Robin Morisset <rmorisset@apple.com> >+ >+ DFGBytecodeParser should not declare that a node won't clobberExit if DFGFixupPhase can later declare it does clobberExit >+ https://bugs.webkit.org/show_bug.cgi?id=194953 >+ <rdar://problem/47595253> >+ >+ Reviewed by Saam Barati. >+ >+ For each node that >+ (a) may or may not clobberExit depending on their arrayMode >+ (b) and get their arrayMode from profiling information in DFGBytecodeParser >+ (c) and can have their arrayMode refined by DFGFixupPhase, >+ We must make sure to be conservative in the DFGBytecodeParser and treat it as if it unconditionnally clobbered the exit. >+ Otherwise we will hit a validation failure after fixup if the next node was marked ExitValid and exits to the same semantic origin. >+ >+ The list of nodes that fit (a) is: >+ - StringCharAt >+ - HasIndexProperty >+ - GetByVal >+ - PutByValDirect >+ - PutByVal >+ - PutByValAlias >+ - GetIndexedPropertyStorage >+ >+ Out of these, the following also fit (b) and (c): >+ - HasIndexedProperty >+ - GetByVal >+ - PutByValDirect >+ - PutByVal >+ >+ GetByVal already had "m_exitOK = false; // GetByVal must be treated as if it clobbers exit state, since FixupPhase may make it generic." >+ So we just have to fix the other three the same way. >+ >+ * dfg/DFGByteCodeParser.cpp: >+ (JSC::DFG::ByteCodeParser::parseBlock): >+ (JSC::DFG::ByteCodeParser::handlePutByVal): >+ > 2019-02-22 Yusuke Suzuki <ysuzuki@apple.com> > > Unreviewed, build fix after r241954 >Index: Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp >=================================================================== >--- Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp (revision 241955) >+++ Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp (working copy) >@@ -6834,6 +6834,7 @@ > addVarArgChild(property); > addVarArgChild(nullptr); > Node* hasIterableProperty = addToGraph(Node::VarArg, HasIndexedProperty, OpInfo(arrayMode.asWord()), OpInfo(static_cast<uint32_t>(PropertySlot::InternalMethodType::GetOwnProperty))); >+ m_exitOK = false; // HasIndexedProperty must be treated as if it clobbers exit state, since FixupPhase may make it generic. > set(bytecode.m_dst, hasIterableProperty); > NEXT_OPCODE(op_has_indexed_property); > } >@@ -7212,6 +7213,7 @@ > addVarArgChild(0); // Leave room for property storage. > addVarArgChild(0); // Leave room for length. > addToGraph(Node::VarArg, isDirect ? PutByValDirect : PutByVal, OpInfo(arrayMode.asWord()), OpInfo(0)); >+ m_exitOK = false; // PutByVal and PutByValDirect must be treated as if they clobber exit state, since FixupPhase may make them generic. > } > } >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 194953
:
362738
| 362758