vm: stage code block map fixup properly for GC

Don't update the map until the very last thing, and pass untranslated addresses to the iterator functors. Somewhat complicated by the fact that, for startup_fixup, the map is initialized with fixed-up values, so the fixup functor needs a flag indicating whether it operates with a fixed or unfixed code heap map.

vm/callstack.hpp

@@ -21,7 +21,9 @@ void factor_vm::iterate_callstack_object_reversed(callstack *stack_,
 		void *frame_top = stack->frame_top_at(frame_offset);
 		void *addr = frame_return_address(frame_top);
 
-		void *fixed_addr = (void*)fixup.translate_code((code_block*)addr);
+		void *fixed_addr = Fixup::translated_code_block_map
+			? (void*)fixup.translate_code((code_block*)addr)
+			: addr;
 		code_block *owner = code->code_block_for_address((cell)fixed_addr);
 		cell frame_size = owner->stack_frame_size_for_address((cell)fixed_addr);
 
@@ -35,13 +37,13 @@ void factor_vm::iterate_callstack_object_reversed(callstack *stack_,
 		FACTOR_ASSERT(frame_size == frame->size);
 #endif
 
-		iterator(frame_top, owner, fixed_addr);
+		iterator(frame_top, frame_size, owner, fixed_addr);
 		frame_offset += frame_size;
 	}
 }
 
-template<typename Iterator> void factor_vm::iterate_callstack_object(callstack *stack_,
-	Iterator &iterator)
+template<typename Iterator>
+void factor_vm::iterate_callstack_object(callstack *stack_, Iterator &iterator)
 {
 	data_root<callstack> stack(stack_,this);
 	fixnum frame_offset = factor::untag_fixnum(stack->length) - sizeof(stack_frame);
@@ -66,23 +68,32 @@ void factor_vm::iterate_callstack_reversed(context *ctx, Iterator &iterator, Fix
 	{
 		void *addr = frame_return_address((void*)frame_top);
 		FACTOR_ASSERT(addr != 0);
-
-		void *fixed_addr = (void*)fixup.translate_code((code_block*)addr);
+		void *fixed_addr = Fixup::translated_code_block_map
+			? (void*)fixup.translate_code((code_block*)addr)
+			: addr;
 
 		code_block *owner = code->code_block_for_address((cell)fixed_addr);
-		cell frame_size = owner->stack_frame_size_for_address((cell)fixed_addr);
+		code_block *fixed_owner = Fixup::translated_code_block_map
+			? owner
+			: fixup.translate_code(owner);
+
+		cell frame_size = fixed_owner->stack_frame_size_for_address((cell)fixed_addr);
 
 #ifdef FACTOR_DEBUG
 		// check our derived owner and frame size against the ones stored in the frame
 		// by the function prolog
 		stack_frame *frame = (stack_frame*)(frame_top + frame_size) - 1;
-		void *fixed_entry_point =
-			(void*)fixup.translate_code((code_block*)frame->entry_point);
+		void *fixed_entry_point = Fixup::translated_code_block_map
+			? (void*)fixup.translate_code((code_block*)frame->entry_point)
+			: frame->entry_point;
 		FACTOR_ASSERT(owner->entry_point() == fixed_entry_point);
 		FACTOR_ASSERT(frame_size == frame->size);
 #endif
+		void *fixed_addr_for_iter = Fixup::translated_code_block_map
+			? fixed_addr
+			: addr;
 
-		iterator(frame_top, owner, fixed_addr);
+		iterator(frame_top, frame_size, owner, fixed_addr_for_iter);
 		frame_top += frame_size;
 	}
 }
@@ -111,7 +122,7 @@ void factor_vm::iterate_callstack_reversed(context *ctx, Iterator &iterator)
 		FACTOR_ASSERT(frame_size == frame->size);
 #endif
 
-		iterator(frame_top, owner, addr);
+		iterator(frame_top, frame_size, owner, addr);
 		frame_top += frame_size;
 	}
 }

vm/code_block_visitor.hpp

@@ -42,13 +42,16 @@ struct call_frame_code_block_visitor {
 	explicit call_frame_code_block_visitor(factor_vm *parent_, Fixup fixup_) :
 		parent(parent_), fixup(fixup_) {}
 
-	void operator()(void *frame_top, code_block *owner, void *addr)
+	void operator()(void *frame_top, cell frame_size, code_block *owner, void *addr)
 	{
-		set_frame_return_address(frame_top, addr);
+		code_block *compiled = Fixup::translated_code_block_map
+			? owner
+			: fixup.fixup_code(owner);
+		void *fixed_addr = compiled->address_for_offset(owner->offset(addr));
+		set_frame_return_address(frame_top, fixed_addr);
 		// XXX remove this when prolog data is removed
-		cell frame_size = owner->stack_frame_size_for_address((cell)addr);
 		stack_frame *frame = (stack_frame*)((char*)frame_top + frame_size) - 1;
-		frame->entry_point = owner->entry_point();
+		frame->entry_point = compiled->entry_point();
 	}
 };
 

vm/code_heap.cpp

@@ -105,6 +105,7 @@ struct all_blocks_set_verifier {
 	void operator()(code_block *block, cell size)
 	{
 		FACTOR_ASSERT(all_blocks->find((cell)block) != all_blocks->end());
+		// XXX check block size
 	}
 };
 
@@ -121,20 +122,9 @@ code_block *code_heap::code_block_for_address(cell address)
 	FACTOR_ASSERT(blocki != all_blocks.begin());
 	--blocki;
 	code_block* found_block = (code_block*)*blocki;
-#ifdef FACTOR_DEBUG
-	if (!((cell)found_block->entry_point() <= address
-		&& address - (cell)found_block->entry_point() < found_block->size()))
-	{
-		std::cerr << "invalid block found in all_blocks set!" << std::endl;
-		std::cerr << "address " << (void*)address
-			<< " block " << (void*)found_block
-			<< " entry point " << (void*)found_block->entry_point()
-			<< " size " << found_block->size()
-			<< " free? " << found_block->free_p();
-		verify_all_blocks_set();
-		FACTOR_ASSERT(false);
-	}
-#endif
+	FACTOR_ASSERT((cell)found_block->entry_point() <= address
+		/* XXX this isn't valid during fixup. should store the size in the map
+		&& address - (cell)found_block->entry_point() < found_block->size()*/);
 	return found_block;
 }
 
@@ -154,6 +144,9 @@ void code_heap::initialize_all_blocks_set()
 	all_blocks.clear();
 	all_blocks_set_inserter inserter(this);
 	allocator->iterate(inserter);
+#if defined(FACTOR_DEBUG)
+	verify_all_blocks_set();
+#endif
 }
 
 /* Allocate a code heap during startup */

vm/collector.hpp

@@ -4,6 +4,8 @@ namespace factor
 struct must_start_gc_again {};
 
 template<typename TargetGeneration, typename Policy> struct gc_workhorse : no_fixup {
+	static const bool translated_code_block_map = false;
+
 	factor_vm *parent;
 	TargetGeneration *target;
 	Policy policy;

vm/compaction.cpp

@@ -3,6 +3,8 @@
 namespace factor {
 
 struct compaction_fixup {
+	static const bool translated_code_block_map = false;
+
 	mark_bits<object> *data_forwarding_map;
 	mark_bits<code_block> *code_forwarding_map;
 	const object **data_finger;
@@ -192,6 +194,10 @@ void factor_vm::collect_compact_impl(bool trace_contexts_p)
 {
 	gc_event *event = current_gc->event;
 
+#if defined(FACTOR_DEBUG)
+	code->verify_all_blocks_set();
+#endif
+
 	if(event) event->started_compaction();
 
 	tenured_space *tenured = data->tenured;
@@ -224,8 +230,6 @@ void factor_vm::collect_compact_impl(bool trace_contexts_p)
 	code_block_compaction_updater<compaction_fixup> code_block_updater(this,fixup,data_forwarder,code_forwarder);
 	code->allocator->compact(code_block_updater,fixup,&code_finger);
 
-	code->update_all_blocks_set(code_forwarding_map);
-
 	data_forwarder.visit_roots();
 	if(trace_contexts_p)
 	{
@@ -242,6 +246,8 @@ void factor_vm::collect_compact_impl(bool trace_contexts_p)
 }
 
 struct code_compaction_fixup {
+	static const bool translated_code_block_map = false;
+
 	mark_bits<code_block> *code_forwarding_map;
 	const code_block **code_finger;
 

vm/debug.cpp

@@ -220,10 +220,10 @@ struct stack_frame_printer {
 	factor_vm *parent;
 
 	explicit stack_frame_printer(factor_vm *parent_) : parent(parent_) {}
-	void operator()(void *frame_top, code_block *owner, void *addr)
+	void operator()(void *frame_top, cell frame_size, code_block *owner, void *addr)
 	{
 		std::cout << std::endl;
-		std::cout << "frame: " << frame_top << std::endl;
+		std::cout << "frame: " << frame_top << " size " << frame_size << std::endl;
 		std::cout << "executing: ";
 		parent->print_obj(owner->owner);
 		std::cout << std::endl;

vm/fixup.hpp

@@ -10,6 +10,8 @@ struct identity {
 };
 
 struct no_fixup {
+	static const bool translated_code_block_map = false;
+
 	object *fixup_data(object *obj)
 	{
 		return obj;

vm/image.cpp

@@ -57,6 +57,8 @@ void factor_vm::load_code_heap(FILE *file, image_header *h, vm_parameters *p)
 }
 
 struct startup_fixup {
+	static const bool translated_code_block_map = true;
+
 	cell data_offset;
 	cell code_offset;
 

vm/mark_bits.hpp

@@ -126,8 +126,7 @@ template<typename Block> struct mark_bits {
 	Block *forward_block(const Block *original)
 	{
 #ifdef FACTOR_DEBUG
-		// XXX this condition seems to be harmless--ask slava
-		//FACTOR_ASSERT(marked_p(original));
+		FACTOR_ASSERT(marked_p(original));
 #endif
 		std::pair<cell,cell> position = bitmap_deref(original);
 		cell offset = (cell)original & (data_alignment - 1);

vm/slot_visitor.hpp

@@ -306,19 +306,22 @@ struct call_frame_slot_visitor {
 	             [entry_point]
 	             [size]
 	*/
-	void operator()(void *frame_top, code_block *owner, void *addr)
+	void operator()(void *frame_top, cell frame_size, code_block *owner, void *addr)
 	{
 		cell return_address = owner->offset(addr);
 
-		gc_info *info = owner->block_gc_info();
+		code_block *compiled = Fixup::translated_code_block_map
+			? owner
+			: visitor->fixup.translate_code(owner);
+		gc_info *info = compiled->block_gc_info();
 
-		FACTOR_ASSERT(return_address < owner->size());
+		FACTOR_ASSERT(return_address < compiled->size());
 		cell callsite = info->return_address_index(return_address);
 		if(callsite == (cell)-1)
 			return;
 
 #ifdef DEBUG_GC_MAPS
-		std::cout << "call frame code block " << owner << " with offset " << return_address << std::endl;
+		std::cout << "call frame code block " << compiled << " with offset " << return_address << std::endl;
 #endif
 		cell *stack_pointer = (cell *)frame_top;
 		u8 *bitmap = info->gc_info_bitmap();