Ignore:
Timestamp:
Sep 4, 2012, 7:36:45 PM (13 years ago)
Author:
[email protected]
Message:

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.

File:
1 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp

    r127393 r127544  
    610610
    611611    if (RegisterID* local = resolveResult.local()) {
    612         if (resolveResult.isReadOnly()) {
    613             if (dst == generator.ignoredResult())
    614                 return 0;
     612        if (resolveResult.isReadOnly())
    615613            return generator.emitToJSNumber(generator.finalDestination(dst), local);
    616         }
    617614        if (dst == generator.ignoredResult())
    618615            return emitPreIncOrDec(generator, local, m_operator);
     
    797794        if (resolveResult.isReadOnly()) {
    798795            if (dst == generator.ignoredResult())
    799                 return 0;
    800             RefPtr<RegisterID> r0 = generator.emitLoad(generator.finalDestination(dst), (m_operator == OpPlusPlus) ? 1.0 : -1.0);
    801             return generator.emitBinaryOp(op_add, r0.get(), local, r0.get(), OperandTypes());
     796                return generator.emitToJSNumber(generator.newTemporary(), local);
     797            RefPtr<RegisterID> r0 = generator.emitLoad(generator.tempDestination(dst), (m_operator == OpPlusPlus) ? 1.0 : -1.0);
     798            generator.emitBinaryOp(op_add, r0.get(), local, r0.get(), OperandTypes());
     799            return generator.moveToDestinationIfNeeded(dst, r0.get());
    802800        }
    803801        emitPreIncOrDec(generator, local, m_operator);
Note: See TracChangeset for help on using the changeset viewer.