Changeset 264643 in webkit


Ignore:
Timestamp:
Jul 20, 2020 10:16:51 PM (4 years ago)
Author:
mark.lam@apple.com
Message:

TryGetById clobberize rules are wrong.
​https://bugs.webkit.org/show_bug.cgi?id=163834
<rdar://problem/65625807>

Reviewed by Keith Miller.

Theoretically, TryGetById can do the same things GetById does i.e. reify lazy
properties, read the stack, etc. Hence, its clobberize rule should be clobberTop
just like GetById. However, in practice, we don't currently use @tryGetById to
access anything on the stack (and probably never will). But as a conservative
measure, we'll just treat TryGetById like it can. In clobberize terms, this
means we declare TryGetById as doing read(World) (just like GetById) instead of
read(Heap).

  • dfg/DFGAbstractInterpreterInlines.h:

(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):

  • dfg/DFGClobberize.h:

(JSC::DFG::clobberize):

  • dfg/DFGClobbersExitState.cpp:

(JSC::DFG::clobbersExitState):

Location:
trunk/Source/JavaScriptCore
Files:
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r264640 r264643  
     12020-07-20  Mark Lam  <mark.lam@apple.com>
     2
     3        TryGetById clobberize rules are wrong.
     4        https://bugs.webkit.org/show_bug.cgi?id=163834
     5        <rdar://problem/65625807>
     6
     7        Reviewed by Keith Miller.
     8
     9        Theoretically, TryGetById can do the same things GetById does i.e. reify lazy
     10        properties, read the stack, etc.  Hence, its clobberize rule should be clobberTop
     11        just like GetById.  However, in practice, we don't currently use @tryGetById to
     12        access anything on the stack (and probably never will).  But as a conservative
     13        measure, we'll just treat TryGetById like it can.  In clobberize terms, this
     14        means we declare TryGetById as doing read(World) (just like GetById) instead of
     15        read(Heap).
     16
     17        * dfg/DFGAbstractInterpreterInlines.h:
     18        (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
     19        * dfg/DFGClobberize.h:
     20        (JSC::DFG::clobberize):
     21        * dfg/DFGClobbersExitState.cpp:
     22        (JSC::DFG::clobbersExitState):
     23
    1242020-07-20  Yusuke Suzuki  <ysuzuki@apple.com>
    225

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

    r264507 r264643  
    33353335        // FIXME: This should constant fold at least as well as the normal GetById case.
    33363336        // https://bugs.webkit.org/show_bug.cgi?id=156422
     3337        clobberWorld();
    33373338        makeHeapTopForNode(node);
    33383339        break;

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

    r264504 r264643  
    656656    case GetByIdDirectFlush:
    657657    case GetByValWithThis:
     658    case TryGetById:
    658659    case PutById:
    659660    case PutByIdWithThis:
    … …  
    13051306    }
    13061307
    1307     case TryGetById: {
    1308         read(Heap);
    1309         return;
    1310     }
    1311 
    13121308    case MultiGetByOffset: {
    13131309        read(JSCell_structureID);

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

    r261755 r264643  
    8686    case FilterInByIdStatus:
    8787    case FilterDeleteByStatus:
     88    case TryGetById:
    8889        // These do clobber memory, but nothing that is observable. It may be nice to separate the
    8990        // heaps into those that are observable and those that aren't, but we don't do that right now.
Note: See TracChangeset for help on using the changeset viewer.