- Revision
- 202608
- Author
- [email protected]
- Date
- 2016-06-28 21:06:31 -0700 (Tue, 28 Jun 2016)
Log Message
We should not crash there is a finally inside a for-in loop
https://bugs.webkit.org/show_bug.cgi?id=159243
<rdar://problem/27018910>
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):
Modified Paths
Added Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (202607 => 202608)
--- trunk/Source/_javascript_Core/ChangeLog 2016-06-29 04:03:43 UTC (rev 202607)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-06-29 04:06:31 UTC (rev 202608)
@@ -1,3 +1,29 @@
+2016-06-28 Keith Miller <[email protected]>
+
+ We should not crash there is a finally inside a for-in loop
+ https://bugs.webkit.org/show_bug.cgi?id=159243
+ <rdar://problem/27018910>
+
+ 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 <[email protected]>
Assertion failure or crash when accessing let-variable in TDZ with eval with a function in it that returns let variable
Modified: trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp (202607 => 202608)
--- trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp 2016-06-29 04:03:43 UTC (rev 202607)
+++ trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp 2016-06-29 04:06:31 UTC (rev 202608)
@@ -3562,7 +3562,7 @@
Vector<ControlFlowContext> savedScopeContextStack;
Vector<SwitchInfo> savedSwitchContextStack;
- Vector<std::unique_ptr<ForInContext>> savedForInContextStack;
+ Vector<RefPtr<ForInContext>> savedForInContextStack;
Vector<TryContext> poppedTryContexts;
Vector<SymbolTableStackEntry> savedSymbolTableStack;
LabelScopeStore savedLabelScopes;
@@ -3591,7 +3591,7 @@
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 @@
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 @@
{
if (!localRegister)
return;
- m_forInContextStack.append(std::make_unique<IndexedForInContext>(localRegister, indexRegister));
+ m_forInContextStack.append(adoptRef(new IndexedForInContext(localRegister, indexRegister)));
}
void BytecodeGenerator::popIndexedForInScope(RegisterID* localRegister)
@@ -4321,7 +4321,7 @@
{
if (!localRegister)
return;
- m_forInContextStack.append(std::make_unique<StructureForInContext>(localRegister, indexRegister, propertyRegister, enumeratorRegister));
+ m_forInContextStack.append(adoptRef(new StructureForInContext(localRegister, indexRegister, propertyRegister, enumeratorRegister)));
}
void BytecodeGenerator::popStructureForInScope(RegisterID* localRegister)
Modified: trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h (202607 => 202608)
--- trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h 2016-06-29 04:03:43 UTC (rev 202607)
+++ trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h 2016-06-29 04:06:31 UTC (rev 202608)
@@ -102,8 +102,9 @@
FinallyContext finallyContext;
};
- class ForInContext {
+ class ForInContext : public RefCounted<ForInContext> {
WTF_MAKE_FAST_ALLOCATED;
+ WTF_MAKE_NONCOPYABLE(ForInContext);
public:
ForInContext(RegisterID* localRegister)
: m_localRegister(localRegister)
@@ -919,7 +920,7 @@
Vector<ControlFlowContext, 0, UnsafeVectorOverflow> m_scopeContextStack;
Vector<SwitchInfo> m_switchContextStack;
- Vector<std::unique_ptr<ForInContext>> m_forInContextStack;
+ Vector<RefPtr<ForInContext>> m_forInContextStack;
Vector<TryContext> m_tryContextStack;
Vector<RefPtr<Label>> m_generatorResumeLabels;
enum FunctionVariableType : uint8_t { NormalFunctionVariable, GlobalFunctionVariable };
Added: trunk/Source/_javascript_Core/tests/stress/finally-for-in.js (0 => 202608)
--- trunk/Source/_javascript_Core/tests/stress/finally-for-in.js (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/finally-for-in.js 2016-06-29 04:06:31 UTC (rev 202608)
@@ -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);
