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

Changeset 267585 in webkit


Ignore:
Timestamp:
Sep 25, 2020, 11:21:03 AM (6 years ago)
Author:
Alan Coon
Message:

Cherry-pick r266813. rdar://problem/69582551

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:

git-svn-id: ​https://svn.webkit.org/repository/webkit/trunk@266813 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Location:
branches/safari-610-branch
Files:
1 added
6 edited

Legend:

Unmodified
Added
Removed

  • branches/safari-610-branch/JSTests/ChangeLog

    r267285 r267585  
     12020-09-25  Alan Coon  <alancoon@apple.com>
     2
     3        Cherry-pick r266813. rdar://problem/69582551
     4
     5    OutOfBoundsSaneChain operations should use their own heap locations
     6    https://bugs.webkit.org/show_bug.cgi?id=216328
     7    <rdar://problem/68568039>
     8   
     9    Reviewed by Keith Miller.
     10   
     11    JSTests:
     12   
     13    * stress/out-of-bounds-sane-chain-need-their-own-heap-location.js: Added.
     14    (foo):
     15   
     16    Source/JavaScriptCore:
     17   
     18    There is code in local CSE that does some basic bounds check elimination
     19    for PutByVal. It does this analysis by seeing if a particular heap location
     20    is already defined, and if so, it eliminates the bounds check for the
     21    PutByVal. This doesn't work for OutOfBoundsSaneChain for the obvious reason
     22    that these GetByVals are not proven to be in bounds. So GetByVal's in the
     23    OutOfBoundsSaneChain mode reusing non OutOfBoundsSaneChain heap locations
     24    can lead to a bug where we mistakenly remove a bounds check. The fix is to
     25    have all OutOfBoundsSaneChain operations use distinct heaps, and for CSE to
     26    not query those heaps.
     27   
     28    * dfg/DFGArrayMode.h:
     29    (JSC::DFG::ArrayMode::isAnySaneChain const): Deleted.
     30    * dfg/DFGClobberize.h:
     31    (JSC::DFG::clobberize):
     32    * dfg/DFGHeapLocation.cpp:
     33    (WTF::printInternal):
     34    * dfg/DFGHeapLocation.h:
     35   
     36   
     37    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@266813 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     38
     39    2020-09-09  Saam Barati  <sbarati@apple.com>
     40
     41            OutOfBoundsSaneChain operations should use their own heap locations
     42            https://bugs.webkit.org/show_bug.cgi?id=216328
     43            <rdar://problem/68568039>
     44
     45            Reviewed by Keith Miller.
     46
     47            * stress/out-of-bounds-sane-chain-need-their-own-heap-location.js: Added.
     48            (foo):
     49
    1502020-09-18  Alan Coon  <alancoon@apple.com>
    251

  • branches/safari-610-branch/Source/JavaScriptCore/ChangeLog

    r267285 r267585  
     12020-09-25  Alan Coon  <alancoon@apple.com>
     2
     3        Cherry-pick r266813. rdar://problem/69582551
     4
     5    OutOfBoundsSaneChain operations should use their own heap locations
     6    https://bugs.webkit.org/show_bug.cgi?id=216328
     7    <rdar://problem/68568039>
     8   
     9    Reviewed by Keith Miller.
     10   
     11    JSTests:
     12   
     13    * stress/out-of-bounds-sane-chain-need-their-own-heap-location.js: Added.
     14    (foo):
     15   
     16    Source/JavaScriptCore:
     17   
     18    There is code in local CSE that does some basic bounds check elimination
     19    for PutByVal. It does this analysis by seeing if a particular heap location
     20    is already defined, and if so, it eliminates the bounds check for the
     21    PutByVal. This doesn't work for OutOfBoundsSaneChain for the obvious reason
     22    that these GetByVals are not proven to be in bounds. So GetByVal's in the
     23    OutOfBoundsSaneChain mode reusing non OutOfBoundsSaneChain heap locations
     24    can lead to a bug where we mistakenly remove a bounds check. The fix is to
     25    have all OutOfBoundsSaneChain operations use distinct heaps, and for CSE to
     26    not query those heaps.
     27   
     28    * dfg/DFGArrayMode.h:
     29    (JSC::DFG::ArrayMode::isAnySaneChain const): Deleted.
     30    * dfg/DFGClobberize.h:
     31    (JSC::DFG::clobberize):
     32    * dfg/DFGHeapLocation.cpp:
     33    (WTF::printInternal):
     34    * dfg/DFGHeapLocation.h:
     35   
     36   
     37    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@266813 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     38
     39    2020-09-09  Saam Barati  <sbarati@apple.com>
     40
     41            OutOfBoundsSaneChain operations should use their own heap locations
     42            https://bugs.webkit.org/show_bug.cgi?id=216328
     43            <rdar://problem/68568039>
     44
     45            Reviewed by Keith Miller.
     46
     47            There is code in local CSE that does some basic bounds check elimination
     48            for PutByVal. It does this analysis by seeing if a particular heap location
     49            is already defined, and if so, it eliminates the bounds check for the
     50            PutByVal. This doesn't work for OutOfBoundsSaneChain for the obvious reason
     51            that these GetByVals are not proven to be in bounds. So GetByVal's in the
     52            OutOfBoundsSaneChain mode reusing non OutOfBoundsSaneChain heap locations
     53            can lead to a bug where we mistakenly remove a bounds check. The fix is to
     54            have all OutOfBoundsSaneChain operations use distinct heaps, and for CSE to
     55            not query those heaps.
     56
     57            * dfg/DFGArrayMode.h:
     58            (JSC::DFG::ArrayMode::isAnySaneChain const): Deleted.
     59            * dfg/DFGClobberize.h:
     60            (JSC::DFG::clobberize):
     61            * dfg/DFGHeapLocation.cpp:
     62            (WTF::printInternal):
     63            * dfg/DFGHeapLocation.h:
     64
    1652020-09-18  Alan Coon  <alancoon@apple.com>
    266

  • branches/safari-610-branch/Source/JavaScriptCore/dfg/DFGArrayMode.h

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

  • branches/safari-610-branch/Source/JavaScriptCore/dfg/DFGClobberize.h

    r265934 r267585  
    939939                read(Butterfly_publicLength);
    940940                read(IndexedInt32Properties);
    941                 LocationKind kind = mode.isOutOfBoundsSaneChain() ? IndexedPropertyInt32OrOtherLoc : indexedPropertyLoc;
     941                LocationKind kind = mode.isOutOfBoundsSaneChain() ? IndexedPropertyInt32OutOfBoundsSaneChainLoc : indexedPropertyLoc;
    942942                def(HeapLocation(kind, IndexedInt32Properties, graph.varArgChild(node, 0), graph.varArgChild(node, 1)), LazyNode(node));
    943943                return;
    … …  
    951951                read(IndexedDoubleProperties);
    952952                LocationKind kind;
    953                 if (node->hasDoubleResult())
    954                     kind = mode.isAnySaneChain() ? IndexedPropertyDoubleSaneChainLoc : IndexedPropertyDoubleLoc;
    955                 else {
     953                if (node->hasDoubleResult()) {
     954                    if (mode.isInBoundsSaneChain())
     955                        kind = IndexedPropertyDoubleSaneChainLoc;
     956                    else if (mode.isOutOfBoundsSaneChain())
     957                        kind = IndexedPropertyDoubleOutOfBoundsSaneChainLoc;
     958                    else
     959                        kind = IndexedPropertyDoubleLoc;
     960                } else {
    956961                    ASSERT(mode.isOutOfBoundsSaneChain());
    957                     kind = IndexedPropertyDoubleOrOtherSaneChainLoc;
     962                    kind = IndexedPropertyDoubleOrOtherOutOfBoundsSaneChainLoc;
    958963                }
    959964                def(HeapLocation(kind, IndexedDoubleProperties, graph.varArgChild(node, 0), graph.varArgChild(node, 1)), LazyNode(node));
    … …  
    967972                read(Butterfly_publicLength);
    968973                read(IndexedContiguousProperties);
    969                 def(HeapLocation(indexedPropertyLoc, IndexedContiguousProperties, graph.varArgChild(node, 0), graph.varArgChild(node, 1)), LazyNode(node));
     974                def(HeapLocation(mode.isOutOfBoundsSaneChain() ? IndexedPropertyJSOutOfBoundsSaneChainLoc : indexedPropertyLoc, IndexedContiguousProperties, graph.varArgChild(node, 0), graph.varArgChild(node, 1)), LazyNode(node));
    970975                return;
    971976            }
    … …  
    10561061                write(Butterfly_publicLength);
    10571062            def(HeapLocation(indexedPropertyLoc, IndexedInt32Properties, base, index), LazyNode(value));
    1058             def(HeapLocation(IndexedPropertyInt32OrOtherLoc, IndexedInt32Properties, base, index), LazyNode(value));
     1063            def(HeapLocation(IndexedPropertyInt32OutOfBoundsSaneChainLoc, IndexedInt32Properties, base, index), LazyNode(value));
    10591064            return;
    10601065           
    … …  
    10721077            def(HeapLocation(IndexedPropertyDoubleLoc, IndexedDoubleProperties, base, index), LazyNode(value));
    10731078            def(HeapLocation(IndexedPropertyDoubleSaneChainLoc, IndexedDoubleProperties, base, index), LazyNode(value));
     1079            def(HeapLocation(IndexedPropertyDoubleOutOfBoundsSaneChainLoc, IndexedDoubleProperties, base, index), LazyNode(value));
    10741080            return;
    10751081           
    … …  
    10861092                write(Butterfly_publicLength);
    10871093            def(HeapLocation(indexedPropertyLoc, IndexedContiguousProperties, base, index), LazyNode(value));
     1094            def(HeapLocation(IndexedPropertyJSOutOfBoundsSaneChainLoc, IndexedContiguousProperties, base, index), LazyNode(value));
    10881095            return;
    10891096           

  • branches/safari-610-branch/Source/JavaScriptCore/dfg/DFGHeapLocation.cpp

    r265907 r267585  
    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

  • branches/safari-610-branch/Source/JavaScriptCore/dfg/DFGHeapLocation.h

    r265907 r267585  
    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.