Changeset 241849 in webkit


Ignore:
Timestamp:
Feb 20, 2019 4:10:43 PM (5 years ago)
Author:
mark.lam@apple.com
Message:

Add code to validate expected GC activity modelled by doesGC() against what the runtime encounters.
https://bugs.webkit.org/show_bug.cgi?id=193938
<rdar://problem/47616277>

Reviewed by Michael Saboff, Saam Barati, and Robin Morisset.

In DFG::SpeculativeJIT::compile() and FTL::LowerDFGToB3::compileNode(), before
emitting code / B3IR for each DFG node, we emit a write to set Heap::m_expectDoesGC
to the value returned by doesGC() for that node. In the runtime (i.e. in allocateCell()
and functions that can resolve a rope), we assert that Heap::m_expectDoesGC is
true.

This validation code is currently only enabled for debug builds. It is disabled
for release builds by default, but it can easily be made to run on release builds
as well by forcing ENABLE_DFG_DOES_GC_VALIDATION to 1 in Heap.h.

To allow this validation code to run on release builds as well, the validation uses
RELEASE_ASSERT instead of ASSERT.

To ensure that Heap.h is #include'd for all files that needs to do this validation
(so that the validation code is accidentally disabled), we guard the validation
code with an if conditional on constexpr bool validateDFGDoesGC (instead of using
a #if ENABLE(DFG_DOES_GC_VALIDATION)). This way, if Heap.h isn't #include'd, the
validation code will fail to build (no silent failures).

Currently, all JSC tests and Layout tests should pass with this validation enabled
in debug builds. We'll only see new failures if there's a regression or if new
tests reveal a previously untested code path that has an undetected issue.

  • dfg/DFGOSRExit.cpp:

(JSC::DFG::OSRExit::executeOSRExit):
(JSC::DFG::OSRExit::compileExit):

  • dfg/DFGSpeculativeJIT64.cpp:

(JSC::DFG::SpeculativeJIT::compile):

  • ftl/FTLLowerDFGToB3.cpp:

(JSC::FTL::DFG::LowerDFGToB3::compileNode):

  • ftl/FTLOSRExitCompiler.cpp:

(JSC::FTL::compileStub):

  • heap/Heap.h:

(JSC::Heap::expectDoesGC const):
(JSC::Heap::setExpectDoesGC):
(JSC::Heap::addressOfExpectDoesGC):

  • jit/JITArithmetic.cpp:

(JSC::JIT::emit_compareAndJump):

  • runtime/JSCellInlines.h:

(JSC::tryAllocateCellHelper):

  • runtime/JSString.h:

(JSC::jsSingleCharacterString):
(JSC::JSString::toAtomicString const):
(JSC::JSString::toExistingAtomicString const):
(JSC::JSString::value const):
(JSC::JSString::tryGetValue const):
(JSC::JSRopeString::unsafeView const):
(JSC::JSRopeString::viewWithUnderlyingString const):
(JSC::JSString::unsafeView const):

Location:
trunk/Source/JavaScriptCore
Files:
9 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/JavaScriptCore/ChangeLog

    r241841 r241849  
     12019-02-20  Mark Lam  <mark.lam@apple.com>
     2
     3        Add code to validate expected GC activity modelled by doesGC() against what the runtime encounters.
     4        https://bugs.webkit.org/show_bug.cgi?id=193938
     5        <rdar://problem/47616277>
     6
     7        Reviewed by Michael Saboff, Saam Barati, and Robin Morisset.
     8
     9        In DFG::SpeculativeJIT::compile() and FTL::LowerDFGToB3::compileNode(), before
     10        emitting code / B3IR for each DFG node, we emit a write to set Heap::m_expectDoesGC
     11        to the value returned by doesGC() for that node.  In the runtime (i.e. in allocateCell()
     12        and functions that can resolve a rope), we assert that Heap::m_expectDoesGC is
     13        true.
     14
     15        This validation code is currently only enabled for debug builds.  It is disabled
     16        for release builds by default, but it can easily be made to run on release builds
     17        as well by forcing ENABLE_DFG_DOES_GC_VALIDATION to 1 in Heap.h.
     18
     19        To allow this validation code to run on release builds as well, the validation uses
     20        RELEASE_ASSERT instead of ASSERT.
     21
     22        To ensure that Heap.h is #include'd for all files that needs to do this validation
     23        (so that the validation code is accidentally disabled), we guard the validation
     24        code with an if conditional on constexpr bool validateDFGDoesGC (instead of using
     25        a #if ENABLE(DFG_DOES_GC_VALIDATION)).  This way, if Heap.h isn't #include'd, the
     26        validation code will fail to build (no silent failures).
     27
     28        Currently, all JSC tests and Layout tests should pass with this validation enabled
     29        in debug builds.  We'll only see new failures if there's a regression or if new
     30        tests reveal a previously untested code path that has an undetected issue.
     31
     32        * dfg/DFGOSRExit.cpp:
     33        (JSC::DFG::OSRExit::executeOSRExit):
     34        (JSC::DFG::OSRExit::compileExit):
     35        * dfg/DFGSpeculativeJIT64.cpp:
     36        (JSC::DFG::SpeculativeJIT::compile):
     37        * ftl/FTLLowerDFGToB3.cpp:
     38        (JSC::FTL::DFG::LowerDFGToB3::compileNode):
     39        * ftl/FTLOSRExitCompiler.cpp:
     40        (JSC::FTL::compileStub):
     41        * heap/Heap.h:
     42        (JSC::Heap::expectDoesGC const):
     43        (JSC::Heap::setExpectDoesGC):
     44        (JSC::Heap::addressOfExpectDoesGC):
     45        * jit/JITArithmetic.cpp:
     46        (JSC::JIT::emit_compareAndJump):
     47        * runtime/JSCellInlines.h:
     48        (JSC::tryAllocateCellHelper):
     49        * runtime/JSString.h:
     50        (JSC::jsSingleCharacterString):
     51        (JSC::JSString::toAtomicString const):
     52        (JSC::JSString::toExistingAtomicString const):
     53        (JSC::JSString::value const):
     54        (JSC::JSString::tryGetValue const):
     55        (JSC::JSRopeString::unsafeView const):
     56        (JSC::JSRopeString::viewWithUnderlyingString const):
     57        (JSC::JSString::unsafeView const):
     58
    1592019-02-20  Andy Estes  <aestes@apple.com>
    260
  • trunk/Source/JavaScriptCore/dfg/DFGOSRExit.cpp

    r241222 r241849  
    11/*
    2  * Copyright (C) 2011-2018 Apple Inc. All rights reserved.
     2 * Copyright (C) 2011-2019 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
     
    338338    ASSERT(&exec->vm() == &vm);
    339339    auto& cpu = context.cpu;
     340
     341    if (validateDFGDoesGC) {
     342        // We're about to exit optimized code. So, there's no longer any optimized
     343        // code running that expects no GC.
     344        vm.heap.setExpectDoesGC(true);
     345    }
    340346
    341347    if (vm.callFrameForCatch) {
     
    13901396    }
    13911397
     1398    if (validateDFGDoesGC) {
     1399        // We're about to exit optimized code. So, there's no longer any optimized
     1400        // code running that expects no GC. We need to set this before arguments
     1401        // materialization below (see emitRestoreArguments()).
     1402        jit.store8(CCallHelpers::TrustedImm32(true), vm.heap.addressOfExpectDoesGC());
     1403    }
     1404
    13921405    // Need to ensure that the stack pointer accounts for the worst-case stack usage at exit. This
    13931406    // could toast some stack that the DFG used. We need to do it before storing to stack offsets
  • trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp

    r241210 r241849  
    11/*
    2  * Copyright (C) 2011-2018 Apple Inc. All rights reserved.
     2 * Copyright (C) 2011-2019 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
     
    3434#include "DFGAbstractInterpreterInlines.h"
    3535#include "DFGCallArrayAllocatorSlowPathGenerator.h"
     36#include "DFGDoesGC.h"
    3637#include "DFGOperations.h"
    3738#include "DFGSlowPathGenerator.h"
     
    19001901{
    19011902    NodeType op = node->op();
    1902    
     1903
     1904    if (validateDFGDoesGC) {
     1905        bool expectDoesGC = doesGC(m_jit.graph(), node);
     1906        m_jit.store8(TrustedImm32(expectDoesGC), m_jit.vm()->heap.addressOfExpectDoesGC());
     1907    }
     1908
    19031909#if ENABLE(DFG_REGISTER_ALLOCATION_VALIDATION)
    19041910    m_jit.clearRegisterAllocationOffsets();
  • trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp

    r241228 r241849  
    11/*
    2  * Copyright (C) 2013-2018 Apple Inc. All rights reserved.
     2 * Copyright (C) 2013-2019 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
     
    4444#include "DFGAbstractInterpreterInlines.h"
    4545#include "DFGCapabilities.h"
     46#include "DFGDoesGC.h"
    4647#include "DFGDominators.h"
    4748#include "DFGInPlaceAbstractState.h"
     
    526527        m_interpreter.startExecuting();
    527528        m_interpreter.executeKnownEdgeTypes(m_node);
    528        
     529
     530        if (validateDFGDoesGC) {
     531            bool expectDoesGC = doesGC(m_graph, m_node);
     532            m_out.store(m_out.constBool(expectDoesGC), m_out.absolute(vm().heap.addressOfExpectDoesGC()));
     533        }
     534
    529535        switch (m_node->op()) {
    530536        case DFG::Upsilon:
  • trunk/Source/JavaScriptCore/ftl/FTLOSRExitCompiler.cpp

    r241222 r241849  
    11/*
    2  * Copyright (C) 2013-2018 Apple Inc. All rights reserved.
     2 * Copyright (C) 2013-2019 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
     
    245245    saveAllRegisters(jit, registerScratch);
    246246   
     247    if (validateDFGDoesGC) {
     248        // We're about to exit optimized code. So, there's no longer any optimized
     249        // code running that expects no GC. We need to set this before object
     250        // materialization below.
     251        jit.store8(CCallHelpers::TrustedImm32(true), vm->heap.addressOfExpectDoesGC());
     252    }
     253
    247254    // Bring the stack back into a sane form and assert that it's sane.
    248255    jit.popToRestore(GPRInfo::regT0);
  • trunk/Source/JavaScriptCore/heap/Heap.h

    r240216 r241849  
    22 *  Copyright (C) 1999-2000 Harri Porten (porten@kde.org)
    33 *  Copyright (C) 2001 Peter Kelly (pmk@post.com)
    4  *  Copyright (C) 2003-2017 Apple Inc. All rights reserved.
     4 *  Copyright (C) 2003-2019 Apple Inc. All rights reserved.
    55 *
    66 *  This library is free software; you can redistribute it and/or
     
    9696}
    9797
     98#if !ASSERT_DISABLED
     99#define ENABLE_DFG_DOES_GC_VALIDATION 1
     100#else
     101#define ENABLE_DFG_DOES_GC_VALIDATION 0
     102#endif
     103constexpr bool validateDFGDoesGC = ENABLE_DFG_DOES_GC_VALIDATION;
     104
    98105typedef HashCountedSet<JSCell*> ProtectCountSet;
    99106typedef HashCountedSet<const char*> TypeCountSet;
     
    294301    unsigned barrierThreshold() const { return m_barrierThreshold; }
    295302    const unsigned* addressOfBarrierThreshold() const { return &m_barrierThreshold; }
     303
     304#if ENABLE(DFG_DOES_GC_VALIDATION)
     305    bool expectDoesGC() const { return m_expectDoesGC; }
     306    void setExpectDoesGC(bool value) { m_expectDoesGC = value; }
     307    bool* addressOfExpectDoesGC() { return &m_expectDoesGC; }
     308#else
     309    bool expectDoesGC() const { UNREACHABLE_FOR_PLATFORM(); return true; }
     310    void setExpectDoesGC(bool) { UNREACHABLE_FOR_PLATFORM(); }
     311    bool* addressOfExpectDoesGC() { UNREACHABLE_FOR_PLATFORM(); return nullptr; }
     312#endif
    296313
    297314    // If true, the GC believes that the mutator is currently messing with the heap. We call this
     
    582599    Markable<CollectionScope, EnumMarkableTraits<CollectionScope>> m_lastCollectionScope;
    583600    Lock m_raceMarkStackLock;
     601#if ENABLE(DFG_DOES_GC_VALIDATION)
     602    bool m_expectDoesGC { true };
     603#endif
    584604
    585605    StructureIDTable m_structureIDTable;
  • trunk/Source/JavaScriptCore/jit/JITArithmetic.cpp

    r240138 r241849  
    180180    int op2 = bytecode.m_rhs.offset();
    181181    unsigned target = jumpTarget(instruction, bytecode.m_targetLabel);
     182    bool disallowAllocation = false;
    182183    if (isOperandConstantChar(op1)) {
    183184        emitGetVirtualRegister(op2, regT0);
     
    186187        emitLoadCharacterString(regT0, regT0, failures);
    187188        addSlowCase(failures);
    188         addJump(branch32(commute(condition), regT0, Imm32(asString(getConstantOperand(op1))->tryGetValue()[0])), target);
     189        addJump(branch32(commute(condition), regT0, Imm32(asString(getConstantOperand(op1))->tryGetValue(disallowAllocation)[0])), target);
    189190        return;
    190191    }
     
    195196        emitLoadCharacterString(regT0, regT0, failures);
    196197        addSlowCase(failures);
    197         addJump(branch32(condition, regT0, Imm32(asString(getConstantOperand(op2))->tryGetValue()[0])), target);
     198        addJump(branch32(condition, regT0, Imm32(asString(getConstantOperand(op2))->tryGetValue(disallowAllocation)[0])), target);
    198199        return;
    199200    }
  • trunk/Source/JavaScriptCore/runtime/JSCellInlines.h

    r240965 r241849  
    11/*
    2  * Copyright (C) 2012-2018 Apple Inc. All rights reserved.
     2 * Copyright (C) 2012-2019 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
     
    167167{
    168168    VM& vm = *heap.vm();
     169    if (validateDFGDoesGC)
     170        RELEASE_ASSERT(heap.expectDoesGC());
     171
    169172    ASSERT(deferralContext || !DisallowGC::isInEffectOnCurrentThread());
    170173    ASSERT(size >= sizeof(T));
  • trunk/Source/JavaScriptCore/runtime/JSString.h

    r241493 r241849  
    22 *  Copyright (C) 1999-2001 Harri Porten (porten@kde.org)
    33 *  Copyright (C) 2001 Peter Kelly (pmk@post.com)
    4  *  Copyright (C) 2003-2018 Apple Inc. All rights reserved.
     4 *  Copyright (C) 2003-2019 Apple Inc. All rights reserved.
    55 *
    66 *  This library is free software; you can redistribute it and/or
     
    164164    inline bool equal(ExecState*, JSString* other) const;
    165165    const String& value(ExecState*) const;
    166     inline const String& tryGetValue() const;
     166    inline const String& tryGetValue(bool allocationAllowed = true) const;
    167167    const StringImpl* tryGetValueImpl() const;
    168168    ALWAYS_INLINE unsigned length() const { return m_length; }
     
    516516ALWAYS_INLINE JSString* jsSingleCharacterString(VM* vm, UChar c)
    517517{
     518    if (validateDFGDoesGC)
     519        RELEASE_ASSERT(vm->heap.expectDoesGC());
    518520    if (c <= maxSingleCharacterString)
    519521        return vm->smallStrings.singleCharacterString(c);
     
    540542ALWAYS_INLINE AtomicString JSString::toAtomicString(ExecState* exec) const
    541543{
     544    if (validateDFGDoesGC)
     545        RELEASE_ASSERT(vm()->heap.expectDoesGC());
    542546    if (isRope())
    543547        static_cast<const JSRopeString*>(this)->resolveRopeToAtomicString(exec);
     
    547551ALWAYS_INLINE RefPtr<AtomicStringImpl> JSString::toExistingAtomicString(ExecState* exec) const
    548552{
     553    if (validateDFGDoesGC)
     554        RELEASE_ASSERT(vm()->heap.expectDoesGC());
    549555    if (isRope())
    550556        return static_cast<const JSRopeString*>(this)->resolveRopeToExistingAtomicString(exec);
     
    556562inline const String& JSString::value(ExecState* exec) const
    557563{
     564    if (validateDFGDoesGC)
     565        RELEASE_ASSERT(vm()->heap.expectDoesGC());
    558566    if (isRope())
    559567        static_cast<const JSRopeString*>(this)->resolveRope(exec);
     
    561569}
    562570
    563 inline const String& JSString::tryGetValue() const
    564 {
    565     if (isRope()) {
    566         // Pass nullptr for the ExecState so that resolveRope does not throw in the event of an OOM error.
    567         static_cast<const JSRopeString*>(this)->resolveRope(nullptr);
    568     }
     571inline const String& JSString::tryGetValue(bool allocationAllowed) const
     572{
     573    if (allocationAllowed) {
     574        if (validateDFGDoesGC)
     575            RELEASE_ASSERT(vm()->heap.expectDoesGC());
     576        if (isRope()) {
     577            // Pass nullptr for the ExecState so that resolveRope does not throw in the event of an OOM error.
     578            static_cast<const JSRopeString*>(this)->resolveRope(nullptr);
     579        }
     580    } else
     581        RELEASE_ASSERT(!isRope());
    569582    return m_value;
    570583}
     
    740753ALWAYS_INLINE StringView JSRopeString::unsafeView(ExecState* exec) const
    741754{
     755    if (validateDFGDoesGC)
     756        RELEASE_ASSERT(vm()->heap.expectDoesGC());
    742757    if (isSubstring()) {
    743758        if (is8Bit())
     
    751766ALWAYS_INLINE StringViewWithUnderlyingString JSRopeString::viewWithUnderlyingString(ExecState* exec) const
    752767{
     768    if (validateDFGDoesGC)
     769        RELEASE_ASSERT(vm()->heap.expectDoesGC());
    753770    if (isSubstring()) {
    754771        auto& base = substringBase()->m_value;
     
    763780ALWAYS_INLINE StringView JSString::unsafeView(ExecState* exec) const
    764781{
     782    if (validateDFGDoesGC)
     783        RELEASE_ASSERT(vm()->heap.expectDoesGC());
    765784    if (isRope())
    766785        return static_cast<const JSRopeString*>(this)->unsafeView(exec);
Note: See TracChangeset for help on using the changeset viewer.