Diff
Modified: releases/WebKitGTK/webkit-2.22/JSTests/ChangeLog (238988 => 238989)
--- releases/WebKitGTK/webkit-2.22/JSTests/ChangeLog 2018-12-08 00:25:43 UTC (rev 238988)
+++ releases/WebKitGTK/webkit-2.22/JSTests/ChangeLog 2018-12-08 00:25:46 UTC (rev 238989)
@@ -1,3 +1,13 @@
+2018-09-17 Mark Lam <mark....@apple.com>
+
+ Ensure that ForInContexts are invalidated if their loop local is over-written.
+ https://bugs.webkit.org/show_bug.cgi?id=189571
+ <rdar://problem/44402277>
+
+ Reviewed by Saam Barati.
+
+ * stress/regress-189571.js: Added.
+
2018-08-24 Yusuke Suzuki <yusukesuz...@slowstart.org>
[JSC] Array.prototype.reverse modifies JSImmutableButterfly
Added: releases/WebKitGTK/webkit-2.22/JSTests/stress/regress-189571.js (0 => 238989)
--- releases/WebKitGTK/webkit-2.22/JSTests/stress/regress-189571.js (rev 0)
+++ releases/WebKitGTK/webkit-2.22/JSTests/stress/regress-189571.js 2018-12-08 00:25:46 UTC (rev 238989)
@@ -0,0 +1,151 @@
+function assert(name, actual, expected) {
+ if (actual != expected) {
+ print("FAILED test " + name + ": expected " + expected + ", actual: " + actual);
+ throw "FAILED";
+ }
+}
+
+function checkResult(name, result, expectedK, expectedProp) {
+ assert(name, result[0], expectedK);
+ assert(name, result[1], expectedProp);
+}
+
+// ForIn on Indexed properties.
+
+function testIndexedProperties(o) {
+ for (var k in o) {
+ {
+ function k() { }
+ }
+ return [ k, o[k] ];
+ }
+}
+
+var o = [42];
+for (var i = 0; i < 10000; ++i) {
+ var result = testIndexedProperties(o);
+ checkResult("testIndexedProperties", result, "function k() { }", undefined);
+}
+
+function testIndexedProperties2(o) {
+ for (var k in o) {
+ {
+ k = "boo";
+ function k() { }
+ }
+ return [ k, o[k] ];
+ }
+}
+
+var o = [42];
+for (var i = 0; i < 10000; ++i) {
+ var result = testIndexedProperties2(o);
+ checkResult("testIndexedProperties2", result, "boo", undefined);
+}
+
+function testIndexedProperties3(o) {
+ for (var k in o) {
+ try {
+ } finally {
+ {
+ function k() { }
+ }
+ }
+ return [ k, o[k] ];
+ }
+}
+
+var o = [42];
+for (var i = 0; i < 10000; ++i) {
+ var result = testIndexedProperties3(o);
+ checkResult("testIndexedProperties3", result, "function k() { }", undefined);
+}
+
+function testIndexedProperties4(o) {
+ for (var k in o) {
+ try {
+ } finally {
+ {
+ k = "boo";
+ function k() { }
+ }
+ }
+ return [ k, o[k] ];
+ }
+}
+
+var o = [42];
+for (var i = 0; i < 10000; ++i) {
+ var result = testIndexedProperties4(o);
+ checkResult("testIndexedProperties4", result, "boo", undefined);
+}
+
+// ForIn on Structure properties.
+
+function testStructureProperties(o) {
+ for (var k in o) {
+ {
+ function k() { }
+ }
+ return [ k, o[k] ];
+ }
+}
+
+var o = {a: 42};
+for (var i = 0; i < 10000; ++i) {
+ var result = testStructureProperties(o);
+ checkResult("testStructureProperties", result, "function k() { }", undefined);
+}
+
+function testStructureProperties2(o) {
+ for (var k in o) {
+ {
+ k = 0x1234;
+ function k() { }
+ }
+ return [ k, o[k] ];
+ }
+}
+
+var o = {a: 42};
+for (var i = 0; i < 10000; ++i) {
+ var result = testStructureProperties2(o);
+ checkResult("testStructureProperties2", result, 0x1234, undefined);
+}
+
+function testStructureProperties3(o) {
+ for (var k in o) {
+ try {
+ } finally {
+ {
+ function k() { }
+ }
+ }
+ return [ k, o[k] ];
+ }
+}
+
+var o = {a: 42};
+for (var i = 0; i < 10000; ++i) {
+ var result = testStructureProperties3(o);
+ checkResult("testStructureProperties3", result, "function k() { }", undefined);
+}
+
+function testStructureProperties4(o) {
+ for (var k in o) {
+ try {
+ } finally {
+ {
+ k = 0x1234;
+ function k() { }
+ }
+ }
+ return [ k, o[k] ];
+ }
+}
+
+var o = {a: 42};
+for (var i = 0; i < 10000; ++i) {
+ var result = testStructureProperties4(o);
+ checkResult("testStructureProperties4", result, 0x1234, undefined);
+}
Modified: releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/ChangeLog (238988 => 238989)
--- releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/ChangeLog 2018-12-08 00:25:43 UTC (rev 238988)
+++ releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/ChangeLog 2018-12-08 00:25:46 UTC (rev 238989)
@@ -1,3 +1,52 @@
+2018-09-18 Mark Lam <mark....@apple.com>
+
+ Ensure that ForInContexts are invalidated if their loop local is over-written.
+ https://bugs.webkit.org/show_bug.cgi?id=189571
+ <rdar://problem/44402277>
+
+ Reviewed by Saam Barati.
+
+ Instead of hunting down every place in the BytecodeGenerator that potentially
+ needs to invalidate an enclosing ForInContext (if one exists), we simply iterate
+ the bytecode range of the loop body when the ForInContext is popped, and
+ invalidate the context if we ever find the loop temp variable over-written.
+
+ This has 2 benefits:
+ 1. It ensures that every type of opcode that can write to the loop temp will be
+ handled appropriately, not just the op_mov that we've hunted down.
+ 2. It avoids us having to check the BytecodeGenerator's m_forInContextStack
+ every time we emit an op_mov (or other opcodes that can write to a local)
+ even when we're not inside a for-in loop.
+
+ JSC benchmarks show that that this change is performance neutral.
+
+ * bytecompiler/BytecodeGenerator.cpp:
+ (JSC::BytecodeGenerator::pushIndexedForInScope):
+ (JSC::BytecodeGenerator::popIndexedForInScope):
+ (JSC::BytecodeGenerator::pushStructureForInScope):
+ (JSC::BytecodeGenerator::popStructureForInScope):
+ (JSC::ForInContext::finalize):
+ (JSC::StructureForInContext::finalize):
+ (JSC::IndexedForInContext::finalize):
+ (JSC::BytecodeGenerator::invalidateForInContextForLocal): Deleted.
+ * bytecompiler/BytecodeGenerator.h:
+ (JSC::ForInContext::ForInContext):
+ (JSC::ForInContext::bodyBytecodeStartOffset const):
+ (JSC::StructureForInContext::StructureForInContext):
+ (JSC::IndexedForInContext::IndexedForInContext):
+ * bytecompiler/NodesCodegen.cpp:
+ (JSC::PostfixNode::emitResolve):
+ (JSC::PrefixNode::emitResolve):
+ (JSC::ReadModifyResolveNode::emitBytecode):
+ (JSC::AssignResolveNode::emitBytecode):
+ (JSC::EmptyLetExpression::emitBytecode):
+ (JSC::ForInNode::emitLoopHeader):
+ (JSC::ForOfNode::emitBytecode):
+ (JSC::BindingNode::bindValue const):
+ (JSC::AssignmentElementNode::bindValue const):
+ * runtime/CommonSlowPaths.cpp:
+ (JSC::SLOW_PATH_DECL):
+
2018-09-14 Mark Lam <mark....@apple.com>
Refactor some ForInContext code for better encapsulation.
Modified: releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp (238988 => 238989)
--- releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp 2018-12-08 00:25:43 UTC (rev 238988)
+++ releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp 2018-12-08 00:25:46 UTC (rev 238989)
@@ -36,6 +36,7 @@
#include "BuiltinNames.h"
#include "BytecodeGeneratorification.h"
#include "BytecodeLivenessAnalysis.h"
+#include "BytecodeUseDef.h"
#include "CatchScope.h"
#include "DefinePropertyAttributes.h"
#include "Interpreter.h"
@@ -4624,7 +4625,8 @@
{
if (!localRegister)
return;
- m_forInContextStack.append(adoptRef(*new IndexedForInContext(localRegister, indexRegister)));
+ unsigned bodyBytecodeStartOffset = instructions().size();
+ m_forInContextStack.append(adoptRef(*new IndexedForInContext(localRegister, indexRegister, bodyBytecodeStartOffset)));
}
void BytecodeGenerator::popIndexedForInScope(RegisterID* localRegister)
@@ -4631,7 +4633,8 @@
{
if (!localRegister)
return;
- m_forInContextStack.last()->asIndexedForInContext().finalize(*this);
+ unsigned bodyBytecodeEndOffset = instructions().size();
+ m_forInContextStack.last()->asIndexedForInContext().finalize(*this, m_codeBlock.get(), bodyBytecodeEndOffset);
m_forInContextStack.removeLast();
}
@@ -4734,7 +4737,8 @@
{
if (!localRegister)
return;
- m_forInContextStack.append(adoptRef(*new StructureForInContext(localRegister, indexRegister, propertyRegister, enumeratorRegister)));
+ unsigned bodyBytecodeStartOffset = instructions().size();
+ m_forInContextStack.append(adoptRef(*new StructureForInContext(localRegister, indexRegister, propertyRegister, enumeratorRegister, bodyBytecodeStartOffset)));
}
void BytecodeGenerator::popStructureForInScope(RegisterID* localRegister)
@@ -4741,30 +4745,11 @@
{
if (!localRegister)
return;
- m_forInContextStack.last()->asStructureForInContext().finalize(*this);
+ unsigned bodyBytecodeEndOffset = instructions().size();
+ m_forInContextStack.last()->asStructureForInContext().finalize(*this, m_codeBlock.get(), bodyBytecodeEndOffset);
m_forInContextStack.removeLast();
}
-void BytecodeGenerator::invalidateForInContextForLocal(RegisterID* localRegister)
-{
- // Lexically invalidating ForInContexts is kind of weak sauce, but it only occurs if
- // either of the following conditions is true:
- //
- // (1) The loop iteration variable is re-assigned within the body of the loop.
- // (2) The loop iteration variable is captured in the lexical scope of the function.
- //
- // These two situations occur sufficiently rarely that it's okay to use this style of
- // "analysis" to make iteration faster. If we didn't want to do this, we would either have
- // to perform some flow-sensitive analysis to see if/when the loop iteration variable was
- // reassigned, or we'd have to resort to runtime checks to see if the variable had been
- // reassigned from its original value.
- for (size_t i = m_forInContextStack.size(); i--; ) {
- ForInContext& context = m_forInContextStack[i].get();
- if (context.local() == localRegister)
- context.invalidate();
- }
-}
-
RegisterID* BytecodeGenerator::emitRestParameter(RegisterID* result, unsigned numParametersToSkip)
{
RefPtr<RegisterID> restArrayLength = newTemporary();
@@ -5190,8 +5175,37 @@
emitJumpIfTrue(equivalenceResult, jumpTarget);
}
-void StructureForInContext::finalize(BytecodeGenerator& generator)
+void ForInContext::finalize(BytecodeGenerator& generator, UnlinkedCodeBlock* codeBlock, unsigned bodyBytecodeEndOffset)
{
+ // Lexically invalidating ForInContexts is kind of weak sauce, but it only occurs if
+ // either of the following conditions is true:
+ //
+ // (1) The loop iteration variable is re-assigned within the body of the loop.
+ // (2) The loop iteration variable is captured in the lexical scope of the function.
+ //
+ // These two situations occur sufficiently rarely that it's okay to use this style of
+ // "analysis" to make iteration faster. If we didn't want to do this, we would either have
+ // to perform some flow-sensitive analysis to see if/when the loop iteration variable was
+ // reassigned, or we'd have to resort to runtime checks to see if the variable had been
+ // reassigned from its original value.
+
+ for (unsigned offset = bodyBytecodeStartOffset(); isValid() && offset < bodyBytecodeEndOffset;) {
+ UnlinkedInstruction* instruction = &generator.instructions()[offset];
+ OpcodeID opcodeID = instruction->u.opcode;
+ unsigned opcodeLength = opcodeLengths[opcodeID];
+
+ ASSERT(opcodeID != op_enter);
+ computeDefsForBytecodeOffset(codeBlock, opcodeID, instruction, [&] (UnlinkedCodeBlock*, UnlinkedInstruction*, OpcodeID, int operand) {
+ if (local()->index() == operand)
+ invalidate();
+ });
+ offset += opcodeLength;
+ }
+}
+
+void StructureForInContext::finalize(BytecodeGenerator& generator, UnlinkedCodeBlock* codeBlock, unsigned bodyBytecodeEndOffset)
+{
+ Base::finalize(generator, codeBlock, bodyBytecodeEndOffset);
if (isValid())
return;
@@ -5219,8 +5233,9 @@
}
}
-void IndexedForInContext::finalize(BytecodeGenerator& generator)
+void IndexedForInContext::finalize(BytecodeGenerator& generator, UnlinkedCodeBlock* codeBlock, unsigned bodyBytecodeEndOffset)
{
+ Base::finalize(generator, codeBlock, bodyBytecodeEndOffset);
if (isValid())
return;
Modified: releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h (238988 => 238989)
--- releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h 2018-12-08 00:25:43 UTC (rev 238988)
+++ releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h 2018-12-08 00:25:46 UTC (rev 238989)
@@ -211,23 +211,30 @@
RegisterID* local() const { return m_localRegister.get(); }
protected:
- ForInContext(RegisterID* localRegister, Type type)
+ ForInContext(RegisterID* localRegister, Type type, unsigned bodyBytecodeStartOffset)
: m_localRegister(localRegister)
, m_type(type)
+ , m_bodyBytecodeStartOffset(bodyBytecodeStartOffset)
{ }
+ unsigned bodyBytecodeStartOffset() const { return m_bodyBytecodeStartOffset; }
+
+ void finalize(BytecodeGenerator&, UnlinkedCodeBlock*, unsigned bodyBytecodeEndOffset);
+
private:
RefPtr<RegisterID> m_localRegister;
bool m_isValid { true };
Type m_type;
+ unsigned m_bodyBytecodeStartOffset;
};
class StructureForInContext : public ForInContext {
+ using Base = ForInContext;
public:
using GetInst = std::tuple<unsigned, int, UnlinkedValueProfile>;
- StructureForInContext(RegisterID* localRegister, RegisterID* indexRegister, RegisterID* propertyRegister, RegisterID* enumeratorRegister)
- : ForInContext(localRegister, Type::StructureForIn)
+ StructureForInContext(RegisterID* localRegister, RegisterID* indexRegister, RegisterID* propertyRegister, RegisterID* enumeratorRegister, unsigned bodyBytecodeStartOffset)
+ : ForInContext(localRegister, Type::StructureForIn, bodyBytecodeStartOffset)
, m_indexRegister(indexRegister)
, m_propertyRegister(propertyRegister)
, m_enumeratorRegister(enumeratorRegister)
@@ -243,7 +250,7 @@
m_getInsts.append(GetInst { instIndex, propertyRegIndex, valueProfile });
}
- void finalize(BytecodeGenerator&);
+ void finalize(BytecodeGenerator&, UnlinkedCodeBlock*, unsigned bodyBytecodeEndOffset);
private:
RefPtr<RegisterID> m_indexRegister;
@@ -253,9 +260,10 @@
};
class IndexedForInContext : public ForInContext {
+ using Base = ForInContext;
public:
- IndexedForInContext(RegisterID* localRegister, RegisterID* indexRegister)
- : ForInContext(localRegister, Type::IndexedForIn)
+ IndexedForInContext(RegisterID* localRegister, RegisterID* indexRegister, unsigned bodyBytecodeStartOffset)
+ : ForInContext(localRegister, Type::IndexedForIn, bodyBytecodeStartOffset)
, m_indexRegister(indexRegister)
{
}
@@ -262,7 +270,7 @@
RegisterID* index() const { return m_indexRegister.get(); }
- void finalize(BytecodeGenerator&);
+ void finalize(BytecodeGenerator&, UnlinkedCodeBlock*, unsigned bodyBytecodeEndOffset);
void addGetInst(unsigned instIndex, int propertyIndex) { m_getInsts.append({ instIndex, propertyIndex }); }
private:
@@ -938,7 +946,6 @@
void popIndexedForInScope(RegisterID* local);
void pushStructureForInScope(RegisterID* local, RegisterID* index, RegisterID* property, RegisterID* enumerator);
void popStructureForInScope(RegisterID* local);
- void invalidateForInContextForLocal(RegisterID* local);
LabelScope* breakTarget(const Identifier&);
LabelScope* continueTarget(const Identifier&);
Modified: releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp (238988 => 238989)
--- releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp 2018-12-08 00:25:43 UTC (rev 238988)
+++ releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp 2018-12-08 00:25:46 UTC (rev 238989)
@@ -1556,7 +1556,6 @@
generator.emitReadOnlyExceptionIfNeeded(var);
localReg = generator.move(generator.tempDestination(dst), local);
}
- generator.invalidateForInContextForLocal(local);
RefPtr<RegisterID> oldValue = emitPostIncOrDec(generator, generator.finalDestination(dst), localReg.get(), m_operator);
generator.emitProfileType(localReg.get(), var, divotStart(), divotEnd());
return oldValue.get();
@@ -1769,7 +1768,6 @@
generator.emitReadOnlyExceptionIfNeeded(var);
localReg = generator.move(generator.tempDestination(dst), localReg.get());
} else if (generator.vm()->typeProfiler()) {
- generator.invalidateForInContextForLocal(local);
RefPtr<RegisterID> tempDst = generator.tempDestination(dst);
generator.move(tempDst.get(), localReg.get());
emitIncOrDec(generator, tempDst.get(), m_operator);
@@ -1777,7 +1775,6 @@
generator.emitProfileType(localReg.get(), var, divotStart(), divotEnd());
return generator.move(dst, tempDst.get());
}
- generator.invalidateForInContextForLocal(local);
emitIncOrDec(generator, localReg.get(), m_operator);
return generator.move(dst, localReg.get());
}
@@ -2441,13 +2438,11 @@
generator.move(result.get(), local);
emitReadModifyAssignment(generator, result.get(), result.get(), m_right, m_operator, OperandTypes(ResultType::unknownType(), m_right->resultDescriptor()));
generator.move(local, result.get());
- generator.invalidateForInContextForLocal(local);
generator.emitProfileType(local, divotStart(), divotEnd());
return generator.move(dst, result.get());
}
RegisterID* result = emitReadModifyAssignment(generator, local, local, m_right, m_operator, OperandTypes(ResultType::unknownType(), m_right->resultDescriptor()));
- generator.invalidateForInContextForLocal(local);
generator.emitProfileType(result, divotStart(), divotEnd());
return generator.move(dst, result);
}
@@ -2505,12 +2500,10 @@
generator.emitNode(tempDst.get(), m_right);
generator.move(local, tempDst.get());
generator.emitProfileType(local, var, divotStart(), divotEnd());
- generator.invalidateForInContextForLocal(local);
result = generator.move(dst, tempDst.get());
} else {
RegisterID* right = generator.emitNode(local, m_right);
generator.emitProfileType(right, var, divotStart(), divotEnd());
- generator.invalidateForInContextForLocal(local);
result = generator.move(dst, right);
}
@@ -2752,7 +2745,6 @@
Variable var = generator.variable(m_ident);
if (RegisterID* local = var.local()) {
generator.emitLoad(local, jsUndefined());
- generator.invalidateForInContextForLocal(local);
generator.emitProfileType(local, var, position(), JSTextPosition(-1, position().offset + m_ident.length(), -1));
} else {
RefPtr<RegisterID> scope = generator.emitResolveScope(nullptr, var);
@@ -2966,7 +2958,6 @@
if (var.isReadOnly())
generator.emitReadOnlyExceptionIfNeeded(var);
generator.move(local, propertyName);
- generator.invalidateForInContextForLocal(local);
} else {
if (generator.isStrictMode())
generator.emitExpressionInfo(divot(), divotStart(), divotEnd());
@@ -3034,7 +3025,6 @@
return;
}
generator.move(var.local(), propertyName);
- generator.invalidateForInContextForLocal(var.local());
generator.emitProfileType(propertyName, var, simpleBinding->divotStart(), simpleBinding->divotEnd());
return;
}
@@ -3225,7 +3215,6 @@
if (var.isReadOnly())
generator.emitReadOnlyExceptionIfNeeded(var);
generator.move(local, value);
- generator.invalidateForInContextForLocal(local);
} else {
if (generator.isStrictMode())
generator.emitExpressionInfo(divot(), divotStart(), divotEnd());
@@ -4399,7 +4388,6 @@
return;
}
generator.move(local, value);
- generator.invalidateForInContextForLocal(local);
generator.emitProfileType(local, var, divotStart(), divotEnd());
if (m_bindingContext == AssignmentContext::DeclarationStatement || m_bindingContext == AssignmentContext::ConstDeclarationStatement)
generator.liftTDZCheckIfPossible(var);
@@ -4448,7 +4436,6 @@
if (isReadOnly)
generator.emitReadOnlyExceptionIfNeeded(var);
else {
- generator.invalidateForInContextForLocal(local);
generator.move(local, value);
generator.emitProfileType(local, divotStart(), divotEnd());
}
Modified: releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/runtime/CommonSlowPaths.cpp (238988 => 238989)
--- releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/runtime/CommonSlowPaths.cpp 2018-12-08 00:25:43 UTC (rev 238988)
+++ releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/runtime/CommonSlowPaths.cpp 2018-12-08 00:25:46 UTC (rev 238989)
@@ -810,6 +810,7 @@
JSObject* base = OP(2).jsValue().toObject(exec);
CHECK_EXCEPTION();
JSValue property = OP(3).jsValue();
+ ASSERT(property.isString());
JSString* string = asString(property);
auto propertyName = string->toIdentifier(exec);
CHECK_EXCEPTION();
@@ -821,6 +822,7 @@
BEGIN();
JSValue baseValue = OP_C(2).jsValue();
JSValue property = OP(3).jsValue();
+ ASSERT(property.isString());
JSString* string = asString(property);
auto propertyName = string->toIdentifier(exec);
CHECK_EXCEPTION();
