Commit 89f4b827 authored by barraclough@apple.com's avatar barraclough@apple.com

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 & 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.



git-svn-id: http://svn.webkit.org/repository/webkit/trunk@127544 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 7aaa1420
2012-09-04 Gavin Barraclough <barraclough@apple.com>
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.
2012-09-04 Kent Tamura <tkent@chromium.org>
[Chromium] Test expectation update
Test for regression against
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
PASS testPostIncConstVarWithIgnoredResult() is true
PASS testPreIncConstVarWithIgnoredResult() is true
PASS testPreIncConstVarWithAssign() is true
PASS successfullyParsed is true
TEST COMPLETE
<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
<html>
<head>
<script src="resources/js-test-pre.js"></script>
</head>
<body>
<script src="script-tests/inc-const-valueOf.js"></script>
<script src="resources/js-test-post.js"></script>
</body>
</html>
description(
'Test for regression against <a href="https://bugs.webkit.org/show_bug.cgi?id=95815">'
);
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;
2012-09-04 Gavin Barraclough <barraclough@apple.com>
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 & 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.
2012-09-04 Filip Pizlo <fpizlo@apple.com>
DFG GetByVal for JSArrays shouldn't OSR exit every time that the index is out of bound
......
......@@ -609,11 +609,8 @@ RegisterID* PostfixResolveNode::emitBytecode(BytecodeGenerator& generator, Regis
ResolveResult resolveResult = generator.resolve(m_ident);
if (RegisterID* local = resolveResult.local()) {
if (resolveResult.isReadOnly()) {
if (dst == generator.ignoredResult())
return 0;
if (resolveResult.isReadOnly())
return generator.emitToJSNumber(generator.finalDestination(dst), local);
}
if (dst == generator.ignoredResult())
return emitPreIncOrDec(generator, local, m_operator);
return emitPostIncOrDec(generator, generator.finalDestination(dst), local, m_operator);
......@@ -796,9 +793,10 @@ RegisterID* PrefixResolveNode::emitBytecode(BytecodeGenerator& generator, Regist
if (RegisterID* local = resolveResult.local()) {
if (resolveResult.isReadOnly()) {
if (dst == generator.ignoredResult())
return 0;
RefPtr<RegisterID> r0 = generator.emitLoad(generator.finalDestination(dst), (m_operator == OpPlusPlus) ? 1.0 : -1.0);
return generator.emitBinaryOp(op_add, r0.get(), local, r0.get(), OperandTypes());
return generator.emitToJSNumber(generator.newTemporary(), local);
RefPtr<RegisterID> 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());
}
emitPreIncOrDec(generator, local, m_operator);
return generator.moveToDestinationIfNeeded(dst, local);
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment