Title: [264643] trunk/Source/_javascript_Core
Revision
264643
Author
[email protected]
Date
2020-07-20 22:16:51 -0700 (Mon, 20 Jul 2020)

Log Message

TryGetById clobberize rules are wrong.
https://bugs.webkit.org/show_bug.cgi?id=163834
<rdar://problem/65625807>

Reviewed by Keith Miller.

Theoretically, TryGetById can do the same things GetById does i.e. reify lazy
properties, read the stack, etc.  Hence, its clobberize rule should be clobberTop
just like GetById.  However, in practice, we don't currently use @tryGetById to
access anything on the stack (and probably never will).  But as a conservative
measure, we'll just treat TryGetById like it can.  In clobberize terms, this
means we declare TryGetById as doing read(World) (just like GetById) instead of
read(Heap).

* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
* dfg/DFGClobberize.h:
(JSC::DFG::clobberize):
* dfg/DFGClobbersExitState.cpp:
(JSC::DFG::clobbersExitState):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (264642 => 264643)


--- trunk/Source/_javascript_Core/ChangeLog	2020-07-21 00:53:46 UTC (rev 264642)
+++ trunk/Source/_javascript_Core/ChangeLog	2020-07-21 05:16:51 UTC (rev 264643)
@@ -1,3 +1,26 @@
+2020-07-20  Mark Lam  <[email protected]>
+
+        TryGetById clobberize rules are wrong.
+        https://bugs.webkit.org/show_bug.cgi?id=163834
+        <rdar://problem/65625807>
+
+        Reviewed by Keith Miller.
+
+        Theoretically, TryGetById can do the same things GetById does i.e. reify lazy
+        properties, read the stack, etc.  Hence, its clobberize rule should be clobberTop
+        just like GetById.  However, in practice, we don't currently use @tryGetById to
+        access anything on the stack (and probably never will).  But as a conservative
+        measure, we'll just treat TryGetById like it can.  In clobberize terms, this
+        means we declare TryGetById as doing read(World) (just like GetById) instead of
+        read(Heap).
+
+        * dfg/DFGAbstractInterpreterInlines.h:
+        (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+        * dfg/DFGClobberize.h:
+        (JSC::DFG::clobberize):
+        * dfg/DFGClobbersExitState.cpp:
+        (JSC::DFG::clobbersExitState):
+
 2020-07-20  Yusuke Suzuki  <[email protected]>
 
         Unreviewed, fix duplicate forward declaration introduced by merge conflict

Modified: trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h (264642 => 264643)


--- trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h	2020-07-21 00:53:46 UTC (rev 264642)
+++ trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h	2020-07-21 05:16:51 UTC (rev 264643)
@@ -3334,6 +3334,7 @@
     case TryGetById:
         // FIXME: This should constant fold at least as well as the normal GetById case.
         // https://bugs.webkit.org/show_bug.cgi?id=156422
+        clobberWorld();
         makeHeapTopForNode(node);
         break;
 

Modified: trunk/Source/_javascript_Core/dfg/DFGClobberize.h (264642 => 264643)


--- trunk/Source/_javascript_Core/dfg/DFGClobberize.h	2020-07-21 00:53:46 UTC (rev 264642)
+++ trunk/Source/_javascript_Core/dfg/DFGClobberize.h	2020-07-21 05:16:51 UTC (rev 264643)
@@ -655,6 +655,7 @@
     case GetByIdDirect:
     case GetByIdDirectFlush:
     case GetByValWithThis:
+    case TryGetById:
     case PutById:
     case PutByIdWithThis:
     case PutByValWithThis:
@@ -1304,11 +1305,6 @@
         return;
     }
 
-    case TryGetById: {
-        read(Heap);
-        return;
-    }
-
     case MultiGetByOffset: {
         read(JSCell_structureID);
         read(JSObject_butterfly);

Modified: trunk/Source/_javascript_Core/dfg/DFGClobbersExitState.cpp (264642 => 264643)


--- trunk/Source/_javascript_Core/dfg/DFGClobbersExitState.cpp	2020-07-21 00:53:46 UTC (rev 264642)
+++ trunk/Source/_javascript_Core/dfg/DFGClobbersExitState.cpp	2020-07-21 05:16:51 UTC (rev 264643)
@@ -85,6 +85,7 @@
     case FilterPutByIdStatus:
     case FilterInByIdStatus:
     case FilterDeleteByStatus:
+    case TryGetById:
         // These do clobber memory, but nothing that is observable. It may be nice to separate the
         // heaps into those that are observable and those that aren't, but we don't do that right now.
         // FIXME: https://bugs.webkit.org/show_bug.cgi?id=148440
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to