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 <simon.hausmann@digia.com>
This commit is contained in:
Lars Knoll 2013-02-11 16:41:42 +01:00 committed by Simon Hausmann
parent 8c10aeb9fa
commit 194c582354
3 changed files with 57 additions and 49 deletions

View File

@ -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)

View File

@ -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<Managed **>(this);
}
Managed *nextFree() {
return *reinterpret_cast<Managed **>(this);
}
void setNextFree(Managed *m) {
*reinterpret_cast<Managed **>(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;
};
};

View File

@ -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<Managed *>(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<Managed *, uint>::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<Managed *>(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<char*>(it->memory.base());
heapChunkBoundaries[i++] = reinterpret_cast<char*>(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<char *>(*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<Managed *>(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();
}
}
}