Changeset 178364 in webkit for trunk/Source/JavaScriptCore/heap


Ignore:
Timestamp:
Jan 13, 2015, 9:46:40 AM (10 years ago)
Author:
[email protected]
Message:

Local JSArray* "keys" in objectConstructorKeys() is not marked during garbage collection
​https://bugs.webkit.org/show_bug.cgi?id=140348

Reviewed by Mark Lam.

We used to read registers in MachineThreads::gatherFromCurrentThread(), but that is too late
because those registers may have been spilled on the stack and replaced with other values by
the time we call down to gatherFromCurrentThread().

Now we get the register contents at the same place that we demarcate the current top of
stack using the address of a local variable, in Heap::markRoots(). The register contents
buffer is passed along with the demarcation pointer. These need to be done at this level
in the call tree and no lower, as markRoots() calls various functions that visit object
pointers that may be latter proven dead. Any of those pointers that are left on the
stack or in registers could be incorrectly marked as live if we scan the stack contents
from a called function or one of its callees. The stack demarcation pointer and register
saving need to be done in the same function so that we have a consistent stack, active
and spilled registers.

Because we don't want to make unnecessary calls to get the register contents, we use
a macro to allocated, and possibly align, the register structure and get the actual
register contents.

  • heap/Heap.cpp:

(JSC::Heap::markRoots):
(JSC::Heap::gatherStackRoots):

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

(JSC::MachineThreads::gatherFromCurrentThread):
(JSC::MachineThreads::gatherConservativeRoots):

  • heap/MachineStackMarker.h:
Location:
trunk/Source/JavaScriptCore/heap
Files:
4 edited

Legend:

Unmodified
Added
Removed

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

    r178284 r178364  
    506506    // gathering uses the mark bits to determine whether a reference is valid.
    507507    void* dummy;
     508    ALLOCATE_AND_GET_REGISTER_STATE(registers);
    508509    ConservativeRoots conservativeRoots(&m_objectSpace.blocks(), &m_storageSpace);
    509     gatherStackRoots(conservativeRoots, &dummy);
     510    gatherStackRoots(conservativeRoots, &dummy, registers);
    510511    gatherJSStackRoots(conservativeRoots);
    511512    gatherScratchBufferRoots(conservativeRoots);
    … …  
    567568}
    568569
    569 void Heap::gatherStackRoots(ConservativeRoots& roots, void** dummy)
     570void Heap::gatherStackRoots(ConservativeRoots& roots, void** dummy, MachineThreads::RegisterState& registers)
    570571{
    571572    GCPHASE(GatherStackRoots);
    572573    m_jitStubRoutines.clearMarks();
    573     m_machineThreads.gatherConservativeRoots(roots, m_jitStubRoutines, m_codeBlocks, dummy);
     574    m_machineThreads.gatherConservativeRoots(roots, m_jitStubRoutines, m_codeBlocks, dummy, registers);
    574575}
    575576

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

    r178284 r178364  
    276276
    277277    void markRoots(double gcStartTime);
    278     void gatherStackRoots(ConservativeRoots&, void** dummy);
     278    void gatherStackRoots(ConservativeRoots&, void** dummy, MachineThreads::RegisterState& registers);
    279279    void gatherJSStackRoots(ConservativeRoots&);
    280280    void gatherScratchBufferRoots(ConservativeRoots&);

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

    r178284 r178364  
    216216}
    217217
    218 #if COMPILER(GCC)
    219 #define REGISTER_BUFFER_ALIGNMENT __attribute__ ((aligned (sizeof(void*))))
    220 #else
    221 #define REGISTER_BUFFER_ALIGNMENT
    222 #endif
    223 
    224 void MachineThreads::gatherFromCurrentThread(ConservativeRoots& conservativeRoots, JITStubRoutineSet& jitStubRoutines, CodeBlockSet& codeBlocks, void* stackCurrent)
    225 {
    226     // setjmp forces volatile registers onto the stack
    227     jmp_buf registers REGISTER_BUFFER_ALIGNMENT;
    228 #if COMPILER(MSVC)
    229 #pragma warning(push)
    230 #pragma warning(disable: 4611)
    231 #endif
    232     setjmp(registers);
    233 #if COMPILER(MSVC)
    234 #pragma warning(pop)
    235 #endif
    236 
     218void MachineThreads::gatherFromCurrentThread(ConservativeRoots& conservativeRoots, JITStubRoutineSet& jitStubRoutines, CodeBlockSet& codeBlocks, void* stackCurrent, RegisterState& registers)
     219{
    237220    void* registersBegin = &registers;
    238221    void* registersEnd = reinterpret_cast<void*>(roundUpToMultipleOf<sizeof(void*)>(reinterpret_cast<uintptr_t>(&registers + 1)));
    … …  
    446429}
    447430
    448 void MachineThreads::gatherConservativeRoots(ConservativeRoots& conservativeRoots, JITStubRoutineSet& jitStubRoutines, CodeBlockSet& codeBlocks, void* stackCurrent)
    449 {
    450     gatherFromCurrentThread(conservativeRoots, jitStubRoutines, codeBlocks, stackCurrent);
     431void MachineThreads::gatherConservativeRoots(ConservativeRoots& conservativeRoots, JITStubRoutineSet& jitStubRoutines, CodeBlockSet& codeBlocks, void* stackCurrent, RegisterState& registers)
     432{
     433    gatherFromCurrentThread(conservativeRoots, jitStubRoutines, codeBlocks, stackCurrent, registers);
    451434
    452435    if (m_threadSpecific) {

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

    r178284 r178364  
    2323#define MachineThreads_h
    2424
     25#include <setjmp.h>
    2526#include <wtf/Noncopyable.h>
    2627#include <wtf/ThreadSpecific.h>
    … …  
    3738        WTF_MAKE_NONCOPYABLE(MachineThreads);
    3839    public:
     40        typedef jmp_buf RegisterState;
     41
    3942        MachineThreads(Heap*);
    4043        ~MachineThreads();
    4144
    42         void gatherConservativeRoots(ConservativeRoots&, JITStubRoutineSet&, CodeBlockSet&, void* stackCurrent);
     45        void gatherConservativeRoots(ConservativeRoots&, JITStubRoutineSet&, CodeBlockSet&, void* stackCurrent, RegisterState& registers);
    4346
    4447        JS_EXPORT_PRIVATE void makeUsableFromMultipleThreads();
    … …  
    4649
    4750    private:
    48         void gatherFromCurrentThread(ConservativeRoots&, JITStubRoutineSet&, CodeBlockSet&, void* stackCurrent);
     51        void gatherFromCurrentThread(ConservativeRoots&, JITStubRoutineSet&, CodeBlockSet&, void* stackCurrent, RegisterState& registers);
    4952
    5053        class Thread;
    … …  
    6568} // namespace JSC
    6669
     70#if COMPILER(GCC)
     71#define REGISTER_BUFFER_ALIGNMENT __attribute__ ((aligned (sizeof(void*))))
     72#else
     73#define REGISTER_BUFFER_ALIGNMENT
     74#endif
     75
     76// ALLOCATE_AND_GET_REGISTER_STATE() is a macro so that it is always "inlined" even in debug builds.
     77#if COMPILER(MSVC)
     78#pragma warning(push)
     79#pragma warning(disable: 4611)
     80#define ALLOCATE_AND_GET_REGISTER_STATE(registers) \
     81    MachineThreads::RegisterState registers REGISTER_BUFFER_ALIGNMENT; \
     82    setjmp(registers)
     83#pragma warning(pop)
     84#else
     85#define ALLOCATE_AND_GET_REGISTER_STATE(registers) \
     86    MachineThreads::RegisterState registers REGISTER_BUFFER_ALIGNMENT; \
     87    setjmp(registers)
     88#endif
     89
    6790#endif // MachineThreads_h
Note: See TracChangeset for help on using the changeset viewer.