Changeset 94477 in webkit for trunk/Source/JavaScriptCore


Ignore:
Timestamp:
Sep 2, 2011, 10:14:04 PM (14 years ago)
Author:
[email protected]
Message:

ValueProfile does not make it safe to introspect cell values
after garbage collection
​https://bugs.webkit.org/show_bug.cgi?id=67354

Reviewed by Gavin Barraclough.

ValueProfile buckets are now weak references, implemented using a
light-weight weak reference mechanism that this patch also adds (the
WeakReferenceHarvester). If a cell stored in a ValueProfile bucket
is not marked, then the bucket is transformed into a Structure
pointer. If the Structure is not marked either, then it is turned
into a ClassInfo pointer.

(JSC::CodeBlock::~CodeBlock):
(JSC::CodeBlock::visitAggregate):
(JSC::CodeBlock::visitWeakReferences):

  • bytecode/CodeBlock.h:
  • bytecode/ValueProfile.h:

(JSC::ValueProfile::ValueProfile):
(JSC::ValueProfile::classInfo):
(JSC::ValueProfile::numberOfInt32s):
(JSC::ValueProfile::numberOfDoubles):
(JSC::ValueProfile::numberOfCells):
(JSC::ValueProfile::numberOfArrays):
(JSC::ValueProfile::probabilityOfArray):
(JSC::ValueProfile::WeakBucket::WeakBucket):
(JSC::ValueProfile::WeakBucket::operator!):
(JSC::ValueProfile::WeakBucket::isEmpty):
(JSC::ValueProfile::WeakBucket::isClassInfo):
(JSC::ValueProfile::WeakBucket::isStructure):
(JSC::ValueProfile::WeakBucket::asStructure):
(JSC::ValueProfile::WeakBucket::asClassInfo):
(JSC::ValueProfile::WeakBucket::getClassInfo):

  • heap/Heap.cpp:

(JSC::Heap::harvestWeakReferences):
(JSC::Heap::markRoots):

  • heap/Heap.h:
  • heap/MarkStack.cpp:

(JSC::SlotVisitor::drain):
(JSC::SlotVisitor::harvestWeakReferences):

  • heap/MarkStack.h:

(JSC::MarkStack::addWeakReferenceHarvester):
(JSC::MarkStack::MarkStack):
(JSC::MarkStack::appendUnbarrieredPointer):

  • heap/SlotVisitor.h:
  • heap/WeakReferenceHarvester.h: Added.

(JSC::WeakReferenceHarvester::WeakReferenceHarvester):
(JSC::WeakReferenceHarvester::~WeakReferenceHarvester):

Location:
trunk/Source/JavaScriptCore
Files:
1 added
10 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r94475 r94477  
     12011-09-02  Filip Pizlo  <[email protected]>
     2
     3        ValueProfile does not make it safe to introspect cell values
     4        after garbage collection
     5        https://bugs.webkit.org/show_bug.cgi?id=67354
     6
     7        Reviewed by Gavin Barraclough.
     8       
     9        ValueProfile buckets are now weak references, implemented using a
     10        light-weight weak reference mechanism that this patch also adds (the
     11        WeakReferenceHarvester).  If a cell stored in a ValueProfile bucket
     12        is not marked, then the bucket is transformed into a Structure
     13        pointer.  If the Structure is not marked either, then it is turned
     14        into a ClassInfo pointer.
     15
     16        * JavaScriptCore.xcodeproj/project.pbxproj:
     17        * bytecode/CodeBlock.cpp:
     18        (JSC::CodeBlock::~CodeBlock):
     19        (JSC::CodeBlock::visitAggregate):
     20        (JSC::CodeBlock::visitWeakReferences):
     21        * bytecode/CodeBlock.h:
     22        * bytecode/ValueProfile.h:
     23        (JSC::ValueProfile::ValueProfile):
     24        (JSC::ValueProfile::classInfo):
     25        (JSC::ValueProfile::numberOfInt32s):
     26        (JSC::ValueProfile::numberOfDoubles):
     27        (JSC::ValueProfile::numberOfCells):
     28        (JSC::ValueProfile::numberOfArrays):
     29        (JSC::ValueProfile::probabilityOfArray):
     30        (JSC::ValueProfile::WeakBucket::WeakBucket):
     31        (JSC::ValueProfile::WeakBucket::operator!):
     32        (JSC::ValueProfile::WeakBucket::isEmpty):
     33        (JSC::ValueProfile::WeakBucket::isClassInfo):
     34        (JSC::ValueProfile::WeakBucket::isStructure):
     35        (JSC::ValueProfile::WeakBucket::asStructure):
     36        (JSC::ValueProfile::WeakBucket::asClassInfo):
     37        (JSC::ValueProfile::WeakBucket::getClassInfo):
     38        * heap/Heap.cpp:
     39        (JSC::Heap::harvestWeakReferences):
     40        (JSC::Heap::markRoots):
     41        * heap/Heap.h:
     42        * heap/MarkStack.cpp:
     43        (JSC::SlotVisitor::drain):
     44        (JSC::SlotVisitor::harvestWeakReferences):
     45        * heap/MarkStack.h:
     46        (JSC::MarkStack::addWeakReferenceHarvester):
     47        (JSC::MarkStack::MarkStack):
     48        (JSC::MarkStack::appendUnbarrieredPointer):
     49        * heap/SlotVisitor.h:
     50        * heap/WeakReferenceHarvester.h: Added.
     51        (JSC::WeakReferenceHarvester::WeakReferenceHarvester):
     52        (JSC::WeakReferenceHarvester::~WeakReferenceHarvester):
     53
    1542011-09-02  Michael Saboff  <[email protected]>
    255

  • trunk/Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj

    r94470 r94477  
    5050                0BDFFAE10FC6193100D69EF4 /* OwnFastMallocPtr.h in Headers */ = {isa = PBXBuildFile; fileRef = 0BDFFAD10FC616EC00D69EF4 /* OwnFastMallocPtr.h */; settings = {ATTRIBUTES = (Private, ); }; };
    5151                0BF28A2911A33DC300638F84 /* SizeLimits.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 0BF28A2811A33DC300638F84 /* SizeLimits.cpp */; };
     52                0F242DA713F3B1E8007ADD4C /* WeakReferenceHarvester.h in Headers */ = {isa = PBXBuildFile; fileRef = 0F242DA513F3B1BB007ADD4C /* WeakReferenceHarvester.h */; settings = {ATTRIBUTES = (Private, ); }; };
    5253                0F7700921402FF3C0078EB39 /* SamplingCounter.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 0F7700911402FF280078EB39 /* SamplingCounter.cpp */; };
    5354                0F963B3813FC6FE90002D9B2 /* ValueProfile.h in Headers */ = {isa = PBXBuildFile; fileRef = 0F963B3613FC6FDE0002D9B2 /* ValueProfile.h */; settings = {ATTRIBUTES = (Private, ); }; };
    … …  
    771772                0BDFFAD40FC6171000D69EF4 /* CrossThreadRefCounted.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = CrossThreadRefCounted.h; sourceTree = "<group>"; };
    772773                0BF28A2811A33DC300638F84 /* SizeLimits.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = SizeLimits.cpp; sourceTree = "<group>"; };
     774                0F242DA513F3B1BB007ADD4C /* WeakReferenceHarvester.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = WeakReferenceHarvester.h; sourceTree = "<group>"; };
    773775                0F77008E1402FDD60078EB39 /* SamplingCounter.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = SamplingCounter.h; sourceTree = "<group>"; };
    774776                0F7700911402FF280078EB39 /* SamplingCounter.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = SamplingCounter.cpp; sourceTree = "<group>"; };
    … …  
    15881590                        isa = PBXGroup;
    15891591                        children = (
     1592                                0F242DA513F3B1BB007ADD4C /* WeakReferenceHarvester.h */,
    15901593                                0FC815141405118D00CFA603 /* VTableSpectrum.h */,
    15911594                                0FC815121405118600CFA603 /* VTableSpectrum.cpp */,
    … …  
    23572360                        buildActionMask = 2147483647;
    23582361                        files = (
     2362                                0F242DA713F3B1E8007ADD4C /* WeakReferenceHarvester.h in Headers */,
    23592363                                860161E30F3A83C100F84710 /* AbstractMacroAssembler.h in Headers */,
    23602364                                BC18C3E40E16F5CD00B34460 /* AlwaysInline.h in Headers */,

  • trunk/Source/JavaScriptCore/bytecode/CodeBlock.cpp

    r93466 r94477  
    14341434{
    14351435#if ENABLE(VERBOSE_VALUE_PROFILE)
    1436     printf("ValueProfile for %p:\n", this);
     1436    fprintf(stderr, "ValueProfile for %p:\n", this);
    14371437    for (unsigned i = 0; i < numberOfValueProfiles(); ++i) {
    14381438        ValueProfile* profile = valueProfile(i);
    14391439        if (profile->bytecodeOffset < 0) {
    14401440            ASSERT(profile->bytecodeOffset == -1);
    1441             printf("   arg = %u: ", i + 1);
     1441            fprintf(stderr, "   arg = %u: ", i + 1);
    14421442        } else
    1443             printf("   bc = %d: ", profile->bytecodeOffset);
    1444         printf("samples = %u, int32 = %u, double = %u, cell = %u\n",
    1445                profile->numberOfSamples(),
    1446                profile->probabilityOfInt32(),
    1447                profile->probabilityOfDouble(),
    1448                profile->probabilityOfCell());
     1443            fprintf(stderr, "   bc = %d: ", profile->bytecodeOffset);
     1444        fprintf(stderr,
     1445                "samples = %u, int32 = %u, double = %u, cell = %u, array = %u\n",
     1446                profile->numberOfSamples(),
     1447                profile->probabilityOfInt32(),
     1448                profile->probabilityOfDouble(),
     1449                profile->probabilityOfCell(),
     1450                profile->probabilityOfArray());
    14491451    }
    14501452#endif
    … …  
    15161518void CodeBlock::visitAggregate(SlotVisitor& visitor)
    15171519{
     1520    bool handleWeakReferences = false;
     1521   
    15181522    visitor.append(&m_globalObject);
    15191523    visitor.append(&m_ownerExecutable);
    … …  
    15631567    }
    15641568#endif
     1569
     1570#if ENABLE(VALUE_PROFILER)
     1571    for (unsigned profileIndex = 0; profileIndex < numberOfValueProfiles(); ++profileIndex) {
     1572        ValueProfile* profile = valueProfile(profileIndex);
     1573       
     1574        for (unsigned index = 0; index < ValueProfile::numberOfBuckets; ++index) {
     1575            if (!profile->buckets[index]) {
     1576                if (!!profile->weakBuckets[index])
     1577                    handleWeakReferences = true;
     1578                continue;
     1579            }
     1580           
     1581            if (!JSValue::decode(profile->buckets[index]).isCell()) {
     1582                profile->weakBuckets[index] = ValueProfile::WeakBucket();
     1583                continue;
     1584            }
     1585           
     1586            handleWeakReferences = true;
     1587        }
     1588    }
     1589#endif
     1590   
     1591    if (handleWeakReferences)
     1592        visitor.addWeakReferenceHarvester(this);
     1593}
     1594
     1595void CodeBlock::visitWeakReferences(SlotVisitor&)
     1596{
     1597#if ENABLE(VALUE_PROFILER)
     1598    for (unsigned profileIndex = 0; profileIndex < numberOfValueProfiles(); ++profileIndex) {
     1599        ValueProfile* profile = valueProfile(profileIndex);
     1600       
     1601        for (unsigned index = 0; index < ValueProfile::numberOfBuckets; ++index) {
     1602            if (!!profile->buckets[index]) {
     1603                JSValue value = JSValue::decode(profile->buckets[index]);
     1604                if (!value.isCell())
     1605                    continue;
     1606               
     1607                JSCell* cell = value.asCell();
     1608                if (Heap::isMarked(cell))
     1609                    continue;
     1610               
     1611                profile->buckets[index] = JSValue::encode(JSValue());
     1612                profile->weakBuckets[index] = cell->structure();
     1613            }
     1614           
     1615            ValueProfile::WeakBucket weak = profile->weakBuckets[index];
     1616            if (!weak || weak.isClassInfo())
     1617                continue;
     1618           
     1619            ASSERT(weak.isStructure());
     1620            if (Heap::isMarked(weak.asStructure()))
     1621                continue;
     1622           
     1623            profile->weakBuckets[index] = weak.asStructure()->classInfo();
     1624        }
     1625    }
     1626#endif
    15651627}
    15661628

  • trunk/Source/JavaScriptCore/bytecode/CodeBlock.h

    r93466 r94477  
    4040#include "RegExpObject.h"
    4141#include "UString.h"
     42#include "WeakReferenceHarvester.h"
    4243#include "ValueProfile.h"
    4344#include <wtf/FastAllocBase.h>
    … …  
    206207#endif
    207208
    208     class CodeBlock {
     209    class CodeBlock: public WeakReferenceHarvester {
    209210        WTF_MAKE_FAST_ALLOCATED;
    210211        friend class JIT;
    … …  
    219220
    220221        void visitAggregate(SlotVisitor&);
     222        void visitWeakReferences(SlotVisitor&);
    221223
    222224        static void dumpStatistics();

  • trunk/Source/JavaScriptCore/bytecode/ValueProfile.h

    r93466 r94477  
    3030#define ValueProfile_h
    3131
     32#include "JSArray.h"
     33#include "Structure.h"
    3234#include "WriteBarrier.h"
    3335
    … …  
    4648    {
    4749        for (unsigned i = 0; i < numberOfBuckets; ++i)
    48             buckets[i].setWithoutWriteBarrier(JSValue());
     50            buckets[i] = JSValue::encode(JSValue());
     51    }
     52   
     53    const ClassInfo* classInfo(unsigned bucket) const
     54    {
     55        if (!!buckets[bucket]) {
     56            JSValue value = JSValue::decode(buckets[bucket]);
     57            if (!value.isCell())
     58                return 0;
     59            return value.asCell()->structure()->classInfo();
     60        }
     61        return weakBuckets[bucket].getClassInfo();
    4962    }
    5063   
    … …  
    5366        unsigned result = 0;
    5467        for (unsigned i = 0; i < numberOfBuckets; ++i) {
    55             if (!!buckets[i])
     68            if (!!buckets[i] || !!weakBuckets[i])
    5669                result++;
    5770        }
    … …  
    7083        unsigned result = 0;
    7184        for (unsigned i = 0; i < numberOfBuckets; ++i) {
    72             if (!!buckets[i] && buckets[i].get().isInt32())
     85            if (!!buckets[i] && JSValue::decode(buckets[i]).isInt32())
    7386                result++;
    7487        }
    … …  
    8093        unsigned result = 0;
    8194        for (unsigned i = 0; i < numberOfBuckets; ++i) {
    82             if (!!buckets[i] && buckets[i].get().isDouble())
     95            if (!!buckets[i] && JSValue::decode(buckets[i]).isDouble())
    8396                result++;
    8497        }
    … …  
    90103        unsigned result = 0;
    91104        for (unsigned i = 0; i < numberOfBuckets; ++i) {
    92             if (!!buckets[i] && buckets[i].get().isCell())
     105            if (!!classInfo(i))
     106                result++;
     107        }
     108        return result;
     109    }
     110   
     111    unsigned numberOfArrays() const
     112    {
     113        unsigned result = 0;
     114        for (unsigned i = 0; i < numberOfBuckets; ++i) {
     115            if (classInfo(i) == &JSArray::s_info)
    93116                result++;
    94117        }
    … …  
    116139    }
    117140   
     141    unsigned probabilityOfArray() const
     142    {
     143        return computeProbability(numberOfArrays(), numberOfSamples());
     144    }
     145   
    118146    int bytecodeOffset; // -1 for prologue
    119     WriteBarrierBase<Unknown> buckets[numberOfBuckets];
     147    EncodedJSValue buckets[numberOfBuckets];
     148   
     149    class WeakBucket {
     150    public:
     151        WeakBucket()
     152            : m_value(0)
     153        {
     154        }
     155       
     156        WeakBucket(Structure* structure)
     157            : m_value(reinterpret_cast<uintptr_t>(structure))
     158        {
     159        }
     160       
     161        WeakBucket(const ClassInfo* classInfo)
     162            : m_value(reinterpret_cast<uintptr_t>(classInfo) | 1)
     163        {
     164        }
     165       
     166        bool operator!() const
     167        {
     168            return !m_value;
     169        }
     170       
     171        bool isEmpty() const
     172        {
     173            return !m_value;
     174        }
     175       
     176        bool isClassInfo() const
     177        {
     178            return !!(m_value & 1);
     179        }
     180       
     181        bool isStructure() const
     182        {
     183            return !isEmpty() && !isClassInfo();
     184        }
     185       
     186        Structure* asStructure() const
     187        {
     188            ASSERT(isStructure());
     189            return reinterpret_cast<Structure*>(m_value);
     190        }
     191       
     192        const ClassInfo* asClassInfo() const
     193        {
     194            ASSERT(isClassInfo());
     195            return reinterpret_cast<ClassInfo*>(m_value & ~static_cast<uintptr_t>(1));
     196        }
     197       
     198        const ClassInfo* getClassInfo() const
     199        {
     200            if (isEmpty())
     201                return 0;
     202            if (isClassInfo())
     203                return asClassInfo();
     204            return asStructure()->classInfo();
     205        }
     206       
     207    private:
     208        uintptr_t m_value;
     209    };
     210   
     211    WeakBucket weakBuckets[numberOfBuckets]; // this is not covered by a write barrier because it is only set from GC
    120212};
    121213

  • trunk/Source/JavaScriptCore/heap/Heap.cpp

    r94461 r94477  
    514514}
    515515
     516void Heap::harvestWeakReferences()
     517{
     518    m_slotVisitor.harvestWeakReferences();
     519}
     520
    516521inline RegisterFile& Heap::registerFile()
    517522{
    … …  
    581586    m_handleStack.visit(heapRootVisitor);
    582587    visitor.drain();
     588
     589    harvestWeakReferences();
    583590
    584591    // Weak handles must be marked last, because their owners use the set of

  • trunk/Source/JavaScriptCore/heap/Heap.h

    r94445 r94477  
    153153        void markProtectedObjects(HeapRootVisitor&);
    154154        void markTempSortVectors(HeapRootVisitor&);
     155        void harvestWeakReferences();
    155156
    156157        void* tryAllocate(NewSpace::SizeClass&);

  • trunk/Source/JavaScriptCore/heap/MarkStack.cpp

    r93918 r94477  
    137137}
    138138
     139void SlotVisitor::harvestWeakReferences()
     140{
     141    while (m_firstWeakReferenceHarvester) {
     142        WeakReferenceHarvester* current = m_firstWeakReferenceHarvester;
     143        WeakReferenceHarvester* next = reinterpret_cast<WeakReferenceHarvester*>(current->m_nextAndFlag & ~1);
     144        current->m_nextAndFlag = 0;
     145        m_firstWeakReferenceHarvester = next;
     146        current->visitWeakReferences(*this);
     147    }
     148}
     149
    139150#if ENABLE(GC_VALIDATION)
    140151void MarkStack::validateSet(JSValue* values, size_t count)

  • trunk/Source/JavaScriptCore/heap/MarkStack.h

    r93918 r94477  
    3131#include "Register.h"
    3232#include "VTableSpectrum.h"
     33#include "WeakReferenceHarvester.h"
    3334#include <wtf/HashMap.h>
    3435#include <wtf/HashSet.h>
    … …  
    9495        inline void appendValues(WriteBarrierBase<Unknown>*, size_t count);
    9596       
     97        template<typename T>
     98        inline void appendUnbarrieredPointer(T**);
     99       
    96100        bool addOpaqueRoot(void*);
    97101        bool containsOpaqueRoot(void*);
    … …  
    103107        VTableSpectrum m_visitedTypeCounts;
    104108#endif
     109
     110        void addWeakReferenceHarvester(WeakReferenceHarvester* weakReferenceHarvester)
     111        {
     112            if (weakReferenceHarvester->m_nextAndFlag & 1)
     113                return;
     114            weakReferenceHarvester->m_nextAndFlag = reinterpret_cast<uintptr_t>(m_firstWeakReferenceHarvester) | 1;
     115            m_firstWeakReferenceHarvester = weakReferenceHarvester;
     116        }
    105117
    106118    protected:
    … …  
    121133        MarkStackArray<JSCell*> m_values;
    122134        HashSet<void*> m_opaqueRoots; // Handle-owning data structures not visible to the garbage collector.
     135        WeakReferenceHarvester* m_firstWeakReferenceHarvester;
    123136       
    124137#if !ASSERT_DISABLED
    … …  
    131144    inline MarkStack::MarkStack(void* jsArrayVPtr)
    132145        : m_jsArrayVPtr(jsArrayVPtr)
     146        , m_firstWeakReferenceHarvester(0)
    133147#if !ASSERT_DISABLED
    134148        , m_isCheckingForDefaultMarkViolation(false)
    … …  
    256270        m_markSets.append(MarkSet(slot, slot + count));
    257271    }
     272
     273    template<typename T>
     274    inline void MarkStack::appendUnbarrieredPointer(T** slot)
     275    {
     276        ASSERT(slot);
     277        JSCell* value = *slot;
     278        if (value)
     279            internalAppend(value);
     280    }
    258281   
    259282    ALWAYS_INLINE void MarkStack::append(JSValue* value)

  • trunk/Source/JavaScriptCore/heap/SlotVisitor.h

    r89069 r94477  
    3636
    3737    void drain();
    38 
     38    void harvestWeakReferences();
     39   
    3940private:
    4041    void visitChildren(JSCell*);
Note: See TracChangeset for help on using the changeset viewer.