ClonedArguments need to also support haveABadTime mode.
https://bugs.webkit.org/show_bug.cgi?id=164200
<rdar://problem/27211336>
Reviewed by Geoffrey Garen.
JSTests:
* stress/have-a-bad-time-with-arguments.js: Added.
Source/JavaScriptCore:
For those who are not familiar with the parlance, "have a bad time" in the VM
means that Object.prototype has been modified in such a way that we can no longer
trivially do indexed property accesses without consulting the Object.prototype.
This defeats JIT indexed put optimizations, and hence, makes the VM "have a
bad time".
Once the VM enters haveABadTime mode, all existing objects are converted to use
slow put storage. Thereafter, JSArrays are always created with slow put storage.
JSObjects are always created with a blank indexing type. When a new indexed
property is put into the new object, its indexing type will be converted to the
slow put array indexing type just before we perform the put operation. This is
how we ensure that the objects will also use slow put storage.
However, ClonedArguments is an object which was previously created unconditionally
to use contiguous storage. Subsequently, if we try to call Object.preventExtensions()
on that ClonedArguments object, Object.preventExtensions() will:
1. make the ClonedArguments enter dictionary indexing mode, which means it will
2. first ensure that the ClonedArguments is using slow put array storage via
JSObject::ensureArrayStorageSlow().
However, JSObject::ensureArrayStorageSlow() expects that we never see an object
with contiguous storage once we're in haveABadTime mode. Our ClonedArguments
object did not obey this invariant.
The fix is to make the ClonedArguments factories create objects that use slow put
array storage when in haveABadTime mode. This means:
1. JSGlobalObject::haveABadTime() now changes m_clonedArgumentsStructure to use
its slow put version.
Also the caching of the slow put version of m_regExpMatchesArrayStructure,
because we only need to create it when we are having a bad time.
2. The ClonedArguments factories now allocates a butterfly with slow put array
storage if we're in haveABadTime mode.
Also added some assertions in ClonedArguments' factory methods to ensure that
the created object has the slow put indexing type when it needsSlowPutIndexing().
3. DFGFixupPhase now watches the havingABadTimeWatchpoint because ClonedArguments'
structure will change when having a bad time.
4. DFGArgumentEliminationPhase and DFGVarargsForwardingPhase need not be changed
because it is still valid to eliminate the creation of the arguments object
even having a bad time, as long as the arguments object does not escape.
5. The DFGAbstractInterpreterInlines now checks for haveABadTime, and sets the
predicted type to be SpecObject.
Note: this issue does not apply to DirectArguments and ScopedArguments because
they use a blank indexing type (just like JSObject).
* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
* dfg/DFGArrayMode.cpp:
(JSC::DFG::ArrayMode::dump):
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
* runtime/ClonedArguments.cpp:
(JSC::ClonedArguments::createEmpty):
(JSC::ClonedArguments::createWithInlineFrame):
(JSC::ClonedArguments::createWithMachineFrame):
(JSC::ClonedArguments::createByCopyingFrom):
(JSC::ClonedArguments::createStructure):
(JSC::ClonedArguments::createSlowPutStructure):
* runtime/ClonedArguments.h:
* runtime/JSGlobalObject.cpp:
(JSC::JSGlobalObject::init):
(JSC::JSGlobalObject::haveABadTime):
(JSC::JSGlobalObject::visitChildren):
* runtime/JSGlobalObject.h:
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@208377 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index 8019031..b357359 100644
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,83 @@
+2016-11-03 Mark Lam <mark.lam@apple.com>
+
+ ClonedArguments need to also support haveABadTime mode.
+ https://bugs.webkit.org/show_bug.cgi?id=164200
+ <rdar://problem/27211336>
+
+ Reviewed by Geoffrey Garen.
+
+ For those who are not familiar with the parlance, "have a bad time" in the VM
+ means that Object.prototype has been modified in such a way that we can no longer
+ trivially do indexed property accesses without consulting the Object.prototype.
+ This defeats JIT indexed put optimizations, and hence, makes the VM "have a
+ bad time".
+
+ Once the VM enters haveABadTime mode, all existing objects are converted to use
+ slow put storage. Thereafter, JSArrays are always created with slow put storage.
+ JSObjects are always created with a blank indexing type. When a new indexed
+ property is put into the new object, its indexing type will be converted to the
+ slow put array indexing type just before we perform the put operation. This is
+ how we ensure that the objects will also use slow put storage.
+
+ However, ClonedArguments is an object which was previously created unconditionally
+ to use contiguous storage. Subsequently, if we try to call Object.preventExtensions()
+ on that ClonedArguments object, Object.preventExtensions() will:
+ 1. make the ClonedArguments enter dictionary indexing mode, which means it will
+ 2. first ensure that the ClonedArguments is using slow put array storage via
+ JSObject::ensureArrayStorageSlow().
+
+ However, JSObject::ensureArrayStorageSlow() expects that we never see an object
+ with contiguous storage once we're in haveABadTime mode. Our ClonedArguments
+ object did not obey this invariant.
+
+ The fix is to make the ClonedArguments factories create objects that use slow put
+ array storage when in haveABadTime mode. This means:
+
+ 1. JSGlobalObject::haveABadTime() now changes m_clonedArgumentsStructure to use
+ its slow put version.
+
+ Also the caching of the slow put version of m_regExpMatchesArrayStructure,
+ because we only need to create it when we are having a bad time.
+
+ 2. The ClonedArguments factories now allocates a butterfly with slow put array
+ storage if we're in haveABadTime mode.
+
+ Also added some assertions in ClonedArguments' factory methods to ensure that
+ the created object has the slow put indexing type when it needsSlowPutIndexing().
+
+ 3. DFGFixupPhase now watches the havingABadTimeWatchpoint because ClonedArguments'
+ structure will change when having a bad time.
+
+ 4. DFGArgumentEliminationPhase and DFGVarargsForwardingPhase need not be changed
+ because it is still valid to eliminate the creation of the arguments object
+ even having a bad time, as long as the arguments object does not escape.
+
+ 5. The DFGAbstractInterpreterInlines now checks for haveABadTime, and sets the
+ predicted type to be SpecObject.
+
+ Note: this issue does not apply to DirectArguments and ScopedArguments because
+ they use a blank indexing type (just like JSObject).
+
+ * dfg/DFGAbstractInterpreterInlines.h:
+ (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+ * dfg/DFGArrayMode.cpp:
+ (JSC::DFG::ArrayMode::dump):
+ * dfg/DFGFixupPhase.cpp:
+ (JSC::DFG::FixupPhase::fixupNode):
+ * runtime/ClonedArguments.cpp:
+ (JSC::ClonedArguments::createEmpty):
+ (JSC::ClonedArguments::createWithInlineFrame):
+ (JSC::ClonedArguments::createWithMachineFrame):
+ (JSC::ClonedArguments::createByCopyingFrom):
+ (JSC::ClonedArguments::createStructure):
+ (JSC::ClonedArguments::createSlowPutStructure):
+ * runtime/ClonedArguments.h:
+ * runtime/JSGlobalObject.cpp:
+ (JSC::JSGlobalObject::init):
+ (JSC::JSGlobalObject::haveABadTime):
+ (JSC::JSGlobalObject::visitChildren):
+ * runtime/JSGlobalObject.h:
+
2016-11-03 Filip Pizlo <fpizlo@apple.com>
DFG plays fast and loose with the shadow values of a Phi
diff --git a/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h b/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
index e45ff5a..fc5bf97 100644
--- a/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
+++ b/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
@@ -1981,6 +1981,10 @@
break;
case CreateClonedArguments:
+ if (!m_graph.isWatchingHavingABadTimeWatchpoint(node)) {
+ forNode(node).setType(m_graph, SpecObject);
+ break;
+ }
forNode(node).set(m_graph, m_codeBlock->globalObjectFor(node->origin.semantic)->clonedArgumentsStructure());
break;
diff --git a/Source/JavaScriptCore/dfg/DFGArrayMode.cpp b/Source/JavaScriptCore/dfg/DFGArrayMode.cpp
index 2c3c0ed..b79df2f 100644
--- a/Source/JavaScriptCore/dfg/DFGArrayMode.cpp
+++ b/Source/JavaScriptCore/dfg/DFGArrayMode.cpp
@@ -734,7 +734,7 @@
void ArrayMode::dump(PrintStream& out) const
{
- out.print(type(), arrayClass(), speculation(), conversion());
+ out.print(type(), "+", arrayClass(), "+", speculation(), "+", conversion());
}
} } // namespace JSC::DFG
diff --git a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
index c8dd562..848e1f4 100644
--- a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
+++ b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
@@ -1550,6 +1550,11 @@
break;
}
+ case CreateClonedArguments: {
+ watchHavingABadTime(node);
+ break;
+ }
+
case CreateScopedArguments:
case CreateActivation:
case NewFunction:
@@ -1776,7 +1781,6 @@
case IsObjectOrNull:
case IsFunction:
case CreateDirectArguments:
- case CreateClonedArguments:
case Jump:
case Return:
case TailCall:
diff --git a/Source/JavaScriptCore/interpreter/Interpreter.cpp b/Source/JavaScriptCore/interpreter/Interpreter.cpp
index 759a042..b7108b0 100644
--- a/Source/JavaScriptCore/interpreter/Interpreter.cpp
+++ b/Source/JavaScriptCore/interpreter/Interpreter.cpp
@@ -32,7 +32,6 @@
#include "BatchedTransitionOptimizer.h"
#include "CallFrameClosure.h"
-#include "ClonedArguments.h"
#include "CodeBlock.h"
#include "DirectArguments.h"
#include "Heap.h"
diff --git a/Source/JavaScriptCore/runtime/ClonedArguments.cpp b/Source/JavaScriptCore/runtime/ClonedArguments.cpp
index 5d405e5..2cfc66a 100644
--- a/Source/JavaScriptCore/runtime/ClonedArguments.cpp
+++ b/Source/JavaScriptCore/runtime/ClonedArguments.cpp
@@ -48,16 +48,23 @@
if (vectorLength > MAX_STORAGE_VECTOR_LENGTH)
return 0;
- void* temp = vm.heap.tryAllocateAuxiliary(nullptr, Butterfly::totalSize(0, structure->outOfLineCapacity(), true, vectorLength * sizeof(EncodedJSValue)));
- if (!temp)
- return 0;
- Butterfly* butterfly = Butterfly::fromBase(temp, 0, structure->outOfLineCapacity());
- butterfly->setVectorLength(vectorLength);
- butterfly->setPublicLength(length);
-
- for (unsigned i = length; i < vectorLength; ++i)
- butterfly->contiguous()[i].clear();
+ Butterfly* butterfly;
+ if (UNLIKELY(structure->needsSlowPutIndexing())) {
+ butterfly = createArrayStorageButterfly(vm, nullptr, structure, length, vectorLength);
+ butterfly->arrayStorage()->m_numValuesInVector = vectorLength;
+ } else {
+ void* temp = vm.heap.tryAllocateAuxiliary(nullptr, Butterfly::totalSize(0, structure->outOfLineCapacity(), true, vectorLength * sizeof(EncodedJSValue)));
+ if (!temp)
+ return 0;
+ butterfly = Butterfly::fromBase(temp, 0, structure->outOfLineCapacity());
+ butterfly->setVectorLength(vectorLength);
+ butterfly->setPublicLength(length);
+
+ for (unsigned i = length; i < vectorLength; ++i)
+ butterfly->contiguous()[i].clear();
+ }
+
ClonedArguments* result =
new (NotNull, allocateCell<ClonedArguments>(vm.heap))
ClonedArguments(vm, structure, butterfly);
@@ -70,9 +77,12 @@
ClonedArguments* ClonedArguments::createEmpty(ExecState* exec, JSFunction* callee, unsigned length)
{
+ VM& vm = exec->vm();
// NB. Some clients might expect that the global object of of this object is the global object
// of the callee. We don't do this for now, but maybe we should.
- return createEmpty(exec->vm(), exec->lexicalGlobalObject()->clonedArgumentsStructure(), callee, length);
+ ClonedArguments* result = createEmpty(vm, exec->lexicalGlobalObject()->clonedArgumentsStructure(), callee, length);
+ ASSERT(!result->structure(vm)->needsSlowPutIndexing() || shouldUseSlowPut(result->structure(vm)->indexingType()));
+ return result;
}
ClonedArguments* ClonedArguments::createWithInlineFrame(ExecState* myFrame, ExecState* targetFrame, InlineCallFrame* inlineCallFrame, ArgumentsMode mode)
@@ -117,12 +127,15 @@
} }
ASSERT(myFrame->lexicalGlobalObject()->clonedArgumentsStructure() == result->structure());
+ ASSERT(!result->structure(vm)->needsSlowPutIndexing() || shouldUseSlowPut(result->structure(vm)->indexingType()));
return result;
}
ClonedArguments* ClonedArguments::createWithMachineFrame(ExecState* myFrame, ExecState* targetFrame, ArgumentsMode mode)
{
- return createWithInlineFrame(myFrame, targetFrame, nullptr, mode);
+ ClonedArguments* result = createWithInlineFrame(myFrame, targetFrame, nullptr, mode);
+ ASSERT(!result->structure()->needsSlowPutIndexing() || shouldUseSlowPut(result->structure()->indexingType()));
+ return result;
}
ClonedArguments* ClonedArguments::createByCopyingFrom(
@@ -134,19 +147,28 @@
for (unsigned i = length; i--;)
result->initializeIndex(vm, i, argumentStart[i].jsValue());
-
+ ASSERT(!result->structure(vm)->needsSlowPutIndexing() || shouldUseSlowPut(result->structure(vm)->indexingType()));
return result;
}
+Structure* ClonedArguments::createStructure(VM& vm, JSGlobalObject* globalObject, JSValue prototype, IndexingType indexingType)
+{
+ Structure* structure = Structure::create(vm, globalObject, prototype, TypeInfo(ObjectType, StructureFlags), info(), indexingType);
+ PropertyOffset offset;
+ structure = structure->addPropertyTransition(vm, structure, vm.propertyNames->length, DontEnum, offset);
+ ASSERT(offset == clonedArgumentsLengthPropertyOffset);
+ return structure;
+}
+
Structure* ClonedArguments::createStructure(VM& vm, JSGlobalObject* globalObject, JSValue prototype)
{
// We use contiguous storage because optimizations in the FTL assume that cloned arguments creation always produces the same initial structure.
+ return createStructure(vm, globalObject, prototype, NonArrayWithContiguous);
+}
- Structure* structure = Structure::create(vm, globalObject, prototype, TypeInfo(ObjectType, StructureFlags), info(), NonArrayWithContiguous);
- PropertyOffset offset;
- structure = structure->addPropertyTransition(vm, structure, vm.propertyNames->length, DontEnum, offset);
- ASSERT(offset == clonedArgumentsLengthPropertyOffset);
- return structure;
+Structure* ClonedArguments::createSlowPutStructure(VM& vm, JSGlobalObject* globalObject, JSValue prototype)
+{
+ return createStructure(vm, globalObject, prototype, NonArrayWithSlowPutArrayStorage);
}
bool ClonedArguments::getOwnPropertySlot(JSObject* object, ExecState* exec, PropertyName ident, PropertySlot& slot)
diff --git a/Source/JavaScriptCore/runtime/ClonedArguments.h b/Source/JavaScriptCore/runtime/ClonedArguments.h
index 9544481..7601c5c 100644
--- a/Source/JavaScriptCore/runtime/ClonedArguments.h
+++ b/Source/JavaScriptCore/runtime/ClonedArguments.h
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2015 Apple Inc. All rights reserved.
+ * Copyright (C) 2015-2016 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -53,12 +53,15 @@
static ClonedArguments* createByCopyingFrom(ExecState*, Structure*, Register* argumentsStart, unsigned length, JSFunction* callee);
static Structure* createStructure(VM&, JSGlobalObject*, JSValue prototype);
+ static Structure* createSlowPutStructure(VM&, JSGlobalObject*, JSValue prototype);
static void visitChildren(JSCell*, SlotVisitor&);
DECLARE_INFO;
private:
+ static Structure* createStructure(VM&, JSGlobalObject*, JSValue prototype, IndexingType);
+
static bool getOwnPropertySlot(JSObject*, ExecState*, PropertyName, PropertySlot&);
static void getOwnPropertyNames(JSObject*, ExecState*, PropertyNameArray&, EnumerationMode);
static bool put(JSCell*, ExecState*, PropertyName, JSValue, PutPropertySlot&);
diff --git a/Source/JavaScriptCore/runtime/JSGlobalObject.cpp b/Source/JavaScriptCore/runtime/JSGlobalObject.cpp
index 4c5cbbb..e9c9876 100644
--- a/Source/JavaScriptCore/runtime/JSGlobalObject.cpp
+++ b/Source/JavaScriptCore/runtime/JSGlobalObject.cpp
@@ -537,7 +537,6 @@
m_regExpPrototype.set(vm, this, RegExpPrototype::create(vm, this, RegExpPrototype::createStructure(vm, this, m_objectPrototype.get())));
m_regExpStructure.set(vm, this, RegExpObject::createStructure(vm, this, m_regExpPrototype.get()));
m_regExpMatchesArrayStructure.set(vm, this, createRegExpMatchesArrayStructure(vm, this));
- m_regExpMatchesArraySlowPutStructure.set(vm, this, createRegExpMatchesArraySlowPutStructure(vm, this));
m_moduleRecordStructure.set(vm, this, JSModuleRecord::createStructure(vm, this, m_objectPrototype.get()));
m_moduleNamespaceObjectStructure.set(vm, this, JSModuleNamespaceObject::createStructure(vm, this, jsNull()));
@@ -1052,8 +1051,12 @@
m_arrayStructureForIndexingShapeDuringAllocation[i].set(vm, this, originalArrayStructureForIndexingType(ArrayWithSlowPutArrayStorage));
// Same for any special array structures.
- m_regExpMatchesArrayStructure.set(vm, this, m_regExpMatchesArraySlowPutStructure.get());
-
+ Structure* slowPutStructure;
+ slowPutStructure = createRegExpMatchesArraySlowPutStructure(vm, this);
+ m_regExpMatchesArrayStructure.set(vm, this, slowPutStructure);
+ slowPutStructure = ClonedArguments::createSlowPutStructure(vm, this, m_objectPrototype.get());
+ m_clonedArgumentsStructure.set(vm, this, slowPutStructure);
+
// Make sure that all objects that have indexed storage switch to the slow kind of
// indexed storage.
MarkedArgumentBuffer foundObjects; // Use MarkedArgumentBuffer because switchToSlowPutArrayStorage() may GC.
@@ -1191,7 +1194,6 @@
visitor.append(&thisObject->m_generatorFunctionStructure);
visitor.append(&thisObject->m_iteratorResultObjectStructure);
visitor.append(&thisObject->m_regExpMatchesArrayStructure);
- visitor.append(&thisObject->m_regExpMatchesArraySlowPutStructure);
visitor.append(&thisObject->m_moduleRecordStructure);
visitor.append(&thisObject->m_moduleNamespaceObjectStructure);
visitor.append(&thisObject->m_dollarVMStructure);
diff --git a/Source/JavaScriptCore/runtime/JSGlobalObject.h b/Source/JavaScriptCore/runtime/JSGlobalObject.h
index ea57506..324bffb 100644
--- a/Source/JavaScriptCore/runtime/JSGlobalObject.h
+++ b/Source/JavaScriptCore/runtime/JSGlobalObject.h
@@ -320,7 +320,6 @@
WriteBarrier<Structure> m_dollarVMStructure;
WriteBarrier<Structure> m_iteratorResultObjectStructure;
WriteBarrier<Structure> m_regExpMatchesArrayStructure;
- WriteBarrier<Structure> m_regExpMatchesArraySlowPutStructure;
WriteBarrier<Structure> m_moduleRecordStructure;
WriteBarrier<Structure> m_moduleNamespaceObjectStructure;
WriteBarrier<Structure> m_proxyObjectStructure;
diff --git a/Source/JavaScriptCore/runtime/JSObject.cpp b/Source/JavaScriptCore/runtime/JSObject.cpp
index 08ae792..7200244 100644
--- a/Source/JavaScriptCore/runtime/JSObject.cpp
+++ b/Source/JavaScriptCore/runtime/JSObject.cpp
@@ -768,14 +768,10 @@
return newButterfly->contiguous();
}
-ArrayStorage* JSObject::createArrayStorage(VM& vm, unsigned length, unsigned vectorLength)
+Butterfly* JSObject::createArrayStorageButterfly(VM& vm, JSCell* intendedOwner, Structure* structure, unsigned length, unsigned vectorLength, Butterfly* oldButterfly)
{
- DeferGC deferGC(vm.heap);
- Structure* structure = this->structure(vm);
- IndexingType oldType = indexingType();
- ASSERT_UNUSED(oldType, !hasIndexedProperties(oldType));
Butterfly* newButterfly = Butterfly::createOrGrowArrayRight(
- m_butterfly.get(), vm, this, structure, structure->outOfLineCapacity(), false, 0,
+ oldButterfly, vm, intendedOwner, structure, structure->outOfLineCapacity(), false, 0,
ArrayStorage::sizeFor(vectorLength));
RELEASE_ASSERT(newButterfly);
@@ -787,6 +783,19 @@
result->m_indexBias = 0;
for (size_t i = vectorLength; i--;)
result->m_vector[i].setWithoutWriteBarrier(JSValue());
+
+ return newButterfly;
+}
+
+ArrayStorage* JSObject::createArrayStorage(VM& vm, unsigned length, unsigned vectorLength)
+{
+ DeferGC deferGC(vm.heap);
+ Structure* structure = this->structure(vm);
+ IndexingType oldType = indexingType();
+ ASSERT_UNUSED(oldType, !hasIndexedProperties(oldType));
+
+ Butterfly* newButterfly = createArrayStorageButterfly(vm, this, structure, length, vectorLength, m_butterfly.get());
+ ArrayStorage* result = newButterfly->arrayStorage();
Structure* newStructure = Structure::nonPropertyTransition(vm, structure, structure->suggestedArrayStorageTransition());
setStructureAndButterfly(vm, newStructure, newButterfly);
return result;
diff --git a/Source/JavaScriptCore/runtime/JSObject.h b/Source/JavaScriptCore/runtime/JSObject.h
index ebe485e..bfd24d0 100644
--- a/Source/JavaScriptCore/runtime/JSObject.h
+++ b/Source/JavaScriptCore/runtime/JSObject.h
@@ -902,6 +902,7 @@
void createInitialForValueAndSet(VM&, unsigned index, JSValue);
void convertInt32ForValue(VM&, JSValue);
+ static Butterfly* createArrayStorageButterfly(VM&, JSCell* intendedOwner, Structure*, unsigned length, unsigned vectorLength, Butterfly* oldButterfly = nullptr);
ArrayStorage* createArrayStorage(VM&, unsigned length, unsigned vectorLength);
ArrayStorage* createInitialArrayStorage(VM&);
