Changeset 244211 in webkit


Ignore:
Timestamp:
Apr 11, 2019 11:35:17 PM (5 years ago)
Author:
ysuzuki@apple.com
Message:

[JSC] op_has_indexed_property should not assume subscript part is Uint32
https://bugs.webkit.org/show_bug.cgi?id=196850

Reviewed by Saam Barati.

JSTests:

  • stress/has-indexed-property-should-accept-non-int32.js: Added.

(foo):

Source/JavaScriptCore:

op_has_indexed_property assumed that subscript part is always Uint32. However, this is just a load from non-constant RegisterID,
DFG can store it in double format and can perform OSR exit. op_has_indexed_property should not assume that.
In this patch, instead, we check it with isAnyInt and get uint32_t from AnyInt.

  • jit/JITOpcodes.cpp:

(JSC::JIT::emit_op_has_indexed_property):

  • jit/JITOpcodes32_64.cpp:

(JSC::JIT::emit_op_has_indexed_property):

  • jit/JITOperations.cpp:
  • runtime/CommonSlowPaths.cpp:

(JSC::SLOW_PATH_DECL):

Location:
trunk
Files:
1 added
10 edited

Legend:

Unmodified
Added
Removed
  • trunk/JSTests/ChangeLog

    r244206 r244211  
     12019-04-11  Yusuke Suzuki  <ysuzuki@apple.com>
     2
     3        [JSC] op_has_indexed_property should not assume subscript part is Uint32
     4        https://bugs.webkit.org/show_bug.cgi?id=196850
     5
     6        Reviewed by Saam Barati.
     7
     8        * stress/has-indexed-property-should-accept-non-int32.js: Added.
     9        (foo):
     10
    1112019-04-11  Saam barati  <sbarati@apple.com>
    212
  • trunk/Source/JavaScriptCore/ChangeLog

    r244206 r244211  
     12019-04-11  Yusuke Suzuki  <ysuzuki@apple.com>
     2
     3        [JSC] op_has_indexed_property should not assume subscript part is Uint32
     4        https://bugs.webkit.org/show_bug.cgi?id=196850
     5
     6        Reviewed by Saam Barati.
     7
     8        op_has_indexed_property assumed that subscript part is always Uint32. However, this is just a load from non-constant RegisterID,
     9        DFG can store it in double format and can perform OSR exit. op_has_indexed_property should not assume that.
     10        In this patch, instead, we check it with isAnyInt and get uint32_t from AnyInt.
     11
     12        * jit/JITOpcodes.cpp:
     13        (JSC::JIT::emit_op_has_indexed_property):
     14        * jit/JITOpcodes32_64.cpp:
     15        (JSC::JIT::emit_op_has_indexed_property):
     16        * jit/JITOperations.cpp:
     17        * runtime/CommonSlowPaths.cpp:
     18        (JSC::SLOW_PATH_DECL):
     19
    1202019-04-11  Saam barati  <sbarati@apple.com>
    221
  • trunk/Source/JavaScriptCore/jit/JITOpcodes.cpp

    r242123 r244211  
    12501250    emitGetVirtualRegisters(base, regT0, property, regT1);
    12511251
     1252    emitJumpSlowCaseIfNotInt(regT1);
     1253
    12521254    // This is technically incorrect - we're zero-extending an int32. On the hot path this doesn't matter.
    12531255    // We check the value as if it was a uint32 against the m_vectorLength - which will always fail if
  • trunk/Source/JavaScriptCore/jit/JITOpcodes32_64.cpp

    r240965 r244211  
    11221122    emitJumpSlowCaseIfNotJSCell(base);
    11231123
    1124     emitLoadPayload(property, regT1);
     1124    emitLoad(property, regT3, regT1);
     1125    addSlowCase(branchIfNotInt32(regT3));
    11251126
    11261127    // This is technically incorrect - we're zero-extending an int32. On the hot path this doesn't matter.
  • trunk/Source/JavaScriptCore/jit/JITOperations.cpp

    r244206 r244211  
    20142014   
    20152015    ASSERT(baseValue.isObject());
    2016     ASSERT(subscript.isUInt32());
     2016    ASSERT(subscript.isUInt32AsAnyInt());
    20172017
    20182018    JSObject* object = asObject(baseValue);
     
    20442044    }
    20452045
    2046     uint32_t index = subscript.asUInt32();
     2046    uint32_t index = subscript.asUInt32AsAnyInt();
    20472047    if (object->canGetIndexQuickly(index))
    20482048        return JSValue::encode(JSValue(JSValue::JSTrue));
     
    20652065   
    20662066    ASSERT(baseValue.isObject());
    2067     ASSERT(subscript.isUInt32());
     2067    ASSERT(subscript.isUInt32AsAnyInt());
    20682068
    20692069    JSObject* object = asObject(baseValue);
    2070     uint32_t index = subscript.asUInt32();
     2070    uint32_t index = subscript.asUInt32AsAnyInt();
    20712071    if (object->canGetIndexQuickly(index))
    20722072        return JSValue::encode(JSValue(JSValue::JSTrue));
     
    20782078        byValInfo->arrayProfile->setOutOfBounds();
    20792079    }
    2080     return JSValue::encode(jsBoolean(object->hasPropertyGeneric(exec, subscript.asUInt32(), PropertySlot::InternalMethodType::GetOwnProperty)));
     2080    return JSValue::encode(jsBoolean(object->hasPropertyGeneric(exec, index, PropertySlot::InternalMethodType::GetOwnProperty)));
    20812081}
    20822082   
  • trunk/Source/JavaScriptCore/runtime/ArrayPrototype.cpp

    r242902 r244211  
    15311531    JSArray* otherArray = jsCast<JSArray*>(exec->uncheckedArgument(1));
    15321532    JSValue startValue = exec->uncheckedArgument(2);
    1533     ASSERT(startValue.isAnyInt() && startValue.asAnyInt() >= 0 && startValue.asAnyInt() <= std::numeric_limits<unsigned>::max());
    1534     unsigned startIndex = static_cast<unsigned>(startValue.asAnyInt());
     1533    ASSERT(startValue.isUInt32AsAnyInt());
     1534    unsigned startIndex = startValue.asUInt32AsAnyInt();
    15351535    bool success = resultArray->appendMemcpy(exec, vm, startIndex, otherArray);
    15361536    EXCEPTION_ASSERT(!scope.exception() || !success);
  • trunk/Source/JavaScriptCore/runtime/CommonSlowPaths.cpp

    r244057 r244211  
    904904    JSValue property = GET(bytecode.m_property).jsValue();
    905905    metadata.m_arrayProfile.observeStructure(base->structure(vm));
    906     ASSERT(property.isUInt32());
    907     RETURN(jsBoolean(base->hasPropertyGeneric(exec, property.asUInt32(), PropertySlot::InternalMethodType::GetOwnProperty)));
     906    ASSERT(property.isUInt32AsAnyInt());
     907    RETURN(jsBoolean(base->hasPropertyGeneric(exec, property.asUInt32AsAnyInt(), PropertySlot::InternalMethodType::GetOwnProperty)));
    908908}
    909909
     
    997997    auto bytecode = pc->as<OpToIndexString>();
    998998    JSValue indexValue = GET(bytecode.m_index).jsValue();
    999     ASSERT(indexValue.isAnyInt());
    1000     ASSERT(indexValue.asAnyInt() <= UINT32_MAX);
    1001     ASSERT(indexValue.asAnyInt() >= 0);
    1002     uint32_t index = static_cast<uint32_t>(indexValue.asAnyInt());
    1003     RETURN(jsString(exec, Identifier::from(exec, index).string()));
     999    ASSERT(indexValue.isUInt32AsAnyInt());
     1000    RETURN(jsString(exec, Identifier::from(exec, indexValue.asUInt32AsAnyInt()).string()));
    10041001}
    10051002
  • trunk/Source/JavaScriptCore/runtime/JSCJSValue.h

    r242114 r244211  
    212212    uint32_t asUInt32() const;
    213213    int64_t asAnyInt() const;
     214    uint32_t asUInt32AsAnyInt() const;
     215    int32_t asInt32AsAnyInt() const;
    214216    double asDouble() const;
    215217    bool asBoolean() const;
     
    229231    bool isBoolean() const;
    230232    bool isAnyInt() const;
     233    bool isUInt32AsAnyInt() const;
     234    bool isInt32AsAnyInt() const;
    231235    bool isNumber() const;
    232236    bool isString() const;
  • trunk/Source/JavaScriptCore/runtime/JSCJSValueInlines.h

    r236697 r244211  
    572572}
    573573
     574inline bool JSValue::isInt32AsAnyInt() const
     575{
     576    if (!isAnyInt())
     577        return false;
     578    int64_t value = asAnyInt();
     579    return value >= INT32_MIN && value <= INT32_MAX;
     580}
     581
     582inline int32_t JSValue::asInt32AsAnyInt() const
     583{
     584    ASSERT(isInt32AsAnyInt());
     585    if (isInt32())
     586        return asInt32();
     587    return static_cast<int32_t>(asDouble());
     588}
     589
     590inline bool JSValue::isUInt32AsAnyInt() const
     591{
     592    if (!isAnyInt())
     593        return false;
     594    int64_t value = asAnyInt();
     595    return value >= 0 && value <= UINT32_MAX;
     596}
     597
     598inline uint32_t JSValue::asUInt32AsAnyInt() const
     599{
     600    ASSERT(isUInt32AsAnyInt());
     601    if (isUInt32())
     602        return asUInt32();
     603    return static_cast<uint32_t>(asDouble());
     604}
     605
    574606inline bool JSValue::isString() const
    575607{
  • trunk/Source/JavaScriptCore/runtime/JSGlobalObject.cpp

    r243925 r244211  
    249249    JSString* nameString = asString(exec->uncheckedArgument(4));
    250250
    251     ASSERT(lengthValue.isAnyInt());
    252     ASSERT(lengthValue.asAnyInt() <= INT32_MAX);
    253     ASSERT(lengthValue.asAnyInt() >= INT32_MIN);
    254     int32_t length = lengthValue.toInt32(exec);
    255     scope.assertNoException();
     251    ASSERT(lengthValue.isInt32AsAnyInt());
     252    int32_t length = lengthValue.asInt32AsAnyInt();
    256253
    257254    String name = nameString->value(exec);
Note: See TracChangeset for help on using the changeset viewer.