Changeset 245769 in webkit


Ignore:
Timestamp:
May 25, 2019 2:25:25 AM (5 years ago)
Author:
Tadeu Zagallo
Message:

JITOperations getByVal should mark negative array indices as out-of-bounds
https://bugs.webkit.org/show_bug.cgi?id=198229

Reviewed by Saam Barati.

JSTests:

  • microbenchmarks/get-by-val-negative-array-index.js: Added.

(foo):

Source/JavaScriptCore:

get_by_val with an array or string as base value and a negative index causes DFG to OSR exit,
but baseline doesn't mark it as out-of-bounds, since it only considers positive indices. This
leads to discarding DFG code, recompiling it and exiting at the same bytecode.

This is observed in the prepack-wtb subtest of JetStream2. In popContext#CdOhFJ, the last item
of the array popped and the new last value is accessed using array[array.length - 1], which
is -1 when the array is empty. It shows a ~0.5% progression in JetStream2, but it's within the
noise.

  • jit/JITOperations.cpp:

(JSC::getByVal):

Location:
trunk
Files:
1 added
3 edited

Legend:

Unmodified
Added
Removed
  • trunk/JSTests/ChangeLog

    r245765 r245769  
     12019-05-25  Tadeu Zagallo  <tzagallo@apple.com>
     2
     3        JITOperations getByVal should mark negative array indices as out-of-bounds
     4        https://bugs.webkit.org/show_bug.cgi?id=198229
     5
     6        Reviewed by Saam Barati.
     7
     8        * microbenchmarks/get-by-val-negative-array-index.js: Added.
     9        (foo):
     10
    1112019-05-24  Justin Michaud  <justin_michaud@apple.com>
    212
  • trunk/Source/JavaScriptCore/ChangeLog

    r245765 r245769  
     12019-05-25  Tadeu Zagallo  <tzagallo@apple.com>
     2
     3        JITOperations getByVal should mark negative array indices as out-of-bounds
     4        https://bugs.webkit.org/show_bug.cgi?id=198229
     5
     6        Reviewed by Saam Barati.
     7
     8        get_by_val with an array or string as base value and a negative index causes DFG to OSR exit,
     9        but baseline doesn't mark it as out-of-bounds, since it only considers positive indices. This
     10        leads to discarding DFG code, recompiling it and exiting at the same bytecode.
     11
     12        This is observed in the prepack-wtb subtest of JetStream2. In popContext#CdOhFJ, the last item
     13        of the array popped and the new last value is accessed using `array[array.length - 1]`, which
     14        is -1 when the array is empty. It shows a ~0.5% progression in JetStream2, but it's within the
     15        noise.
     16
     17        * jit/JITOperations.cpp:
     18        (JSC::getByVal):
     19
    1202019-05-24  Justin Michaud  <justin_michaud@apple.com>
    221
  • trunk/Source/JavaScriptCore/jit/JITOperations.cpp

    r245658 r245769  
    18511851    }
    18521852
    1853     if (subscript.isUInt32()) {
     1853    if (subscript.isInt32()) {
    18541854        ASSERT(exec->bytecodeOffset());
    18551855        byValInfo->tookSlowPath = true;
    18561856
    1857         uint32_t i = subscript.asUInt32();
     1857        int32_t i = subscript.asInt32();
    18581858        if (isJSString(baseValue)) {
    1859             if (asString(baseValue)->canGetIndex(i)) {
     1859            if (i >= 0 && asString(baseValue)->canGetIndex(i)) {
    18601860                ctiPatchCallByReturnAddress(returnAddress, operationGetByValString);
    18611861                RELEASE_AND_RETURN(scope, asString(baseValue)->getIndex(exec, i));
     
    18691869            bool skipMarkingOutOfBounds = false;
    18701870
    1871             if (object->indexingType() == ArrayWithContiguous && i < object->butterfly()->publicLength()) {
     1871            if (object->indexingType() == ArrayWithContiguous && i >= 0 && static_cast<uint32_t>(i) < object->butterfly()->publicLength()) {
    18721872                // FIXME: expand this to ArrayStorage, Int32, and maybe Double:
    18731873                // https://bugs.webkit.org/show_bug.cgi?id=182940
     
    18841884        }
    18851885
    1886         RELEASE_AND_RETURN(scope, baseValue.get(exec, i));
     1886        if (i >= 0)
     1887            RELEASE_AND_RETURN(scope, baseValue.get(exec, static_cast<uint32_t>(i)));
    18871888    }
    18881889
Note: See TracChangeset for help on using the changeset viewer.