diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog index 75bf6ef07e5c..486bc21508cf 100644 --- a/Source/JavaScriptCore/ChangeLog +++ b/Source/JavaScriptCore/ChangeLog @@ -1,3 +1,29 @@ +2016-06-28 Keith Miller + + We should not crash there is a finally inside a for-in loop + https://bugs.webkit.org/show_bug.cgi?id=159243 + + + Reviewed by Benjamin Poulain. + + Previously we would swap the m_forInContext with an empty vector + then attempt to shrink the size of m_forInContext by the amount + we expected. This meant that if there was more than one ForInContext + on the stack and we wanted to pop exactly one off we would crash. + This patch makes ForInContexts RefCounted so they can be duplicated + into other vectors. It also has ForInContexts copy the entire stack + rather than do the swap that we did before. This makes ForInContexts + work the same as the other contexts. + + * bytecompiler/BytecodeGenerator.cpp: + (JSC::BytecodeGenerator::emitComplexPopScopes): + (JSC::BytecodeGenerator::pushIndexedForInScope): + (JSC::BytecodeGenerator::pushStructureForInScope): + * bytecompiler/BytecodeGenerator.h: + * tests/stress/finally-for-in.js: Added. + (repeat): + (createSimple): + 2016-06-28 Saam Barati Assertion failure or crash when accessing let-variable in TDZ with eval with a function in it that returns let variable diff --git a/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp b/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp index f34347e84796..2ef382317c02 100644 --- a/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp +++ b/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp @@ -3562,7 +3562,7 @@ void BytecodeGenerator::emitComplexPopScopes(RegisterID* scope, ControlFlowConte Vector savedScopeContextStack; Vector savedSwitchContextStack; - Vector> savedForInContextStack; + Vector> savedForInContextStack; Vector poppedTryContexts; Vector savedSymbolTableStack; LabelScopeStore savedLabelScopes; @@ -3591,7 +3591,7 @@ void BytecodeGenerator::emitComplexPopScopes(RegisterID* scope, ControlFlowConte m_switchContextStack.shrink(finallyContext.switchContextStackSize); } if (flipForIns) { - savedForInContextStack.swap(m_forInContextStack); + savedForInContextStack = m_forInContextStack; m_forInContextStack.shrink(finallyContext.forInContextStackSize); } if (flipTries) { @@ -3641,7 +3641,7 @@ void BytecodeGenerator::emitComplexPopScopes(RegisterID* scope, ControlFlowConte if (flipSwitches) m_switchContextStack = savedSwitchContextStack; if (flipForIns) - m_forInContextStack.swap(savedForInContextStack); + m_forInContextStack = savedForInContextStack; if (flipTries) { ASSERT(m_tryContextStack.size() == finallyContext.tryContextStackSize); for (unsigned i = poppedTryContexts.size(); i--;) { @@ -4211,7 +4211,7 @@ void BytecodeGenerator::pushIndexedForInScope(RegisterID* localRegister, Registe { if (!localRegister) return; - m_forInContextStack.append(std::make_unique(localRegister, indexRegister)); + m_forInContextStack.append(adoptRef(new IndexedForInContext(localRegister, indexRegister))); } void BytecodeGenerator::popIndexedForInScope(RegisterID* localRegister) @@ -4321,7 +4321,7 @@ void BytecodeGenerator::pushStructureForInScope(RegisterID* localRegister, Regis { if (!localRegister) return; - m_forInContextStack.append(std::make_unique(localRegister, indexRegister, propertyRegister, enumeratorRegister)); + m_forInContextStack.append(adoptRef(new StructureForInContext(localRegister, indexRegister, propertyRegister, enumeratorRegister))); } void BytecodeGenerator::popStructureForInScope(RegisterID* localRegister) diff --git a/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h b/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h index 8dce299087c7..f7c28a3ff1e8 100644 --- a/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h +++ b/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h @@ -102,8 +102,9 @@ namespace JSC { FinallyContext finallyContext; }; - class ForInContext { + class ForInContext : public RefCounted { WTF_MAKE_FAST_ALLOCATED; + WTF_MAKE_NONCOPYABLE(ForInContext); public: ForInContext(RegisterID* localRegister) : m_localRegister(localRegister) @@ -919,7 +920,7 @@ namespace JSC { Vector m_scopeContextStack; Vector m_switchContextStack; - Vector> m_forInContextStack; + Vector> m_forInContextStack; Vector m_tryContextStack; Vector> m_generatorResumeLabels; enum FunctionVariableType : uint8_t { NormalFunctionVariable, GlobalFunctionVariable }; diff --git a/Source/JavaScriptCore/tests/stress/finally-for-in.js b/Source/JavaScriptCore/tests/stress/finally-for-in.js new file mode 100644 index 000000000000..82ffcb47bc33 --- /dev/null +++ b/Source/JavaScriptCore/tests/stress/finally-for-in.js @@ -0,0 +1,38 @@ +function repeat(count, thunk) { + let result = ""; + for (let i = 0; i < count; i++) + result += thunk(i); + return result; +} + +function createSimple(outerDepth, innerDepth, returnDepth) { + return Function( + ` + return (function(arg) { + ${repeat(outerDepth, (i) => `for (let a${i} in arg) ` + "{\n" )} + try { + ${repeat(innerDepth, (i) => `for (let b${i} in arg) ` + "{\n" )} + return {}; + ${repeat(innerDepth, () => "}")} + } + finally { return a${returnDepth}} + ${repeat(outerDepth, () => "}")} + }) + ` + )(); +} + +function test(result, argument, ...args) { + let f = createSimple(...args); + + let r = f(argument); + if (r !== result) { + throw new Error(r); + } +} + + +test("0", [1,2], 1, 1, 0); +test("0", [1,2], 2, 1, 0); +test("0", [1,2], 2, 4, 1); +test("0", [1,2], 1, 0, 0);