⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 266813 in webkit


Ignore:
Timestamp:
Sep 9, 2020, 9:47:38 PM (6 years ago)
Author:
sbarati@apple.com
Message:

OutOfBoundsSaneChain operations should use their own heap locations
​https://bugs.webkit.org/show_bug.cgi?id=216328
<rdar://problem/68568039>

Reviewed by Keith Miller.

JSTests:

  • stress/out-of-bounds-sane-chain-need-their-own-heap-location.js: Added.

(foo):

Source/JavaScriptCore:

There is code in local CSE that does some basic bounds check elimination
for PutByVal. It does this analysis by seeing if a particular heap location
is already defined, and if so, it eliminates the bounds check for the
PutByVal. This doesn't work for OutOfBoundsSaneChain for the obvious reason
that these GetByVals are not proven to be in bounds. So GetByVal's in the
OutOfBoundsSaneChain mode reusing non OutOfBoundsSaneChain heap locations
can lead to a bug where we mistakenly remove a bounds check. The fix is to
have all OutOfBoundsSaneChain operations use distinct heaps, and for CSE to
not query those heaps.

  • dfg/DFGArrayMode.h:

(JSC::DFG::ArrayMode::isAnySaneChain const): Deleted.

  • dfg/DFGClobberize.h:

(JSC::DFG::clobberize):

  • dfg/DFGHeapLocation.cpp:

(WTF::printInternal):

  • dfg/DFGHeapLocation.h:
Location:
trunk
Files:
1 added
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/JSTests/ChangeLog

    r266778 r266813  
     12020-09-09  Saam Barati  <sbarati@apple.com>
     2
     3        OutOfBoundsSaneChain operations should use their own heap locations
     4        https://bugs.webkit.org/show_bug.cgi?id=216328
     5        <rdar://problem/68568039>
     6
     7        Reviewed by Keith Miller.
     8
     9        * stress/out-of-bounds-sane-chain-need-their-own-heap-location.js: Added.
     10        (foo):
     11
    1122020-09-09  Alexey Shvayka  <shvaikalesh@gmail.com>
    213

  • trunk/Source/JavaScriptCore/ChangeLog

    r266795 r266813  
     12020-09-09  Saam Barati  <sbarati@apple.com>
     2
     3        OutOfBoundsSaneChain operations should use their own heap locations
     4        https://bugs.webkit.org/show_bug.cgi?id=216328
     5        <rdar://problem/68568039>
     6
     7        Reviewed by Keith Miller.
     8
     9        There is code in local CSE that does some basic bounds check elimination
     10        for PutByVal. It does this analysis by seeing if a particular heap location
     11        is already defined, and if so, it eliminates the bounds check for the
     12        PutByVal. This doesn't work for OutOfBoundsSaneChain for the obvious reason
     13        that these GetByVals are not proven to be in bounds. So GetByVal's in the
     14        OutOfBoundsSaneChain mode reusing non OutOfBoundsSaneChain heap locations
     15        can lead to a bug where we mistakenly remove a bounds check. The fix is to
     16        have all OutOfBoundsSaneChain operations use distinct heaps, and for CSE to
     17        not query those heaps.
     18
     19        * dfg/DFGArrayMode.h:
     20        (JSC::DFG::ArrayMode::isAnySaneChain const): Deleted.
     21        * dfg/DFGClobberize.h:
     22        (JSC::DFG::clobberize):
     23        * dfg/DFGHeapLocation.cpp:
     24        (WTF::printInternal):
     25        * dfg/DFGHeapLocation.h:
     26
    1272020-09-09  Keith Miller  <keith_miller@apple.com>
    228

  • trunk/Source/JavaScriptCore/dfg/DFGArrayMode.h

    r265775 r266813  
    288288    }
    289289
    290     bool isAnySaneChain() const
    291     {
    292         return isInBoundsSaneChain() || isOutOfBoundsSaneChain();
    293     }
    294    
    295290    bool isOutOfBounds() const
    296291    {

  • trunk/Source/JavaScriptCore/dfg/DFGClobberize.h

    r266095 r266813  
    938938                read(Butterfly_publicLength);
    939939                read(IndexedInt32Properties);
    940                 LocationKind kind = mode.isOutOfBoundsSaneChain() ? IndexedPropertyInt32OrOtherLoc : indexedPropertyLoc;
     940                LocationKind kind = mode.isOutOfBoundsSaneChain() ? IndexedPropertyInt32OutOfBoundsSaneChainLoc : indexedPropertyLoc;
    941941                def(HeapLocation(kind, IndexedInt32Properties, graph.varArgChild(node, 0), graph.varArgChild(node, 1)), LazyNode(node));
    942942                return;
    … …  
    950950                read(IndexedDoubleProperties);
    951951                LocationKind kind;
    952                 if (node->hasDoubleResult())
    953                     kind = mode.isAnySaneChain() ? IndexedPropertyDoubleSaneChainLoc : IndexedPropertyDoubleLoc;
    954                 else {
     952                if (node->hasDoubleResult()) {
     953                    if (mode.isInBoundsSaneChain())
     954                        kind = IndexedPropertyDoubleSaneChainLoc;
     955                    else if (mode.isOutOfBoundsSaneChain())
     956                        kind = IndexedPropertyDoubleOutOfBoundsSaneChainLoc;
     957                    else
     958                        kind = IndexedPropertyDoubleLoc;
     959                } else {
    955960                    ASSERT(mode.isOutOfBoundsSaneChain());
    956                     kind = IndexedPropertyDoubleOrOtherSaneChainLoc;
     961                    kind = IndexedPropertyDoubleOrOtherOutOfBoundsSaneChainLoc;
    957962                }
    958963                def(HeapLocation(kind, IndexedDoubleProperties, graph.varArgChild(node, 0), graph.varArgChild(node, 1)), LazyNode(node));
    … …  
    966971                read(Butterfly_publicLength);
    967972                read(IndexedContiguousProperties);
    968                 def(HeapLocation(indexedPropertyLoc, IndexedContiguousProperties, graph.varArgChild(node, 0), graph.varArgChild(node, 1)), LazyNode(node));
     973                def(HeapLocation(mode.isOutOfBoundsSaneChain() ? IndexedPropertyJSOutOfBoundsSaneChainLoc : indexedPropertyLoc, IndexedContiguousProperties, graph.varArgChild(node, 0), graph.varArgChild(node, 1)), LazyNode(node));
    969974                return;
    970975            }
    … …  
    10551060                write(Butterfly_publicLength);
    10561061            def(HeapLocation(indexedPropertyLoc, IndexedInt32Properties, base, index), LazyNode(value));
    1057             def(HeapLocation(IndexedPropertyInt32OrOtherLoc, IndexedInt32Properties, base, index), LazyNode(value));
     1062            def(HeapLocation(IndexedPropertyInt32OutOfBoundsSaneChainLoc, IndexedInt32Properties, base, index), LazyNode(value));
    10581063            return;
    10591064           
    … …  
    10711076            def(HeapLocation(IndexedPropertyDoubleLoc, IndexedDoubleProperties, base, index), LazyNode(value));
    10721077            def(HeapLocation(IndexedPropertyDoubleSaneChainLoc, IndexedDoubleProperties, base, index), LazyNode(value));
     1078            def(HeapLocation(IndexedPropertyDoubleOutOfBoundsSaneChainLoc, IndexedDoubleProperties, base, index), LazyNode(value));
    10731079            return;
    10741080           
    … …  
    10851091                write(Butterfly_publicLength);
    10861092            def(HeapLocation(indexedPropertyLoc, IndexedContiguousProperties, base, index), LazyNode(value));
     1093            def(HeapLocation(IndexedPropertyJSOutOfBoundsSaneChainLoc, IndexedContiguousProperties, base, index), LazyNode(value));
    10871094            return;
    10881095           

  • trunk/Source/JavaScriptCore/dfg/DFGHeapLocation.cpp

    r265907 r266813  
    143143        return;
    144144
    145     case IndexedPropertyDoubleOrOtherSaneChainLoc:
    146         out.print("IndexedPropertyDoubleOrOtherSaneChainLoc");
     145    case IndexedPropertyDoubleOutOfBoundsSaneChainLoc:
     146        out.print("IndexedPropertyDoubleOutOfBoundsSaneChainLoc");
     147        return;
     148
     149    case IndexedPropertyDoubleOrOtherOutOfBoundsSaneChainLoc:
     150        out.print("IndexedPropertyDoubleOrOtherOutOfBoundsSaneChainLoc");
    147151        return;
    148152
    … …  
    151155        return;
    152156
    153     case IndexedPropertyInt32OrOtherLoc:
    154         out.print("IndexedPropertyInt32OrOtherLoc");
     157    case IndexedPropertyInt32OutOfBoundsSaneChainLoc:
     158        out.print("IndexedPropertyInt32OutOfBoundsSaneChainLoc");
    155159        return;
    156160
    … …  
    161165    case IndexedPropertyJSLoc:
    162166        out.print("IndexedPropertyJSLoc");
     167        return;
     168
     169    case IndexedPropertyJSOutOfBoundsSaneChainLoc:
     170        out.print("IndexedPropertyJSOutOfBoundsSaneChainLoc");
    163171        return;
    164172

  • trunk/Source/JavaScriptCore/dfg/DFGHeapLocation.h

    r265907 r266813  
    5050    IndexedPropertyDoubleLoc,
    5151    IndexedPropertyDoubleSaneChainLoc,
    52     IndexedPropertyDoubleOrOtherSaneChainLoc,
     52    IndexedPropertyDoubleOutOfBoundsSaneChainLoc,
     53    IndexedPropertyDoubleOrOtherOutOfBoundsSaneChainLoc,
    5354    IndexedPropertyInt32Loc,
    54     IndexedPropertyInt32OrOtherLoc,
     55    IndexedPropertyInt32OutOfBoundsSaneChainLoc,
    5556    IndexedPropertyInt52Loc,
     57    IndexedPropertyJSOutOfBoundsSaneChainLoc,
    5658    IndexedPropertyJSLoc,
    5759    IndexedPropertyStorageLoc,
Note: See TracChangeset for help on using the changeset viewer.