<!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>[127544] trunk</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/127544">127544</a></dd>
<dt>Author</dt> <dd>barraclough@apple.com</dd>
<dt>Date</dt> <dd>2012-09-04 19:36:45 -0700 (Tue, 04 Sep 2012)</dd>
</dl>

<h3>Log Message</h3>
<pre>inc/dec behave incorrectly operating on a resolved const
https://bugs.webkit.org/show_bug.cgi?id=95815

Reviewed by Geoff Garen.

Source/JavaScriptCore: 

There are two bugs here.

(1) When the value being incremented is const, and the result is ignored, we assume this cannot be observed, and emit no code.
    However if the value being incremented is not a primitive &amp; has a valueOf conversion, then this should be being called.

(2) In the case of a pre-increment of a const value where the result is not ignored, we'll move +/-1 to the destination, then
    add the resolved const value being incremented to this. This is problematic if the destination is a local, and the const
    value being incremented has a valueOf conversion that throws - the destination will be modified erroneously. Instead, we
    need to use a temporary location.

* bytecompiler/NodesCodegen.cpp:
(JSC::PostfixResolveNode::emitBytecode):
(JSC::PrefixResolveNode::emitBytecode):
    - always at least perform a toNumber conversion, use tempDestination when reducing inc/dec to an add +/-1.

LayoutTests: 

Added test cases.

* fast/js/inc-const-valueOf-expected.txt: Added.
* fast/js/inc-const-valueOf.html: Added.
* fast/js/script-tests/inc-const-valueOf.js: Added.
(testPostIncConstVarWithIgnoredResult.const.a.valueOf):
(testPostIncConstVarWithIgnoredResult):
    test that 'a++' results in a valueOf call, where 'a' is const.
(testPreIncConstVarWithIgnoredResult.const.a.valueOf):
(testPreIncConstVarWithIgnoredResult):
    test that '++a' results in a valueOf call, where 'a' is const.
(testPreIncConstVarWithAssign.const.a.valueOf):
(testPreIncConstVarWithAssign):
    test that 'b = ++a' does not erroneously update 'b', where 'a' is const.</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCorebytecompilerNodesCodegencpp">trunk/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp</a></li>
</ul>

<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsfastjsincconstvalueOfexpectedtxt">trunk/LayoutTests/fast/js/inc-const-valueOf-expected.txt</a></li>
<li><a href="#trunkLayoutTestsfastjsincconstvalueOfhtml">trunk/LayoutTests/fast/js/inc-const-valueOf.html</a></li>
<li><a href="#trunkLayoutTestsfastjsscripttestsincconstvalueOfjs">trunk/LayoutTests/fast/js/script-tests/inc-const-valueOf.js</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (127543 => 127544)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog        2012-09-05 02:28:29 UTC (rev 127543)
+++ trunk/LayoutTests/ChangeLog        2012-09-05 02:36:45 UTC (rev 127544)
</span><span class="lines">@@ -1,3 +1,25 @@
</span><ins>+2012-09-04  Gavin Barraclough  &lt;barraclough@apple.com&gt;
+
+        inc/dec behave incorrectly operating on a resolved const
+        https://bugs.webkit.org/show_bug.cgi?id=95815
+
+        Reviewed by Geoff Garen.
+
+        Added test cases.
+
+        * fast/js/inc-const-valueOf-expected.txt: Added.
+        * fast/js/inc-const-valueOf.html: Added.
+        * fast/js/script-tests/inc-const-valueOf.js: Added.
+        (testPostIncConstVarWithIgnoredResult.const.a.valueOf):
+        (testPostIncConstVarWithIgnoredResult):
+            test that 'a++' results in a valueOf call, where 'a' is const.
+        (testPreIncConstVarWithIgnoredResult.const.a.valueOf):
+        (testPreIncConstVarWithIgnoredResult):
+            test that '++a' results in a valueOf call, where 'a' is const.
+        (testPreIncConstVarWithAssign.const.a.valueOf):
+        (testPreIncConstVarWithAssign):
+            test that 'b = ++a' does not erroneously update 'b', where 'a' is const.
+
</ins><span class="cx"> 2012-09-04  Kent Tamura  &lt;tkent@chromium.org&gt;
</span><span class="cx"> 
</span><span class="cx">         [Chromium] Test expectation update
</span></span></pre></div>
<a id="trunkLayoutTestsfastjsincconstvalueOfexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/js/inc-const-valueOf-expected.txt (0 => 127544)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/js/inc-const-valueOf-expected.txt                                (rev 0)
+++ trunk/LayoutTests/fast/js/inc-const-valueOf-expected.txt        2012-09-05 02:36:45 UTC (rev 127544)
</span><span class="lines">@@ -0,0 +1,12 @@
</span><ins>+Test for regression against
+
+On success, you will see a series of &quot;PASS&quot; messages, followed by &quot;TEST COMPLETE&quot;.
+
+
+PASS testPostIncConstVarWithIgnoredResult() is true
+PASS testPreIncConstVarWithIgnoredResult() is true
+PASS testPreIncConstVarWithAssign() is true
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
</ins></span></pre></div>
<a id="trunkLayoutTestsfastjsincconstvalueOfhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/js/inc-const-valueOf.html (0 => 127544)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/js/inc-const-valueOf.html                                (rev 0)
+++ trunk/LayoutTests/fast/js/inc-const-valueOf.html        2012-09-05 02:36:45 UTC (rev 127544)
</span><span class="lines">@@ -0,0 +1,10 @@
</span><ins>+&lt;!DOCTYPE HTML PUBLIC &quot;-//IETF//DTD HTML//EN&quot;&gt;
+&lt;html&gt;
+&lt;head&gt;
+&lt;script src=&quot;resources/js-test-pre.js&quot;&gt;&lt;/script&gt;
+&lt;/head&gt;
+&lt;body&gt;
+&lt;script src=&quot;script-tests/inc-const-valueOf.js&quot;&gt;&lt;/script&gt;
+&lt;script src=&quot;resources/js-test-post.js&quot;&gt;&lt;/script&gt;
+&lt;/body&gt;
+&lt;/html&gt;
</ins></span></pre></div>
<a id="trunkLayoutTestsfastjsscripttestsincconstvalueOfjs"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/js/script-tests/inc-const-valueOf.js (0 => 127544)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/js/script-tests/inc-const-valueOf.js                                (rev 0)
+++ trunk/LayoutTests/fast/js/script-tests/inc-const-valueOf.js        2012-09-05 02:36:45 UTC (rev 127544)
</span><span class="lines">@@ -0,0 +1,56 @@
</span><ins>+description(
+'Test for regression against &lt;a href=&quot;https://bugs.webkit.org/show_bug.cgi?id=95815&quot;&gt;'
+);
+
+function testPostIncConstVarWithIgnoredResult()
+{
+    var okay = false;
+    const a = {
+        valueOf: (function(){
+            okay = true;
+        })
+    };
+
+    a++;
+
+    return okay;
+}
+
+function testPreIncConstVarWithIgnoredResult()
+{
+    var okay = false;
+    const a = {
+        valueOf: (function(){
+            okay = true;
+        })
+    };
+
+    ++a;
+
+    return okay;
+}
+
+function testPreIncConstVarWithAssign()
+{
+    var okay = false;
+    var x = 42;
+    const a = {
+        valueOf: (function(){
+            throw x == 42;
+        })
+    };
+
+    try {
+        x = ++a;
+    } catch (e) {
+        okay = e
+    };
+
+    return okay;
+}
+
+shouldBeTrue('testPostIncConstVarWithIgnoredResult()');
+shouldBeTrue('testPreIncConstVarWithIgnoredResult()');
+shouldBeTrue('testPreIncConstVarWithAssign()');
+
+successfullyParsed = true;
</ins></span></pre></div>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (127543 => 127544)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog        2012-09-05 02:28:29 UTC (rev 127543)
+++ trunk/Source/JavaScriptCore/ChangeLog        2012-09-05 02:36:45 UTC (rev 127544)
</span><span class="lines">@@ -1,3 +1,25 @@
</span><ins>+2012-09-04  Gavin Barraclough  &lt;barraclough@apple.com&gt;
+
+        inc/dec behave incorrectly operating on a resolved const
+        https://bugs.webkit.org/show_bug.cgi?id=95815
+
+        Reviewed by Geoff Garen.
+
+        There are two bugs here.
+
+        (1) When the value being incremented is const, and the result is ignored, we assume this cannot be observed, and emit no code.
+            However if the value being incremented is not a primitive &amp; has a valueOf conversion, then this should be being called.
+
+        (2) In the case of a pre-increment of a const value where the result is not ignored, we'll move +/-1 to the destination, then
+            add the resolved const value being incremented to this. This is problematic if the destination is a local, and the const
+            value being incremented has a valueOf conversion that throws - the destination will be modified erroneously. Instead, we
+            need to use a temporary location.
+
+        * bytecompiler/NodesCodegen.cpp:
+        (JSC::PostfixResolveNode::emitBytecode):
+        (JSC::PrefixResolveNode::emitBytecode):
+            - always at least perform a toNumber conversion, use tempDestination when reducing inc/dec to an add +/-1.
+
</ins><span class="cx"> 2012-09-04  Filip Pizlo  &lt;fpizlo@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         DFG GetByVal for JSArrays shouldn't OSR exit every time that the index is out of bound
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorebytecompilerNodesCodegencpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp (127543 => 127544)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp        2012-09-05 02:28:29 UTC (rev 127543)
+++ trunk/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp        2012-09-05 02:36:45 UTC (rev 127544)
</span><span class="lines">@@ -609,11 +609,8 @@
</span><span class="cx">     ResolveResult resolveResult = generator.resolve(m_ident);
</span><span class="cx"> 
</span><span class="cx">     if (RegisterID* local = resolveResult.local()) {
</span><del>-        if (resolveResult.isReadOnly()) {
-            if (dst == generator.ignoredResult())
-                return 0;
</del><ins>+        if (resolveResult.isReadOnly())
</ins><span class="cx">             return generator.emitToJSNumber(generator.finalDestination(dst), local);
</span><del>-        }
</del><span class="cx">         if (dst == generator.ignoredResult())
</span><span class="cx">             return emitPreIncOrDec(generator, local, m_operator);
</span><span class="cx">         return emitPostIncOrDec(generator, generator.finalDestination(dst), local, m_operator);
</span><span class="lines">@@ -796,9 +793,10 @@
</span><span class="cx">     if (RegisterID* local = resolveResult.local()) {
</span><span class="cx">         if (resolveResult.isReadOnly()) {
</span><span class="cx">             if (dst == generator.ignoredResult())
</span><del>-                return 0;
-            RefPtr&lt;RegisterID&gt; r0 = generator.emitLoad(generator.finalDestination(dst), (m_operator == OpPlusPlus) ? 1.0 : -1.0);
-            return generator.emitBinaryOp(op_add, r0.get(), local, r0.get(), OperandTypes());
</del><ins>+                return generator.emitToJSNumber(generator.newTemporary(), local);
+            RefPtr&lt;RegisterID&gt; r0 = generator.emitLoad(generator.tempDestination(dst), (m_operator == OpPlusPlus) ? 1.0 : -1.0);
+            generator.emitBinaryOp(op_add, r0.get(), local, r0.get(), OperandTypes());
+            return generator.moveToDestinationIfNeeded(dst, r0.get());
</ins><span class="cx">         }
</span><span class="cx">         emitPreIncOrDec(generator, local, m_operator);
</span><span class="cx">         return generator.moveToDestinationIfNeeded(dst, local);
</span></span></pre>
</div>
</div>

</body>
</html>