vm: fix fencepost error in write barrier on large object allocation; fixes benchmark.sort crash

Makefile

@@ -44,6 +44,7 @@ DLL_OBJS = $(PLAF_DLL_OBJS) \
 	vm/compaction.o \
 	vm/contexts.o \
 	vm/data_heap.o \
+	vm/data_heap_checker.o \
 	vm/debug.o \
 	vm/dispatch.o \
 	vm/errors.o \

vm/data_heap_checker.cpp

@@ -0,0 +1,101 @@
+#include "master.hpp"
+
+/* A tool to debug write barriers. Call check_data_heap() to ensure that all
+cards that should be marked are actually marked. */
+
+namespace factor
+{
+
+enum generation {
+	nursery_generation,
+	aging_generation,
+	tenured_generation
+};
+
+inline generation generation_of(factor_vm *parent, object *obj)
+{
+	if(parent->data->nursery->contains_p(obj))
+		return nursery_generation;
+	else if(parent->data->aging->contains_p(obj))
+		return aging_generation;
+	else if(parent->data->tenured->contains_p(obj))
+		return tenured_generation;
+	else
+	{
+		critical_error("Bad object",(cell)obj);
+		return (generation)-1;
+	}
+}
+
+struct slot_checker {
+	factor_vm *parent;
+	object *obj;
+	generation gen;
+
+	explicit slot_checker(factor_vm *parent_, object *obj_, generation gen_) :
+		parent(parent_), obj(obj_), gen(gen_) {}
+
+	void check_write_barrier(cell *slot_ptr, generation target, char mask)
+	{
+		cell object_card_pointer = parent->cards_offset + ((cell)obj >> card_bits);
+		cell slot_card_pointer = parent->cards_offset + ((cell)slot_ptr >> card_bits);
+		char slot_card_value = *(char *)slot_card_pointer;
+		if((slot_card_value & mask) != mask)
+		{
+			printf("card not marked\n");
+			printf("source generation: %d\n",gen);
+			printf("target generation: %d\n",target);
+			printf("object: 0x%lx\n",(cell)obj);
+			printf("object type: %ld\n",obj->type());
+			printf("slot pointer: 0x%lx\n",(cell)slot_ptr);
+			printf("slot value: 0x%lx\n",*slot_ptr);
+			printf("card of object: 0x%lx\n",object_card_pointer);
+			printf("card of slot: 0x%lx\n",slot_card_pointer);
+			printf("\n");
+			parent->factorbug();
+		}
+	}
+
+	void operator()(cell *slot_ptr)
+	{
+		if(!immediate_p(*slot_ptr))
+		{
+			generation target = generation_of(parent,untag<object>(*slot_ptr));
+			switch(gen)
+			{
+			case nursery_generation:
+				break;
+			case aging_generation:
+				if(target == nursery_generation)
+					check_write_barrier(slot_ptr,target,card_points_to_nursery);
+				break;
+			case tenured_generation:
+				if(target == nursery_generation)
+					check_write_barrier(slot_ptr,target,card_points_to_nursery);
+				else if(target == aging_generation)
+					check_write_barrier(slot_ptr,target,card_points_to_aging);
+				break;
+			}
+		}
+	}
+};
+
+struct object_checker {
+	factor_vm *parent;
+
+	explicit object_checker(factor_vm *parent_) : parent(parent_) {}
+
+	void operator()(object *obj)
+	{
+		slot_checker checker(parent,obj,generation_of(parent,obj));
+		obj->each_slot(checker);
+	}
+};
+
+void factor_vm::check_data_heap()
+{
+	object_checker checker(this);
+	each_object(checker);
+}
+
+}

vm/debug.cpp

@@ -288,7 +288,7 @@ struct data_reference_object_visitor {
 	void operator()(object *obj)
 	{
 		data_reference_slot_visitor visitor(look_for,obj,parent);
-		parent->do_slots(obj,visitor);
+		obj->each_slot(visitor);
 	}
 };
 

vm/image.cpp

@@ -138,7 +138,7 @@ void factor_vm::relocate_object(object *object,
 	cell type = object->type();
 	
 	/* Tuple relocation is a bit trickier; we have to fix up the
-	layout object before we can get the tuple size, so do_slots is
+	layout object before we can get the tuple size, so each_slot is
 	out of the question */
 	if(type == TUPLE_TYPE)
 	{
@@ -154,7 +154,7 @@ void factor_vm::relocate_object(object *object,
 	else
 	{
 		object_fixupper fixupper(this,data_relocation_base);
-		do_slots(object,fixupper);
+		object->each_slot(fixupper);
 
 		switch(type)
 		{

vm/layouts.hpp

@@ -102,7 +102,9 @@ struct object {
 	cell size() const;
 	cell binary_payload_start() const;
 
-	cell *slots()  const { return (cell *)this; }
+	cell *slots() const { return (cell *)this; }
+
+	template<typename Iterator> void each_slot(Iterator &iter);
 
 	/* Only valid for objects in tenured space; must cast to free_heap_block
 	to do anything with it if its free */

vm/objects.hpp

@@ -98,4 +98,19 @@ inline static bool save_env_p(cell i)
 	return (i >= OBJ_FIRST_SAVE && i <= OBJ_LAST_SAVE);
 }
 
+template<typename Iterator> void object::each_slot(Iterator &iter)
+{
+	cell scan = (cell)this;
+	cell payload_start = binary_payload_start();
+	cell end = scan + payload_start;
+
+	scan += sizeof(cell);
+
+	while(scan < end)
+	{
+		iter((cell *)scan);
+		scan += sizeof(cell);
+	}
+}
+
 }

vm/vm.hpp

@@ -262,11 +262,16 @@ struct factor_vm
 
 	inline void write_barrier(object *obj, cell size)
 	{
-		char *start = (char *)obj;
+		cell start = (cell)obj & -card_size;
-		for(cell offset = 0; offset < size; offset += card_size)
+		cell end = ((cell)obj + size + card_size - 1) & -card_size;
-			write_barrier((cell *)(start + offset));
+
+		for(cell offset = start; offset < end; offset += card_size)
+			write_barrier((cell *)offset);
 	}
 
+	// data heap checker
+	void check_data_heap();
+
 	// gc
 	void end_gc();
 	void start_gc_again();
@@ -585,24 +590,6 @@ struct factor_vm
 	void save_callstack_bottom(stack_frame *callstack_bottom);
 	template<typename Iterator> void iterate_callstack(context *ctx, Iterator &iterator);
 
-	/* Every object has a regular representation in the runtime, which makes GC
-	much simpler. Every slot of the object until binary_payload_start is a pointer
-	to some other object. */
-	template<typename Iterator> void do_slots(object *obj, Iterator &iter)
-	{
-		cell scan = (cell)obj;
-		cell payload_start = obj->binary_payload_start();
-		cell end = scan + payload_start;
-
-		scan += sizeof(cell);
-
-		while(scan < end)
-		{
-			iter((cell *)scan);
-			scan += sizeof(cell);
-		}
-	}
-
 	//alien
 	char *pinned_alien_offset(cell obj);
 	cell allot_alien(cell delegate_, cell displacement);