Attachment 361210 Details for Bug 194298 – proposed patch.

[patch] proposed patch.

bug-194298.patch (text/plain), 8.61 KB, created by Mark Lam on 2019-02-05 12:51:11 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Mark Lam

Created: 2019-02-05 12:51:11 PST

Size: 8.61 KB

patch

obsolete

>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 240986)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,83 @@
>+2019-02-05  Mark Lam  <mark.lam@apple.com>
>+
>+        Move DFG nodes that clobberize() says will write(Heap) to the doesGC() list that returns true.
>+        https://bugs.webkit.org/show_bug.cgi?id=194298
>+        <rdar://problem/47827555>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        We do this for 3 reasons:
>+        1. It's clearer when reading doesGC()'s code that these nodes will return true.
>+        2. If things change in the future where clobberize() no longer reports these nodes
>+           as write(Heap), each node should be vetted first to make sure that it can never
>+           GC before being moved back to the doesGC() list that returns false.
>+        3. This reduces the list of nodes that we need to audit to make sure doesGC() is
>+           correct in its claims about the nodes' GCing possibility.
>+
>+        The list of nodes moved are:
>+
>+            ArrayPush
>+            ArrayPop
>+            Call
>+            CallEval
>+            CallForwardVarargs
>+            CallVarargs
>+            Construct
>+            ConstructForwardVarargs
>+            ConstructVarargs
>+            DefineDataProperty
>+            DefineAccessorProperty
>+            DeleteById
>+            DeleteByVal
>+            DirectCall
>+            DirectConstruct
>+            DirectTailCallInlinedCaller
>+            GetById
>+            GetByIdDirect
>+            GetByIdDirectFlush
>+            GetByIdFlush
>+            GetByIdWithThis
>+            GetByValWithThis
>+            GetDirectPname
>+            GetDynamicVar
>+            HasGenericProperty
>+            HasOwnProperty
>+            HasStructureProperty
>+            InById
>+            InByVal
>+            InstanceOf
>+            InstanceOfCustom
>+            LoadVarargs
>+            NumberToStringWithRadix
>+            PutById
>+            PutByIdDirect
>+            PutByIdFlush
>+            PutByIdWithThis
>+            PutByOffset
>+            PutByValWithThis
>+            PutDynamicVar
>+            PutGetterById
>+            PutGetterByVal
>+            PutGetterSetterById
>+            PutSetterById
>+            PutSetterByVal
>+            PutStack
>+            PutToArguments
>+            RegExpExec
>+            RegExpTest
>+            ResolveScope
>+            ResolveScopeForHoistingFuncDeclInEval
>+            TailCall
>+            TailCallForwardVarargsInlinedCaller
>+            TailCallInlinedCaller
>+            TailCallVarargsInlinedCaller
>+            ToNumber
>+            ToPrimitive
>+            ValueNegate
>+
>+        * dfg/DFGDoesGC.cpp:
>+        (JSC::DFG::doesGC):
>+
> 2019-02-05  Yusuke Suzuki  <ysuzuki@apple.com>
> 
>         [JSC] Shrink sizeof(UnlinkedCodeBlock)
>Index: Source/JavaScriptCore/dfg/DFGDoesGC.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGDoesGC.cpp	(revision 240935)
>+++ Source/JavaScriptCore/dfg/DFGDoesGC.cpp	(working copy)
>@@ -97,27 +97,7 @@ bool doesGC(Graph& graph, Node* node)
>     case ArithTrunc:
>     case ArithFRound:
>     case ArithUnary:
>-    case ValueNegate:
>     case TryGetById:
>-    case GetById:
>-    case GetByIdFlush:
>-    case GetByIdWithThis:
>-    case GetByIdDirect:
>-    case GetByIdDirectFlush:
>-    case PutById:
>-    case PutByIdFlush:
>-    case PutByIdWithThis:
>-    case PutByValWithThis:
>-    case PutByIdDirect:
>-    case PutGetterById:
>-    case PutSetterById:
>-    case PutGetterSetterById:
>-    case PutGetterByVal:
>-    case PutSetterByVal:
>-    case DefineDataProperty:
>-    case DefineAccessorProperty:
>-    case DeleteById:
>-    case DeleteByVal:
>     case CheckStructure:
>     case CheckStructureOrEmpty:
>     case CheckStructureImmediate:
>@@ -141,9 +121,7 @@ bool doesGC(Graph& graph, Node* node)
>     case CheckNotEmpty:
>     case AssertNotEmpty:
>     case CheckStringIdent:
>-    case RegExpExec:
>     case RegExpExecNonGlobalOrSticky:
>-    case RegExpTest:
>     case RegExpMatchFast:
>     case RegExpMatchFastGlobal:
>     case CompareLess:
>@@ -155,26 +133,10 @@ bool doesGC(Graph& graph, Node* node)
>     case CompareEq:
>     case CompareStrictEq:
>     case CompareEqPtr:
>-    case Call:
>-    case DirectCall:
>-    case TailCallInlinedCaller:
>-    case DirectTailCallInlinedCaller:
>-    case Construct:
>-    case DirectConstruct:
>-    case CallVarargs:
>-    case CallEval:
>-    case TailCallVarargsInlinedCaller:
>-    case ConstructVarargs:
>-    case LoadVarargs:
>-    case CallForwardVarargs:
>-    case ConstructForwardVarargs:
>     case TailCallForwardVarargs:
>-    case TailCallForwardVarargsInlinedCaller:
>     case ProfileType:
>     case ProfileControlFlow:
>     case OverridesHasInstance:
>-    case InstanceOf:
>-    case InstanceOfCustom:
>     case IsEmpty:
>     case IsUndefined:
>     case IsUndefinedOrNull:
>@@ -188,19 +150,12 @@ bool doesGC(Graph& graph, Node* node)
>     case IsTypedArrayView:
>     case TypeOf:
>     case LogicalNot:
>-    case ToPrimitive:
>-    case ToNumber:
>-    case NumberToStringWithRadix:
>     case NumberToStringWithValidRadixConstant:
>-    case InByVal:
>-    case InById:
>-    case HasOwnProperty:
>     case Jump:
>     case Branch:
>     case Switch:
>     case EntrySwitch:
>     case Return:
>-    case TailCall:
>     case DirectTailCall:
>     case TailCallVarargs:
>     case Throw:
>@@ -245,11 +200,8 @@ bool doesGC(Graph& graph, Node* node)
>     case GetGetter:
>     case GetSetter:
>     case GetByVal:
>-    case GetByValWithThis:
>     case GetArrayLength:
>     case GetVectorLength:
>-    case ArrayPush:
>-    case ArrayPop:
>     case StringCharAt:
>     case StringCharCodeAt:
>     case GetTypedArrayByteOffset:
>@@ -260,12 +212,8 @@ bool doesGC(Graph& graph, Node* node)
>     case PutStructure:
>     case GetByOffset:
>     case GetGetterSetterByOffset:
>-    case PutByOffset:
>     case GetEnumerableLength:
>-    case HasGenericProperty:
>-    case HasStructureProperty:
>     case HasIndexedProperty:
>-    case GetDirectPname:
>     case FiatInt52:
>     case BooleanToNumber:
>     case CheckBadCell:
>@@ -287,18 +235,12 @@ bool doesGC(Graph& graph, Node* node)
>     case GetMyArgumentByValOutOfBounds:
>     case ForwardVarargs:
>     case PutHint:
>-    case PutStack:
>     case KillStack:
>     case GetStack:
>     case GetFromArguments:
>-    case PutToArguments:
>     case GetArgument:
>     case LogShadowChickenPrologue:
>     case LogShadowChickenTail:
>-    case GetDynamicVar:
>-    case PutDynamicVar:
>-    case ResolveScopeForHoistingFuncDeclInEval:
>-    case ResolveScope:
>     case NukeStructureAndSetButterfly:
>     case AtomicsAdd:
>     case AtomicsAnd:
>@@ -320,13 +262,70 @@ bool doesGC(Graph& graph, Node* node)
>     case DataViewSet:
>         return false;
> 
>+    case ArrayPush:
>+    case ArrayPop:
>     case PushWithScope:
>     case CreateActivation:
>     case CreateDirectArguments:
>     case CreateScopedArguments:
>     case CreateClonedArguments:
>+    case Call:
>+    case CallEval:
>+    case CallForwardVarargs:
>     case CallObjectConstructor:
>+    case CallVarargs:
>+    case Construct:
>+    case ConstructForwardVarargs:
>+    case ConstructVarargs:
>+    case DefineDataProperty:
>+    case DefineAccessorProperty:
>+    case DeleteById:
>+    case DeleteByVal:
>+    case DirectCall:
>+    case DirectConstruct:
>+    case DirectTailCallInlinedCaller:
>+    case GetById:
>+    case GetByIdDirect:
>+    case GetByIdDirectFlush:
>+    case GetByIdFlush:
>+    case GetByIdWithThis:
>+    case GetByValWithThis:
>+    case GetDirectPname:
>+    case GetDynamicVar:
>+    case HasGenericProperty:
>+    case HasOwnProperty:
>+    case HasStructureProperty:
>+    case InById:
>+    case InByVal:
>+    case InstanceOf:
>+    case InstanceOfCustom:
>+    case LoadVarargs:
>+    case NumberToStringWithRadix:
>+    case PutById:
>+    case PutByIdDirect:
>+    case PutByIdFlush:
>+    case PutByIdWithThis:
>+    case PutByOffset:
>+    case PutByValWithThis:
>+    case PutDynamicVar:
>+    case PutGetterById:
>+    case PutGetterByVal:
>+    case PutGetterSetterById:
>+    case PutSetterById:
>+    case PutSetterByVal:
>+    case PutStack:
>+    case PutToArguments:
>+    case RegExpExec:
>+    case RegExpTest:
>+    case ResolveScope:
>+    case ResolveScopeForHoistingFuncDeclInEval:
>+    case TailCall:
>+    case TailCallForwardVarargsInlinedCaller:
>+    case TailCallInlinedCaller:
>+    case TailCallVarargsInlinedCaller:
>+    case ToNumber:
>     case ToObject:
>+    case ToPrimitive:
>     case ToThis:
>     case CreateThis:
>     case ObjectCreate:
>@@ -379,6 +378,7 @@ bool doesGC(Graph& graph, Node* node)
>     case ValueSub:
>     case ValueMul:
>     case ValueDiv:
>+    case ValueNegate:
>         return true;
> 
>     case CallStringConstructor:

Flags:

saam: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 194298: 361210