Browse files

Fix memory corruption bug, improve immix metadata

  • Loading branch information...
1 parent 3a06656 commit 16f7f92942abc1faa4da82aaef076c3f11e711a3 Evan Phoenix committed Mar 27, 2009
Showing with 97 additions and 43 deletions.
  1. +8 −6 vm/builtin/bytearray.cpp
  2. +0 −6 vm/builtin/object.cpp
  3. +0 −7 vm/builtin/object.hpp
  4. +16 −0 vm/gc/immix.cpp
  5. +15 −2 vm/gc/immix.hpp
  6. +3 −1 vm/object_position.hpp
  7. +3 −0 vm/objectmemory.cpp
  8. +12 −4 vm/oop.hpp
  9. +13 −13 vm/test/test_gc_immix.hpp
  10. +27 −4 vm/util/immix.hpp
View
14 vm/builtin/bytearray.cpp
@@ -19,16 +19,18 @@ namespace rubinius {
}
ByteArray* ByteArray::create(STATE, size_t bytes) {
- ByteArray* ba = state->om->new_object_bytes<ByteArray>(G(bytearray), bytes);
- ba->init_bytes(state);
- ba->full_size_ = bytes;
+ size_t body = bytes;
+ ByteArray* ba = state->om->new_object_bytes<ByteArray>(G(bytearray), body);
+ ba->full_size_ = body;
+ memset(ba->bytes, 0, bytes);
return ba;
}
ByteArray* ByteArray::create_pinned(STATE, size_t bytes) {
- ByteArray* ba = state->om->new_object_bytes_mature<ByteArray>(G(bytearray), bytes);
- ba->init_bytes(state);
- ba->full_size_ = bytes;
+ size_t body = bytes;
+ ByteArray* ba = state->om->new_object_bytes_mature<ByteArray>(G(bytearray), body);
+ ba->full_size_ = body;
+ memset(ba->bytes, 0, bytes);
assert(ba->pin());
return ba;
View
6 vm/builtin/object.cpp
@@ -254,12 +254,6 @@ namespace rubinius {
}
}
- /* Initialize the object as storing bytes, by setting the flag then clearing the
- * body of the object, by setting the entire body as bytes to 0 */
- void Object::init_bytes(STATE) {
- clear_body_to_null(size_in_bytes(state));
- }
-
bool Object::kind_of_p(STATE, Object* module) {
Module* found = NULL;
View
7 vm/builtin/object.hpp
@@ -98,13 +98,6 @@ namespace rubinius {
/** Calls cleanup() on the TypeInfo for this object's type. */
void cleanup(STATE);
- /**
- * Initialize the object to store bytes.
- *
- * Sets flag and overwrites body with NULLs.
- */
- void init_bytes(STATE);
-
/** Provides access to the GC write barrier from any object. */
void write_barrier(STATE, void* obj);
/** Special-case write_barrier() for Fixnums. */
View
16 vm/gc/immix.cpp
@@ -25,6 +25,7 @@ namespace rubinius {
ImmixGC::ImmixGC(ObjectMemory* om)
: GarbageCollector(om)
, allocator_(gc_.block_allocator())
+ , which_mark_(1)
{
gc_.describer().set_object_memory(om, this);
}
@@ -81,6 +82,18 @@ namespace rubinius {
return fwd.as<Object>();
}
+ ObjectPosition ImmixGC::validate_object(Object* obj) {
+ if(gc_.allocated_address(immix::Address(obj))) {
+ if(obj->in_immix_p()) {
+ return cInImmix;
+ } else {
+ return cInImmixCorruptHeader;
+ }
+ }
+
+ return cUnknown;
+ }
+
void ImmixGC::collect(GCData& data) {
Object* tmp;
@@ -144,6 +157,9 @@ namespace rubinius {
}
}
+ // Switch the which_mark_ for next time.
+ which_mark_ = (which_mark_ == 1 ? 2 : 1);
+
#ifdef IMMIX_DEBUG
std::cout << "Immix: RS size cleared: " << cleared << "\n";
View
17 vm/gc/immix.hpp
@@ -4,6 +4,8 @@
#include "util/immix.hpp"
#include "gc.hpp"
+#include "object_position.hpp"
+
namespace rubinius {
class ObjectMemory;
class ImmixGC;
@@ -50,8 +52,11 @@ namespace rubinius {
bool mark_address(immix::Address addr, immix::MarkStack& ms) {
Object* obj = addr.as<Object>();
- if(obj->marked_p()) return false;
- obj->mark();
+ if(obj->marked_p()) {
+ if(obj->marked_p(gc_->which_mark())) return false;
+ assert(0 && "invalid mark detectet!\n");
+ }
+ obj->mark(gc_->which_mark());
ms.push_back(addr);
@@ -66,6 +71,7 @@ namespace rubinius {
immix::GC<ObjectDescriber> gc_;
immix::ExpandingAllocator allocator_;
+ int which_mark_;
public:
ImmixGC(ObjectMemory* om);
@@ -75,6 +81,13 @@ namespace rubinius {
virtual Object* saw_object(Object*);
void collect(GCData& data);
+
+ ObjectPosition validate_object(Object*);
+
+ public: // Inline
+ int which_mark() {
+ return which_mark_;
+ }
};
}
View
4 vm/object_position.hpp
@@ -7,7 +7,9 @@ namespace rubinius {
cValid,
cInWrongYoungHalf,
cMatureObject,
- cContextStack
+ cContextStack,
+ cInImmix,
+ cInImmixCorruptHeader
};
}
View
3 vm/objectmemory.cpp
@@ -284,6 +284,9 @@ namespace rubinius {
pos = young.validate_object(obj);
if(pos != cUnknown) return pos;
+ pos = immix_.validate_object(obj);
+ if(pos != cUnknown) return pos;
+
return mark_sweep_.validate_object(obj);
}
};
View
16 vm/oop.hpp
@@ -137,7 +137,7 @@ const int cUndef = 0x22L;
unsigned int Forwarded : 1;
unsigned int Remember : 1;
- unsigned int Marked : 1;
+ unsigned int Marked : 2;
unsigned int RequiresCleanup : 1;
unsigned int RefsAreWeak : 1;
@@ -247,11 +247,19 @@ const int cUndef = 0x22L;
}
bool marked_p() const {
- return Marked == 1;
+ return Marked != 0;
}
- void mark() {
- Marked = 1;
+ bool marked_p(unsigned int which) const {
+ return Marked == which;
+ }
+
+ void mark(unsigned int which=1) {
+ Marked = which;
+ }
+
+ int which_mark() {
+ return Marked;
}
void clear_mark() {
View
26 vm/test/test_gc_immix.hpp
@@ -87,14 +87,14 @@ class TestImmixGC : public CxxTest::TestSuite {
TS_ASSERT_EQUALS(block.size(), immix::cBlockSize);
TS_ASSERT(block.address() != 0);
TS_ASSERT_EQUALS(block.status(), immix::cFree);
- TS_ASSERT_EQUALS(block.lines_used(), 0);
+ TS_ASSERT_EQUALS(block.lines_used(), 1);
}
void test_Block_is_line_free() {
immix::Block& block = gc->get_block();
- TS_ASSERT(block.is_line_free(0));
- block.mark_line(0);
- TS_ASSERT(!block.is_line_free(0));
+ TS_ASSERT(block.is_line_free(1));
+ block.mark_line(1);
+ TS_ASSERT(!block.is_line_free(1));
}
void test_Block_address_of_line() {
@@ -117,9 +117,9 @@ class TestImmixGC : public CxxTest::TestSuite {
void test_SingleBlockAllocator_allocate_checks_mark_on_spill() {
immix::Block& block = gc->get_block();
- immix::Address top = block.address();
+ immix::Address top = block.first_address();
- block.mark_line(1);
+ block.mark_line(2);
immix::SingleBlockAllocator alloc(block);
alloc.allocate(96);
@@ -245,7 +245,7 @@ class TestImmixGC : public CxxTest::TestSuite {
block.update_stats();
TS_ASSERT_EQUALS(block.status(), immix::cFree);
TS_ASSERT_EQUALS(block.holes(), 1);
- TS_ASSERT_EQUALS(block.lines_used(), 0);
+ TS_ASSERT_EQUALS(block.lines_used(), 1);
}
void test_Block_update_stats_finds_unavailable_blocks() {
@@ -277,9 +277,9 @@ class TestImmixGC : public CxxTest::TestSuite {
immix::SingleBlockAllocator alloc(block);
immix::Address addr = alloc.allocate(24);
- TS_ASSERT(block.is_line_free(0));
+ TS_ASSERT(block.is_line_free(1));
gc->mark_address(addr, alloc);
- TS_ASSERT(!block.is_line_free(0));
+ TS_ASSERT(!block.is_line_free(1));
}
void test_mark_address_ignores_already_marked_objects() {
@@ -289,9 +289,9 @@ class TestImmixGC : public CxxTest::TestSuite {
addr.as<SimpleObject>()->marked = true;
- TS_ASSERT(block.is_line_free(0));
+ TS_ASSERT(block.is_line_free(1));
gc->mark_address(addr, alloc);
- TS_ASSERT(block.is_line_free(0));
+ TS_ASSERT(block.is_line_free(1));
}
void test_mark_address_returns_forwarding_pointer() {
@@ -408,11 +408,11 @@ class TestImmixGC : public CxxTest::TestSuite {
block.mark_line(1);
block.mark_line(3);
- TS_ASSERT_EQUALS(block.lines_used(), 0);
+ TS_ASSERT_EQUALS(block.lines_used(), 1);
immix::BlockAllocator& ba = gc->block_allocator();
ba.reset();
- TS_ASSERT_EQUALS(block.lines_used(), 2);
+ TS_ASSERT_EQUALS(block.lines_used(), 3);
}
void test_BlockAllocator_get_free_block() {
View
31 vm/util/immix.hpp
@@ -105,7 +105,7 @@ namespace immix {
: address_(0)
, status_(cFree)
, holes_(1)
- , lines_used_(0)
+ , lines_used_(1)
, objects_(0)
, object_bytes_(0)
{
@@ -116,6 +116,7 @@ namespace immix {
objects_ = 0;
object_bytes_ = 0;
memset(lines_, 0, sizeof(lines_));
+ lines_[0] = 1; // always exclude the first line, it's got metadata
}
void set_address(Address addr) {
@@ -135,7 +136,7 @@ namespace immix {
}
Address first_address() {
- return address_ + sizeof(BlockHeader);
+ return address_ + cLineSize; // skip line 0
}
BlockStatus status() const {
@@ -225,9 +226,10 @@ namespace immix {
}
}
- if(lines_used_ == 0) {
+ // 1 is always used for metadata
+ if(lines_used_ == 1) {
status_ = cFree;
- } else if(holes_ > 0) {
+ } else if(holes_ > 1) {
status_ = cRecyclable;
} else {
status_ = cUnavailable;
@@ -315,6 +317,10 @@ namespace immix {
return system_size_;
}
+ Address last_address() {
+ return system_base_ + system_size_;
+ }
+
void add_blocks() {
assert(base_ == Block::align(base_));
@@ -338,6 +344,10 @@ namespace immix {
blocks_[i].update_stats();
}
}
+
+ bool contains_address(Address addr) {
+ return addr > base_ && addr <= last_address();
+ }
};
typedef std::vector<Chunk*> Chunks;
@@ -523,9 +533,11 @@ namespace immix {
cursor_ = block_->address_of_line(hole_start_line_);
// Compensate for the header
+ /*
if(hole_start_line_ == 0) {
cursor_ = block_->first_address();
}
+ */
while(hole_start_line_ < cLineTableSize &&
block_->is_line_free(hole_start_line_)) {
@@ -742,6 +754,17 @@ namespace immix {
return bytes;
}
+
+ bool allocated_address(Address addr) {
+ Chunks& chunks = block_allocator_.chunks();
+ for(Chunks::iterator i = chunks.begin();
+ i != chunks.end();
+ i++) {
+ if((*i)->contains_address(addr)) return true;
+ }
+
+ return false;
+ }
};
}

0 comments on commit 16f7f92

Please sign in to comment.