- Revision
- 228725
- Author
- sbar...@apple.com
- Date
- 2018-02-19 19:45:03 -0800 (Mon, 19 Feb 2018)
Log Message
Don't use JSFunction's allocation profile when getting the prototype can be effectful
https://bugs.webkit.org/show_bug.cgi?id=182942
<rdar://problem/37584764>
Reviewed by Mark Lam.
JSTests:
* stress/get-prototype-create-this-effectful.js: Added.
Source/_javascript_Core:
Prior to this patch, the create_this implementation assumed that anything
that is a JSFunction can use the object allocation profile and go down the
fast path to allocate the |this| object. Implied by this approach is that
accessing the 'prototype' property of the incoming function is not an
effectful operation. This is inherent to the ObjectAllocationProfile
data structure: it caches the prototype field. However, getting the
'prototype' property might be an effectful operation, e.g, it could
be a getter. Many variants of functions in JS have the 'prototype' property
as non-configurable. However, some functions, like bound functions, do not
have the 'prototype' field with these attributes.
This patch adds the notion of 'canUseAllocationProfile' to JSFunction
and threads it through so that we only go down the fast path and use
the allocation profile when the prototype property is non-configurable.
* bytecompiler/NodesCodegen.cpp:
(JSC::ClassExprNode::emitBytecode):
* dfg/DFGOperations.cpp:
* runtime/CommonSlowPaths.cpp:
(JSC::SLOW_PATH_DECL):
* runtime/JSFunction.cpp:
(JSC::JSFunction::prototypeForConstruction):
(JSC::JSFunction::allocateAndInitializeRareData):
(JSC::JSFunction::initializeRareData):
(JSC::JSFunction::getOwnPropertySlot):
(JSC::JSFunction::canUseAllocationProfileNonInline):
* runtime/JSFunction.h:
(JSC::JSFunction::ensureRareDataAndAllocationProfile):
* runtime/JSFunctionInlines.h:
(JSC::JSFunction::canUseAllocationProfile):
Modified Paths
Added Paths
Diff
Modified: trunk/JSTests/ChangeLog (228724 => 228725)
--- trunk/JSTests/ChangeLog 2018-02-20 03:21:53 UTC (rev 228724)
+++ trunk/JSTests/ChangeLog 2018-02-20 03:45:03 UTC (rev 228725)
@@ -1,3 +1,13 @@
+2018-02-19 Saam Barati <sbar...@apple.com>
+
+ Don't use JSFunction's allocation profile when getting the prototype can be effectful
+ https://bugs.webkit.org/show_bug.cgi?id=182942
+ <rdar://problem/37584764>
+
+ Reviewed by Mark Lam.
+
+ * stress/get-prototype-create-this-effectful.js: Added.
+
2018-02-16 Saam Barati <sbar...@apple.com>
Fix bugs from r228411
Added: trunk/JSTests/stress/get-prototype-create-this-effectful.js (0 => 228725)
--- trunk/JSTests/stress/get-prototype-create-this-effectful.js (rev 0)
+++ trunk/JSTests/stress/get-prototype-create-this-effectful.js 2018-02-20 03:45:03 UTC (rev 228725)
@@ -0,0 +1,40 @@
+function assert(b) {
+ if (!b)
+ throw new Error("Bad assertion")
+}
+
+function test1() {
+ let boundFunction = function () {}.bind();
+ Object.defineProperty(boundFunction, "prototype", {
+ get() {
+ throw Error("Hello");
+ }
+ });
+
+ let threw = false;
+ try {
+ Reflect.construct(function() {}, [], boundFunction);
+ } catch(e) {
+ threw = true;
+ assert(e.message === "Hello");
+ }
+ assert(threw);
+}
+test1();
+
+function test2() {
+ let boundFunction = function () {}.bind();
+ let counter = 0;
+ Object.defineProperty(boundFunction, "prototype", {
+ get() {
+ ++counter;
+ return {};
+ }
+ });
+
+ const iters = 1000;
+ for (let i = 0; i < iters; ++i)
+ Reflect.construct(function() {}, [], boundFunction);
+ assert(counter === iters);
+}
+test2();
Modified: trunk/Source/_javascript_Core/ChangeLog (228724 => 228725)
--- trunk/Source/_javascript_Core/ChangeLog 2018-02-20 03:21:53 UTC (rev 228724)
+++ trunk/Source/_javascript_Core/ChangeLog 2018-02-20 03:45:03 UTC (rev 228725)
@@ -1,5 +1,44 @@
2018-02-19 Saam Barati <sbar...@apple.com>
+ Don't use JSFunction's allocation profile when getting the prototype can be effectful
+ https://bugs.webkit.org/show_bug.cgi?id=182942
+ <rdar://problem/37584764>
+
+ Reviewed by Mark Lam.
+
+ Prior to this patch, the create_this implementation assumed that anything
+ that is a JSFunction can use the object allocation profile and go down the
+ fast path to allocate the |this| object. Implied by this approach is that
+ accessing the 'prototype' property of the incoming function is not an
+ effectful operation. This is inherent to the ObjectAllocationProfile
+ data structure: it caches the prototype field. However, getting the
+ 'prototype' property might be an effectful operation, e.g, it could
+ be a getter. Many variants of functions in JS have the 'prototype' property
+ as non-configurable. However, some functions, like bound functions, do not
+ have the 'prototype' field with these attributes.
+
+ This patch adds the notion of 'canUseAllocationProfile' to JSFunction
+ and threads it through so that we only go down the fast path and use
+ the allocation profile when the prototype property is non-configurable.
+
+ * bytecompiler/NodesCodegen.cpp:
+ (JSC::ClassExprNode::emitBytecode):
+ * dfg/DFGOperations.cpp:
+ * runtime/CommonSlowPaths.cpp:
+ (JSC::SLOW_PATH_DECL):
+ * runtime/JSFunction.cpp:
+ (JSC::JSFunction::prototypeForConstruction):
+ (JSC::JSFunction::allocateAndInitializeRareData):
+ (JSC::JSFunction::initializeRareData):
+ (JSC::JSFunction::getOwnPropertySlot):
+ (JSC::JSFunction::canUseAllocationProfileNonInline):
+ * runtime/JSFunction.h:
+ (JSC::JSFunction::ensureRareDataAndAllocationProfile):
+ * runtime/JSFunctionInlines.h:
+ (JSC::JSFunction::canUseAllocationProfile):
+
+2018-02-19 Saam Barati <sbar...@apple.com>
+
Don't mark an array profile out of bounds for the cases where the DFG will convert the access to SaneChain
https://bugs.webkit.org/show_bug.cgi?id=182912
<rdar://problem/37685083>
Modified: trunk/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp (228724 => 228725)
--- trunk/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp 2018-02-20 03:21:53 UTC (rev 228724)
+++ trunk/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp 2018-02-20 03:45:03 UTC (rev 228725)
@@ -3897,7 +3897,6 @@
RefPtr<RegisterID> constructor;
bool needsHomeObject = false;
- // FIXME: Make the prototype non-configurable & non-writable.
if (m_constructorExpression) {
ASSERT(m_constructorExpression->isFuncExprNode());
FunctionMetadataNode* metadata = static_cast<FuncExprNode*>(m_constructorExpression)->metadata();
Modified: trunk/Source/_javascript_Core/dfg/DFGOperations.cpp (228724 => 228725)
--- trunk/Source/_javascript_Core/dfg/DFGOperations.cpp 2018-02-20 03:21:53 UTC (rev 228724)
+++ trunk/Source/_javascript_Core/dfg/DFGOperations.cpp 2018-02-20 03:45:03 UTC (rev 228725)
@@ -246,8 +246,8 @@
VM& vm = exec->vm();
NativeCallFrameTracer tracer(&vm, exec);
auto scope = DECLARE_THROW_SCOPE(vm);
- if (constructor->type() == JSFunctionType) {
- auto rareData = jsCast<JSFunction*>(constructor)->rareData(exec, inlineCapacity);
+ if (constructor->type() == JSFunctionType && jsCast<JSFunction*>(constructor)->canUseAllocationProfile()) {
+ auto rareData = jsCast<JSFunction*>(constructor)->ensureRareDataAndAllocationProfile(exec, inlineCapacity);
RETURN_IF_EXCEPTION(scope, nullptr);
Structure* structure = rareData->objectAllocationProfile()->structure();
JSObject* result = constructEmptyObject(exec, structure);
Modified: trunk/Source/_javascript_Core/runtime/CommonSlowPaths.cpp (228724 => 228725)
--- trunk/Source/_javascript_Core/runtime/CommonSlowPaths.cpp 2018-02-20 03:21:53 UTC (rev 228724)
+++ trunk/Source/_javascript_Core/runtime/CommonSlowPaths.cpp 2018-02-20 03:45:03 UTC (rev 228725)
@@ -222,7 +222,7 @@
auto& bytecode = *reinterpret_cast<OpCreateThis*>(pc);
JSObject* result;
JSObject* constructorAsObject = asObject(GET(bytecode.callee()).jsValue());
- if (constructorAsObject->type() == JSFunctionType) {
+ if (constructorAsObject->type() == JSFunctionType && jsCast<JSFunction*>(constructorAsObject)->canUseAllocationProfile()) {
JSFunction* constructor = jsCast<JSFunction*>(constructorAsObject);
WriteBarrier<JSCell>& cachedCallee = bytecode.cachedCallee();
if (!cachedCallee)
@@ -231,7 +231,7 @@
cachedCallee.setWithoutWriteBarrier(JSCell::seenMultipleCalleeObjects());
size_t inlineCapacity = bytecode.inlineCapacity();
- Structure* structure = constructor->rareData(exec, inlineCapacity)->objectAllocationProfile()->structure();
+ Structure* structure = constructor->ensureRareDataAndAllocationProfile(exec, inlineCapacity)->objectAllocationProfile()->structure();
result = constructEmptyObject(exec, structure);
if (structure->hasPolyProto()) {
JSObject* prototype = constructor->prototypeForConstruction(vm, exec);
Modified: trunk/Source/_javascript_Core/runtime/JSFunction.cpp (228724 => 228725)
--- trunk/Source/_javascript_Core/runtime/JSFunction.cpp 2018-02-20 03:21:53 UTC (rev 228724)
+++ trunk/Source/_javascript_Core/runtime/JSFunction.cpp 2018-02-20 03:45:03 UTC (rev 228725)
@@ -139,9 +139,12 @@
JSObject* JSFunction::prototypeForConstruction(VM& vm, ExecState* exec)
{
+ // This code assumes getting the prototype is not effectful. That's only
+ // true when we can use the allocation profile.
+ ASSERT(canUseAllocationProfile());
auto scope = DECLARE_CATCH_SCOPE(vm);
JSValue prototype = get(exec, vm.propertyNames->prototype);
- ASSERT_UNUSED(scope, !scope.exception());
+ scope.releaseAssertNoException();
if (prototype.isObject())
return asObject(prototype);
@@ -151,6 +154,7 @@
FunctionRareData* JSFunction::allocateAndInitializeRareData(ExecState* exec, size_t inlineCapacity)
{
ASSERT(!m_rareData);
+ ASSERT(canUseAllocationProfile());
VM& vm = exec->vm();
JSObject* prototype = prototypeForConstruction(vm, exec);
FunctionRareData* rareData = FunctionRareData::create(vm);
@@ -167,6 +171,7 @@
FunctionRareData* JSFunction::initializeRareData(ExecState* exec, size_t inlineCapacity)
{
ASSERT(!!m_rareData);
+ ASSERT(canUseAllocationProfile());
VM& vm = exec->vm();
JSObject* prototype = prototypeForConstruction(vm, exec);
m_rareData->initializeObjectAllocationProfile(vm, globalObject(vm), prototype, inlineCapacity, this);
@@ -374,6 +379,13 @@
}
if (propertyName == vm.propertyNames->prototype && thisObject->jsExecutable()->hasPrototypeProperty() && !thisObject->jsExecutable()->isClassConstructorFunction()) {
+ // NOTE: class constructors define the prototype property in bytecode using
+ // defineOwnProperty, which ends up calling into this code (see our defineOwnProperty
+ // implementation below). The bytecode will end up doing the proper definition
+ // with the property being non-writable/non-configurable. However, we must ignore
+ // the initial materialization of the property so that the defineOwnProperty call
+ // from bytecode succeeds. Otherwise, the materialization here would prevent the
+ // defineOwnProperty from being able to overwrite the property.
unsigned attributes;
PropertyOffset offset = thisObject->getDirectOffset(vm, propertyName, attributes);
if (!isValidOffset(offset)) {
Modified: trunk/Source/_javascript_Core/runtime/JSFunction.h (228724 => 228725)
--- trunk/Source/_javascript_Core/runtime/JSFunction.h 2018-02-20 03:21:53 UTC (rev 228724)
+++ trunk/Source/_javascript_Core/runtime/JSFunction.h 2018-02-20 03:45:03 UTC (rev 228725)
@@ -130,14 +130,7 @@
return m_rareData.get();
}
- FunctionRareData* rareData(ExecState* exec, unsigned inlineCapacity)
- {
- if (UNLIKELY(!m_rareData))
- return allocateAndInitializeRareData(exec, inlineCapacity);
- if (UNLIKELY(!m_rareData->isObjectAllocationProfileInitialized()))
- return initializeRareData(exec, inlineCapacity);
- return m_rareData.get();
- }
+ FunctionRareData* ensureRareDataAndAllocationProfile(ExecState*, unsigned inlineCapacity);
FunctionRareData* rareData()
{
@@ -160,6 +153,9 @@
// Returns the __proto__ for the |this| value if this JSFunction were to be constructed.
JSObject* prototypeForConstruction(VM&, ExecState*);
+ bool canUseAllocationProfile();
+ bool canUseAllocationProfileNonInline();
+
protected:
JS_EXPORT_PRIVATE JSFunction(VM&, JSGlobalObject*, Structure*);
JSFunction(VM&, FunctionExecutable*, JSScope*, Structure*);
@@ -167,10 +163,6 @@
void finishCreation(VM&, NativeExecutable*, int length, const String& name);
void finishCreation(VM&);
- FunctionRareData* allocateRareData(VM&);
- FunctionRareData* allocateAndInitializeRareData(ExecState*, size_t inlineCapacity);
- FunctionRareData* initializeRareData(ExecState*, size_t inlineCapacity);
-
static bool getOwnPropertySlot(JSObject*, ExecState*, PropertyName, PropertySlot&);
static void getOwnNonIndexPropertyNames(JSObject*, ExecState*, PropertyNameArray&, EnumerationMode = EnumerationMode());
static bool defineOwnProperty(JSObject*, ExecState*, PropertyName, const PropertyDescriptor&, bool shouldThrow);
@@ -192,6 +184,10 @@
return function;
}
+ FunctionRareData* allocateRareData(VM&);
+ FunctionRareData* allocateAndInitializeRareData(ExecState*, size_t inlineCapacity);
+ FunctionRareData* initializeRareData(ExecState*, size_t inlineCapacity);
+
bool hasReifiedLength() const;
bool hasReifiedName() const;
void reifyLength(VM&);
Modified: trunk/Source/_javascript_Core/runtime/JSFunctionInlines.h (228724 => 228725)
--- trunk/Source/_javascript_Core/runtime/JSFunctionInlines.h 2018-02-20 03:21:53 UTC (rev 228724)
+++ trunk/Source/_javascript_Core/runtime/JSFunctionInlines.h 2018-02-20 03:45:03 UTC (rev 228725)
@@ -107,4 +107,26 @@
return m_rareData ? m_rareData->hasReifiedName() : false;
}
+inline bool JSFunction::canUseAllocationProfile()
+{
+ if (isHostFunction())
+ 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
+ // as a getter. JS semantics require that the getter is called every
+ // time |construct| occurs with this function as new.target.
+ return jsExecutable()->hasPrototypeProperty();
+}
+
+inline FunctionRareData* JSFunction::ensureRareDataAndAllocationProfile(ExecState* exec, unsigned inlineCapacity)
+{
+ ASSERT(canUseAllocationProfile());
+ if (UNLIKELY(!m_rareData))
+ return allocateAndInitializeRareData(exec, inlineCapacity);
+ if (UNLIKELY(!m_rareData->isObjectAllocationProfileInitialized()))
+ return initializeRareData(exec, inlineCapacity);
+ return m_rareData.get();
+}
+
} // namespace JSC
