Attachment 359116 Details for Bug 193423 – proposed patch.

[patch] proposed patch.

bug-193423.patch (text/plain), 5.21 KB, created by Mark Lam on 2019-01-14 18:49:03 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Mark Lam

Created: 2019-01-14 18:49:03 PST

Size: 5.21 KB

patch

obsolete

>Index: JSTests/ChangeLog
>===================================================================
>--- JSTests/ChangeLog	(revision 239968)
>+++ JSTests/ChangeLog	(working copy)
>@@ -1,3 +1,13 @@
>+2019-01-14  Mark Lam  <mark.lam@apple.com>
>+
>+        JSFunction::canUseAllocationProfile() should account for builtin functions with no own prototypes.
>+        https://bugs.webkit.org/show_bug.cgi?id=193423
>+        <rdar://problem/46209355>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * stress/jsfunction-cannot-use-allocation-profile-with-builtin-functions-with-no-prototype.js: Added.
>+
> 2019-01-14  Keith Miller  <keith_miller@apple.com>
> 
>         Skip type-check-hoisting-phase-hoist... with no jit
>Index: JSTests/stress/jsfunction-cannot-use-allocation-profile-with-builtin-functions-with-no-prototype.js
>===================================================================
>--- JSTests/stress/jsfunction-cannot-use-allocation-profile-with-builtin-functions-with-no-prototype.js	(nonexistent)
>+++ JSTests/stress/jsfunction-cannot-use-allocation-profile-with-builtin-functions-with-no-prototype.js	(working copy)
>@@ -0,0 +1,9 @@
>+Object.defineProperty(Function.prototype, 'prototype', {
>+    get: function () {
>+        throw new Error('hello');
>+    }
>+});
>+
>+new Promise(resolve => {
>+    new resolve();
>+});
>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 239936)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,25 @@
>+2019-01-14  Mark Lam  <mark.lam@apple.com>
>+
>+        JSFunction::canUseAllocationProfile() should account for builtin functions with no own prototypes.
>+        https://bugs.webkit.org/show_bug.cgi?id=193423
>+        <rdar://problem/46209355>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        JSFunction::canUseAllocationProfile() should return false for most builtins
>+        because the majority of them has no prototype property.  The only exception to
>+        this is the few builtin functions that are explicitly used as constructors.
>+
>+        For these builtin constructors, JSFunction::canUseAllocationProfile() should also
>+        return false if the prototype property is a getter, custom getter, or proxy
>+        because getting the prototype would then be effectful.
>+
>+        * dfg/DFGOperations.cpp:
>+        * runtime/CommonSlowPaths.cpp:
>+        (JSC::SLOW_PATH_DECL):
>+        * runtime/JSFunctionInlines.h:
>+        (JSC::JSFunction::canUseAllocationProfile):
>+
> 2019-01-14  Keith Miller  <keith_miller@apple.com>
> 
>         JSC should have a module loader API
>Index: Source/JavaScriptCore/dfg/DFGOperations.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGOperations.cpp	(revision 239936)
>+++ Source/JavaScriptCore/dfg/DFGOperations.cpp	(working copy)
>@@ -301,7 +301,7 @@ JSCell* JIT_OPERATION operationCreateThi
>     auto scope = DECLARE_THROW_SCOPE(vm);
>     if (constructor->type() == JSFunctionType && jsCast<JSFunction*>(constructor)->canUseAllocationProfile()) {
>         auto rareData = jsCast<JSFunction*>(constructor)->ensureRareDataAndAllocationProfile(exec, inlineCapacity);
>-        RETURN_IF_EXCEPTION(scope, nullptr);
>+        scope.releaseAssertNoException();
>         ObjectAllocationProfile* allocationProfile = rareData->objectAllocationProfile();
>         Structure* structure = allocationProfile->structure();
>         JSObject* result = constructEmptyObject(exec, structure);
>Index: Source/JavaScriptCore/runtime/CommonSlowPaths.cpp
>===================================================================
>--- Source/JavaScriptCore/runtime/CommonSlowPaths.cpp	(revision 239936)
>+++ Source/JavaScriptCore/runtime/CommonSlowPaths.cpp	(working copy)
>@@ -243,6 +243,7 @@ SLOW_PATH_DECL(slow_path_create_this)
> 
>         size_t inlineCapacity = bytecode.inlineCapacity;
>         ObjectAllocationProfile* allocationProfile = constructor->ensureRareDataAndAllocationProfile(exec, inlineCapacity)->objectAllocationProfile();
>+        throwScope.releaseAsertNoException();
>         Structure* structure = allocationProfile->structure();
>         result = constructEmptyObject(exec, structure);
>         if (structure->hasPolyProto()) {
>Index: Source/JavaScriptCore/runtime/JSFunctionInlines.h
>===================================================================
>--- Source/JavaScriptCore/runtime/JSFunctionInlines.h	(revision 239936)
>+++ Source/JavaScriptCore/runtime/JSFunctionInlines.h	(working copy)
>@@ -110,8 +110,20 @@ inline bool JSFunction::hasReifiedName()
> 
> inline bool JSFunction::canUseAllocationProfile()
> {
>-    if (isHostFunction())
>-        return false;
>+    if (isHostOrBuiltinFunction()) {
>+        if (isHostFunction())
>+            return false;
>+
>+        VM& vm = globalObject()->vm();
>+        JSValue prototype = getDirect(vm, vm.propertyNames->prototype);
>+        if (!prototype)
>+            return false;
>+        if (prototype.isCell()) {
>+            JSCell* cell = prototype.asCell();
>+            if (cell->isGetterSetter() || cell->isCustomGetterSetter() || cell->isProxy())
>+                return false;
>+        }
>+    }
> 
>     // If we don't have a prototype property, we're not guaranteed it's
>     // non-configurable. For example, user code can define the prototype

Flags:

saam: review-

Actions: View | Formatted Diff | Diff

Attachments on bug 193423: 359116 | 359137 | 359239