Changeset 240040 in webkit

Timestamp:

Jan 16, 2019, 10:10:44 AM (6 years ago)

Author:

mark.lam@apple.com

Message:

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 Saam Barati.

JSTests:

• microbenchmarks/sinkable-new-object-with-builtin-constructor.js: Added.
• stress/constructing-builtin-functions-with-getter-prototype-should-only-call-getter-once-per-new-1.js: Added.
• stress/constructing-builtin-functions-with-getter-prototype-should-only-call-getter-once-per-new-2.js: Added.
• stress/jsfunction-cannot-use-allocation-profile-with-builtin-functions-with-no-prototype.js: Added.

Source/JavaScriptCore:

JSFunction::canUseAllocationProfile() should return false for most builtins
because the majority of them have 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 or custom getter because
getting the prototype would then be effectful.

• dfg/DFGOperations.cpp:
• runtime/CommonSlowPaths.cpp:

(JSC::SLOW_PATH_DECL):

• runtime/JSFunctionInlines.h:

(JSC::JSFunction::canUseAllocationProfile):

• runtime/PropertySlot.h:

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/microbenchmarks/sinkable-new-object-with-builtin-constructor.js (added)
• JSTests/stress/constructing-builtin-functions-with-getter-prototype-should-only-call-getter-once-per-new-1.js (added)
• JSTests/stress/constructing-builtin-functions-with-getter-prototype-should-only-call-getter-once-per-new-2.js (added)
• JSTests/stress/jsfunction-cannot-use-allocation-profile-with-builtin-functions-with-no-prototype.js (added)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/dfg/DFGOperations.cpp (modified) (1 diff)
• Source/JavaScriptCore/runtime/CommonSlowPaths.cpp (modified) (1 diff)
• Source/JavaScriptCore/runtime/JSFunctionInlines.h (modified) (1 diff)
• Source/JavaScriptCore/runtime/PropertySlot.h (modified) (1 diff)

trunk/JSTests/ChangeLog

@@ +1 @@
+2019-01-15  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 Saam Barati.
+
+        * microbenchmarks/sinkable-new-object-with-builtin-constructor.js: Added.
+        * stress/constructing-builtin-functions-with-getter-prototype-should-only-call-getter-once-per-new-1.js: Added.
+        * stress/constructing-builtin-functions-with-getter-prototype-should-only-call-getter-once-per-new-2.js: Added.
+        * stress/jsfunction-cannot-use-allocation-profile-with-builtin-functions-with-no-prototype.js: Added.
+
 2019-01-15  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2019-01-15  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 Saam Barati.
+
+        JSFunction::canUseAllocationProfile() should return false for most builtins
+        because the majority of them have 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 or custom getter because
+        getting the prototype would then be effectful.
+
+        * dfg/DFGOperations.cpp:
+        * runtime/CommonSlowPaths.cpp:
+        (JSC::SLOW_PATH_DECL):
+        * runtime/JSFunctionInlines.h:
+        (JSC::JSFunction::canUseAllocationProfile):
+        * runtime/PropertySlot.h:
+
 2019-01-15  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
 

trunk/Source/JavaScriptCore/dfg/DFGOperations.cpp

@@ -302 +302 @@
     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();

trunk/Source/JavaScriptCore/runtime/CommonSlowPaths.cpp

@@ -244 +244 @@
         size_t inlineCapacity = bytecode.inlineCapacity;
         ObjectAllocationProfile* allocationProfile = constructor->ensureRareDataAndAllocationProfile(exec, inlineCapacity)->objectAllocationProfile();
+        throwScope.releaseAssertNoException();
         Structure* structure = allocationProfile->structure();
         result = constructEmptyObject(exec, structure);

trunk/Source/JavaScriptCore/runtime/JSFunctionInlines.h

@@ -111 +111 @@
 inline bool JSFunction::canUseAllocationProfile()
 {
-    if (isHostFunction())
-        return false;
+    if (isHostOrBuiltinFunction()) {
+        if (isHostFunction())
+            return false;
+
+        VM& vm = globalObject()->vm();
+        unsigned attributes;
+        JSValue prototype = getDirect(vm, vm.propertyNames->prototype, attributes);
+        if (!prototype || (attributes & PropertyAttribute::AccessorOrCustomAccessorOrValue))
+            return false;
+    }
 
     // If we don't have a prototype property, we're not guaranteed it's

trunk/Source/JavaScriptCore/runtime/PropertySlot.h

@@ -46 +46 @@
     CustomValue       = 1 << 6,
     CustomAccessorOrValue = CustomAccessor | CustomValue,
+    AccessorOrCustomAccessorOrValue = Accessor | CustomAccessor | CustomValue,
 
     // Things that are used by static hashtables are not in the attributes byte in PropertyMapEntry.