Changeset 241140 in webkit


Ignore:
Timestamp:
Feb 7, 2019 12:20:23 PM (5 years ago)
Author:
mark.lam@apple.com
Message:

Fix more doesGC() for CheckTraps, GetMapBucket, and Switch nodes.
https://bugs.webkit.org/show_bug.cgi?id=194399
<rdar://problem/47889777>

Reviewed by Yusuke Suzuki.

Fix doesGC() for the following nodes:

CheckTraps:

We normally will not emit this node because Options::usePollingTraps() is
false by default. However, as it is implemented now, CheckTraps can GC
because it can allocate a TerminatedExecutionException. If we make the
TerminatedExecutionException a singleton allocated at initialization time,
doesGC() can return false for CheckTraps.
https://bugs.webkit.org/show_bug.cgi?id=194323

GetMapBucket:

Can call operationJSMapFindBucket() or operationJSSetFindBucket(),
which calls HashMapImpl::findBucket(), which calls jsMapHash(), which
can resolve a rope.

Switch:

If switchData kind is SwitchChar, can call operationResolveRope() .
If switchData kind is SwitchString and the child use kind is not StringIdentUse,

can call operationSwitchString() which resolves ropes.

DirectTailCall:
ForceOSRExit:
Return:
TailCallForwardVarargs:
TailCallVarargs:
Throw:

These are terminal nodes. It shouldn't really matter what doesGC() returns
for them, but following our conservative practice, unless we have a good
reason for doesGC() to return false, we should just return true.

  • dfg/DFGDoesGC.cpp:

(JSC::DFG::doesGC):

Location:
trunk/Source/JavaScriptCore
Files:
2 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/JavaScriptCore/ChangeLog

    r241126 r241140  
     12019-02-07  Mark Lam  <mark.lam@apple.com>
     2
     3        Fix more doesGC() for CheckTraps, GetMapBucket, and Switch nodes.
     4        https://bugs.webkit.org/show_bug.cgi?id=194399
     5        <rdar://problem/47889777>
     6
     7        Reviewed by Yusuke Suzuki.
     8
     9        Fix doesGC() for the following nodes:
     10
     11            CheckTraps:
     12                We normally will not emit this node because Options::usePollingTraps() is
     13                false by default.  However, as it is implemented now, CheckTraps can GC
     14                because it can allocate a TerminatedExecutionException.  If we make the
     15                TerminatedExecutionException a singleton allocated at initialization time,
     16                doesGC() can return false for CheckTraps.
     17                https://bugs.webkit.org/show_bug.cgi?id=194323
     18
     19            GetMapBucket:
     20                Can call operationJSMapFindBucket() or operationJSSetFindBucket(),
     21                which calls HashMapImpl::findBucket(), which calls jsMapHash(), which
     22                can resolve a rope.
     23
     24            Switch:
     25                If switchData kind is SwitchChar, can call operationResolveRope() .
     26                If switchData kind is SwitchString and the child use kind is not StringIdentUse,
     27                    can call operationSwitchString() which resolves ropes.
     28
     29            DirectTailCall:
     30            ForceOSRExit:
     31            Return:
     32            TailCallForwardVarargs:
     33            TailCallVarargs:
     34            Throw:
     35                These are terminal nodes.  It shouldn't really matter what doesGC() returns
     36                for them, but following our conservative practice, unless we have a good
     37                reason for doesGC() to return false, we should just return true.
     38
     39        * dfg/DFGDoesGC.cpp:
     40        (JSC::DFG::doesGC):
     41
    1422019-02-07  Robin Morisset  <rmorisset@apple.com>
    243
  • trunk/Source/JavaScriptCore/dfg/DFGDoesGC.cpp

    r240998 r241140  
    4242   
    4343    // Now consider nodes that don't clobber the world but that still may GC. This includes all
    44     // nodes. By convention we put world-clobbering nodes in the block of "false" cases but we can
    45     // put them anywhere.
     44    // nodes. By default, we should assume every node can GC and return true. This includes the
     45    // world-clobbering nodes. We should only return false if we have proven that the node cannot
     46    // GC. Typical examples of how a node can GC is if the code emitted for the node does any of the
     47    // following:
     48    //     1. Allocates any objects.
     49    //     2. Resolves a rope string, which allocates a string.
     50    //     3. Produces a string (which allocates the string) except when we can prove that
     51    //        the string will always be one of the pre-allcoated SmallStrings.
     52    //     4. Triggers a structure transition (which can allocate a new structure)
     53    //        unless it is a known transition between previously allocated structures
     54    //        such as between Array types.
     55    //     5. Calls to a JS function, which can execute arbitrary code including allocating objects.
     56
    4657    switch (node->op()) {
    4758    case JSConstant:
     
    131142    case CompareStrictEq:
    132143    case CompareEqPtr:
    133     case TailCallForwardVarargs:
    134144    case ProfileType:
    135145    case ProfileControlFlow:
     
    150160    case Jump:
    151161    case Branch:
    152     case Switch:
    153162    case EntrySwitch:
    154     case Return:
    155     case DirectTailCall:
    156     case TailCallVarargs:
    157     case Throw:
    158163    case CountExecution:
    159164    case SuperSamplerBegin:
    160165    case SuperSamplerEnd:
    161     case ForceOSRExit:
    162166    case CPUIntrinsic:
    163     case CheckTraps:
    164167    case NormalizeMapKey:
    165     case GetMapBucket:
    166168    case GetMapBucketHead:
    167169    case GetMapBucketNext:
     
    258260        return false;
    259261
     262#if !ASSERT_DISABLED
    260263    case ArrayPush:
    261264    case ArrayPop:
     
    279282    case DirectCall:
    280283    case DirectConstruct:
     284    case DirectTailCall:
    281285    case DirectTailCallInlinedCaller:
     286    case ForceOSRExit:
    282287    case GetById:
    283288    case GetByIdDirect:
     
    288293    case GetDirectPname:
    289294    case GetDynamicVar:
     295    case GetMapBucket:
    290296    case HasGenericProperty:
    291297    case HasOwnProperty:
     
    319325    case ResolveScope:
    320326    case ResolveScopeForHoistingFuncDeclInEval:
     327    case Return:
    321328    case TailCall:
     329    case TailCallForwardVarargs:
    322330    case TailCallForwardVarargsInlinedCaller:
    323331    case TailCallInlinedCaller:
     332    case TailCallVarargs:
    324333    case TailCallVarargsInlinedCaller:
     334    case Throw:
    325335    case ToNumber:
    326336    case ToObject:
     
    379389    case ValueDiv:
    380390    case ValueNegate:
     391#else
     392    // See comment at the top for why be default for all nodes should be to
     393    // return true.
     394    default:
     395#endif
    381396        return true;
    382397
     
    390405            break;
    391406        }
     407        return true;
     408
     409    case CheckTraps:
     410        // FIXME: https://bugs.webkit.org/show_bug.cgi?id=194323
     411        ASSERT(Options::usePollingTraps());
    392412        return true;
    393413
     
    424444        return true;
    425445
     446    case Switch:
     447        switch (node->switchData()->kind) {
     448        case SwitchCell:
     449            ASSERT(graph.m_plan.isFTL());
     450            FALLTHROUGH;
     451        case SwitchImm:
     452            return false;
     453        case SwitchChar:
     454            return true;
     455        case SwitchString:
     456            if (node->child1().useKind() == StringIdentUse)
     457                return false;
     458            ASSERT(node->child1().useKind() == StringUse || node->child1().useKind() == UntypedUse);
     459            return true;
     460        }
     461
    426462    case LastNodeType:
    427463        RELEASE_ASSERT_NOT_REACHED();
Note: See TracChangeset for help on using the changeset viewer.