From 194c582354785f9a62b2e0c947b9b998a8596898 Mon Sep 17 00:00:00 2001 From: Lars Knoll Date: Mon, 11 Feb 2013 16:41:42 +0100 Subject: [PATCH] Fixes to the memory manager Don't use the location of the bitfield to store the nextFree pointer. This can easily lead to trouble when changing the layout of the bitfield, as a we rely on inUse being 0. Rather use the location where the vtable is being stored. Fix the sweep method in the memory manager to correctly insert all objects into the free list. Fix mark, to really discard objects that are out of bounds. Change-Id: I902e46907043c9d4288d0e3d1564460b5b265c4f Reviewed-by: Simon Hausmann --- src/v4/qv4managed.cpp | 3 +-- src/v4/qv4managed.h | 54 +++++++++++++++++++++++---------------------------- src/v4/qv4mm.cpp | 49 ++++++++++++++++++++++++++++++---------------- 3 files changed, 57 insertions(+), 49 deletions(-) diff --git a/src/v4/qv4managed.cpp b/src/v4/qv4managed.cpp index e0448a4..656531c 100644 --- a/src/v4/qv4managed.cpp +++ b/src/v4/qv4managed.cpp @@ -47,8 +47,7 @@ using namespace QQmlJS::VM; Managed::~Managed() { - nextFree = 0; - inUse = 0; + _data = 0; } void *Managed::operator new(size_t size, MemoryManager *mm) diff --git a/src/v4/qv4managed.h b/src/v4/qv4managed.h index 7aa1452..14063de 100644 --- a/src/v4/qv4managed.h +++ b/src/v4/qv4managed.h @@ -81,21 +81,8 @@ private: protected: Managed() - : markBit(0) - , inUse(1) - , extensible(1) - , isNonStrictArgumentsObject(0) - , isBuiltinFunction(0) - , needsActivation(0) - , usesArgumentsObject(0) - , strictMode(0) - , type(Type_Invalid) - , subtype(0) - , stringIdentifier(0) -#if CPU(X86_64) - , unused(0) -#endif - {} + : _data(0) + { inUse = 1; extensible = 1; } virtual ~Managed(); public: @@ -147,26 +134,33 @@ public: QString className() const; + Managed **nextFreeRef() { + return reinterpret_cast(this); + } + Managed *nextFree() { + return *reinterpret_cast(this); + } + void setNextFree(Managed *m) { + *reinterpret_cast(this) = m; + } + protected: virtual void markObjects() {} union { - Managed *nextFree; + uint _data; struct { - quintptr markBit : 1; - quintptr inUse : 1; - quintptr extensible : 1; // used by Object - quintptr isNonStrictArgumentsObject : 1; - quintptr isBuiltinFunction : 1; // used by FunctionObject - quintptr needsActivation : 1; // used by FunctionObject - quintptr usesArgumentsObject : 1; // used by FunctionObject - quintptr strictMode : 1; // used by FunctionObject - quintptr type : 5; - mutable quintptr subtype : 3; - mutable quintptr stringIdentifier : 16; -#if CPU(X86_64) - quintptr unused : 32; -#endif + uint markBit : 1; + uint inUse : 1; + uint extensible : 1; // used by Object + uint isNonStrictArgumentsObject : 1; + uint isBuiltinFunction : 1; // used by FunctionObject + uint needsActivation : 1; // used by FunctionObject + uint usesArgumentsObject : 1; // used by FunctionObject + uint strictMode : 1; // used by FunctionObject + uint type : 5; + mutable uint subtype : 3; + uint unused : 16; }; }; diff --git a/src/v4/qv4mm.cpp b/src/v4/qv4mm.cpp index 2724700..81dd39a 100644 --- a/src/v4/qv4mm.cpp +++ b/src/v4/qv4mm.cpp @@ -90,6 +90,11 @@ struct MemoryManager::Data } }; +#define SCRIBBLE(obj, c, size) \ + if (m_d->scribble) \ + ::memset((void *)(obj + 1), c, size - sizeof(Managed)); + + namespace QQmlJS { namespace VM { bool operator<(const MemoryManager::Data::Chunk &a, const MemoryManager::Data::Chunk &b) @@ -152,10 +157,9 @@ Managed *MemoryManager::alloc(std::size_t size) Managed **last = &m_d->smallItems[pos]; while (chunk <= end) { Managed *o = reinterpret_cast(chunk); - o->inUse = 0; - o->markBit = 0; + o->_data = 0; *last = o; - last = &o->nextFree; + last = o->nextFreeRef(); chunk += size; } *last = 0; @@ -163,14 +167,10 @@ Managed *MemoryManager::alloc(std::size_t size) } found: - m_d->smallItems[pos] = m->nextFree; + m_d->smallItems[pos] = m->nextFree(); return m; } -#define SCRIBBLE(obj, c, size) \ - if (m_d->scribble) \ - ::memset((void *)(obj + 1), c, size - sizeof(Managed)); - void MemoryManager::mark() { m_d->engine->markObjects(); @@ -181,6 +181,20 @@ void MemoryManager::mark() for (QHash::const_iterator it = m_d->protectedObject.begin(); it != m_d->protectedObject.constEnd(); ++it) it.key()->mark(); +#if CPU(X86_64) + // push all caller saved registers to the stack, so we can find the objects living in these registers + quintptr regs[5]; + asm( + "mov %%rbp, %0\n" + "mov %%r12, %1\n" + "mov %%r13, %2\n" + "mov %%r14, %3\n" + "mov %%r15, %4\n" + : "=m" (regs[0]), "=m" (regs[1]), "=m" (regs[2]), "=m" (regs[3]), "=m" (regs[4]) + : + : + ); +#endif collectFromStack(); return; @@ -203,23 +217,22 @@ std::size_t MemoryManager::sweep(char *chunkStart, std::size_t chunkSize, size_t Managed **f = &m_d->smallItems[size >> 4]; - for (char *chunk = chunkStart, *chunkEnd = chunk + chunkSize - size; chunk <= chunkEnd; ) { + for (char *chunk = chunkStart, *chunkEnd = chunk + chunkSize - size; chunk <= chunkEnd; chunk += size) { Managed *m = reinterpret_cast(chunk); // qDebug("chunk @ %p, size = %lu, in use: %s, mark bit: %s", // chunk, m->size, (m->inUse ? "yes" : "no"), (m->markBit ? "true" : "false")); assert((intptr_t) chunk % 16 == 0); - chunk = chunk + size; if (m->inUse) { if (m->markBit) { m->markBit = 0; } else { -// qDebug() << "-- collecting it." << m << *f << &m->nextFree; +// qDebug() << "-- collecting it." << m << *f << m->nextFree(); m->~Managed(); - m->nextFree = *f; - f = &m->nextFree; + m->setNextFree(*f); + *f = m; SCRIBBLE(m, 0x99, size); ++freedCount; } @@ -360,7 +373,7 @@ void MemoryManager::collectFromStack() const // qDebug() << "stack:" << hex << stackTop << stackSize << (stackTop + stackSize); quintptr *current = (&valueOnStack) + 1; -// qDebug() << "collectFromStack" << top << current << &valueOnStack; +// qDebug() << "collectFromStack";// << top << current << &valueOnStack; char** heapChunkBoundaries = (char**)alloca(m_d->heapChunks.count() * 2 * sizeof(char*)); char** heapChunkBoundariesEnd = heapChunkBoundaries + 2 * m_d->heapChunks.count(); @@ -370,6 +383,7 @@ void MemoryManager::collectFromStack() const heapChunkBoundaries[i++] = reinterpret_cast(it->memory.base()); heapChunkBoundaries[i++] = reinterpret_cast(it->memory.base()) + it->memory.size() - it->chunkSize; } + assert(i == m_d->heapChunks.count() * 2); for (; current < top; ++current) { char* genericPtr = @@ -379,14 +393,15 @@ void MemoryManager::collectFromStack() const reinterpret_cast(*current); #endif - if (genericPtr < *heapChunkBoundaries || genericPtr > *heapChunkBoundariesEnd) + if (genericPtr < *heapChunkBoundaries || genericPtr > *(heapChunkBoundariesEnd - 1)) continue; int index = qLowerBound(heapChunkBoundaries, heapChunkBoundariesEnd, genericPtr) - heapChunkBoundaries; // An odd index means the pointer is _before_ the end of a heap chunk and therefore valid. + assert(index >= 0 && index < m_d->heapChunks.count() * 2); if (index & 1) { int size = m_d->heapChunks.at(index >> 1).chunkSize; Managed *m = reinterpret_cast(genericPtr); -// qDebug() << " inside" << size << m; +// qDebug() << " inside" << size; if (((quintptr)m - (quintptr)heapChunkBoundaries[index-1]) % size) // wrongly aligned value, skip it @@ -396,8 +411,8 @@ void MemoryManager::collectFromStack() const // Skip pointers to already freed objects, they are bogus as well continue; - m->mark(); // qDebug() << " marking"; + m->mark(); } } } -- 2.7.4