<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[202608] trunk/Source/JavaScriptCore</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/202608">202608</a></dd>
<dt>Author</dt> <dd>keith_miller@apple.com</dd>
<dt>Date</dt> <dd>2016-06-28 21:06:31 -0700 (Tue, 28 Jun 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>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):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCorebytecompilerBytecodeGeneratorcpp">trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCorebytecompilerBytecodeGeneratorh">trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoretestsstressfinallyforinjs">trunk/Source/JavaScriptCore/tests/stress/finally-for-in.js</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (202607 => 202608)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2016-06-29 04:03:43 UTC (rev 202607)
+++ trunk/Source/JavaScriptCore/ChangeLog 2016-06-29 04:06:31 UTC (rev 202608)
</span><span class="lines">@@ -1,3 +1,29 @@
</span><ins>+2016-06-28 Keith Miller <keith_miller@apple.com>
+
+ 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):
+
</ins><span class="cx"> 2016-06-28 Saam Barati <sbarati@apple.com>
</span><span class="cx">
</span><span class="cx"> Assertion failure or crash when accessing let-variable in TDZ with eval with a function in it that returns let variable
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorebytecompilerBytecodeGeneratorcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp (202607 => 202608)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp 2016-06-29 04:03:43 UTC (rev 202607)
+++ trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp 2016-06-29 04:06:31 UTC (rev 202608)
</span><span class="lines">@@ -3562,7 +3562,7 @@
</span><span class="cx">
</span><span class="cx"> Vector<ControlFlowContext> savedScopeContextStack;
</span><span class="cx"> Vector<SwitchInfo> savedSwitchContextStack;
</span><del>- Vector<std::unique_ptr<ForInContext>> savedForInContextStack;
</del><ins>+ Vector<RefPtr<ForInContext>> savedForInContextStack;
</ins><span class="cx"> Vector<TryContext> poppedTryContexts;
</span><span class="cx"> Vector<SymbolTableStackEntry> savedSymbolTableStack;
</span><span class="cx"> LabelScopeStore savedLabelScopes;
</span><span class="lines">@@ -3591,7 +3591,7 @@
</span><span class="cx"> m_switchContextStack.shrink(finallyContext.switchContextStackSize);
</span><span class="cx"> }
</span><span class="cx"> if (flipForIns) {
</span><del>- savedForInContextStack.swap(m_forInContextStack);
</del><ins>+ savedForInContextStack = m_forInContextStack;
</ins><span class="cx"> m_forInContextStack.shrink(finallyContext.forInContextStackSize);
</span><span class="cx"> }
</span><span class="cx"> if (flipTries) {
</span><span class="lines">@@ -3641,7 +3641,7 @@
</span><span class="cx"> if (flipSwitches)
</span><span class="cx"> m_switchContextStack = savedSwitchContextStack;
</span><span class="cx"> if (flipForIns)
</span><del>- m_forInContextStack.swap(savedForInContextStack);
</del><ins>+ m_forInContextStack = savedForInContextStack;
</ins><span class="cx"> if (flipTries) {
</span><span class="cx"> ASSERT(m_tryContextStack.size() == finallyContext.tryContextStackSize);
</span><span class="cx"> for (unsigned i = poppedTryContexts.size(); i--;) {
</span><span class="lines">@@ -4211,7 +4211,7 @@
</span><span class="cx"> {
</span><span class="cx"> if (!localRegister)
</span><span class="cx"> return;
</span><del>- m_forInContextStack.append(std::make_unique<IndexedForInContext>(localRegister, indexRegister));
</del><ins>+ m_forInContextStack.append(adoptRef(new IndexedForInContext(localRegister, indexRegister)));
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void BytecodeGenerator::popIndexedForInScope(RegisterID* localRegister)
</span><span class="lines">@@ -4321,7 +4321,7 @@
</span><span class="cx"> {
</span><span class="cx"> if (!localRegister)
</span><span class="cx"> return;
</span><del>- m_forInContextStack.append(std::make_unique<StructureForInContext>(localRegister, indexRegister, propertyRegister, enumeratorRegister));
</del><ins>+ m_forInContextStack.append(adoptRef(new StructureForInContext(localRegister, indexRegister, propertyRegister, enumeratorRegister)));
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void BytecodeGenerator::popStructureForInScope(RegisterID* localRegister)
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorebytecompilerBytecodeGeneratorh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h (202607 => 202608)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h 2016-06-29 04:03:43 UTC (rev 202607)
+++ trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h 2016-06-29 04:06:31 UTC (rev 202608)
</span><span class="lines">@@ -102,8 +102,9 @@
</span><span class="cx"> FinallyContext finallyContext;
</span><span class="cx"> };
</span><span class="cx">
</span><del>- class ForInContext {
</del><ins>+ class ForInContext : public RefCounted<ForInContext> {
</ins><span class="cx"> WTF_MAKE_FAST_ALLOCATED;
</span><ins>+ WTF_MAKE_NONCOPYABLE(ForInContext);
</ins><span class="cx"> public:
</span><span class="cx"> ForInContext(RegisterID* localRegister)
</span><span class="cx"> : m_localRegister(localRegister)
</span><span class="lines">@@ -919,7 +920,7 @@
</span><span class="cx">
</span><span class="cx"> Vector<ControlFlowContext, 0, UnsafeVectorOverflow> m_scopeContextStack;
</span><span class="cx"> Vector<SwitchInfo> m_switchContextStack;
</span><del>- Vector<std::unique_ptr<ForInContext>> m_forInContextStack;
</del><ins>+ Vector<RefPtr<ForInContext>> m_forInContextStack;
</ins><span class="cx"> Vector<TryContext> m_tryContextStack;
</span><span class="cx"> Vector<RefPtr<Label>> m_generatorResumeLabels;
</span><span class="cx"> enum FunctionVariableType : uint8_t { NormalFunctionVariable, GlobalFunctionVariable };
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoretestsstressfinallyforinjs"></a>
<div class="addfile"><h4>Added: trunk/Source/JavaScriptCore/tests/stress/finally-for-in.js (0 => 202608)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/tests/stress/finally-for-in.js (rev 0)
+++ trunk/Source/JavaScriptCore/tests/stress/finally-for-in.js 2016-06-29 04:06:31 UTC (rev 202608)
</span><span class="lines">@@ -0,0 +1,38 @@
</span><ins>+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);
</ins></span></pre>
</div>
</div>
</body>
</html>