From e8b1a06dad261b28ff57077f9ee9a547c61bfac2 Mon Sep 17 00:00:00 2001
From: Slava Pestov <slava@slava-pestovs-macbook-pro.local>
Date: Fri, 17 Sep 2010 19:20:40 -0700
Subject: [PATCH] vm: a stack underflow inside a primitive could leave a data
 root in an uninitialized state; if allocating the thrown underflow error
 triggered a GC, the GC would visit a garbage pointer

---
 vm/callstack.cpp |  5 -----
 vm/errors.cpp    | 46 +++++++++++++++++++++++++---------------------
 vm/vm.hpp        |  1 -
 3 files changed, 25 insertions(+), 27 deletions(-)

diff --git a/vm/callstack.cpp b/vm/callstack.cpp
index d5155d2e6e..5bec7f17cf 100755
--- a/vm/callstack.cpp
+++ b/vm/callstack.cpp
@@ -137,11 +137,6 @@ void factor_vm::scrub_return_address()
 		frame = frame_successor(frame);
 
 	set_frame_offset(frame,0);
-
-#ifdef FACTOR_DEBUG
-	/* Doing a GC here triggers all kinds of funny errors */
-	primitive_compact_gc();
-#endif
 }
 
 cell factor_vm::frame_scan(stack_frame *frame)
diff --git a/vm/errors.cpp b/vm/errors.cpp
index 6bd34b8442..7c4b043423 100755
--- a/vm/errors.cpp
+++ b/vm/errors.cpp
@@ -27,25 +27,35 @@ void out_of_memory()
 	exit(1);
 }
 
-void factor_vm::throw_error(cell error)
+void factor_vm::general_error(vm_error_type error, cell arg1, cell arg2)
 {
+	/* Reset local roots before allocating anything */
+	data_roots.clear();
+	bignum_roots.clear();
+	code_roots.clear();
+
+	/* If we had an underflow or overflow, data or retain stack
+	pointers might be out of bounds, so fix them before allocating
+	anything */
+	ctx->fix_stacks();
+
+	/* If error was thrown during heap scan, we re-enable the GC */
+	gc_off = false;
+
 	/* If the error handler is set, we rewind any C stack frames and
 	pass the error to user-space. */
 	if(!current_gc && to_boolean(special_objects[ERROR_HANDLER_QUOT]))
 	{
-		/* If error was thrown during heap scan, we re-enable the GC */
-		gc_off = false;
+#ifdef FACTOR_DEBUG
+		/* Doing a GC here triggers all kinds of funny errors */
+		primitive_compact_gc();
+#endif
 
-		/* Reset local roots */
-		data_roots.clear();
-		bignum_roots.clear();
-		code_roots.clear();
+		/* Now its safe to allocate and GC */
+		cell error_object = allot_array_4(special_objects[OBJ_ERROR],
+			tag_fixnum(error),arg1,arg2);
 
-		/* If we had an underflow or overflow, data or retain stack
-		pointers might be out of bounds */
-		ctx->fix_stacks();
-
-		ctx->push(error);
+		ctx->push(error_object);
 
 		unwind_native_frames(special_objects[ERROR_HANDLER_QUOT],
 			ctx->callstack_top);
@@ -55,19 +65,13 @@ void factor_vm::throw_error(cell error)
 	else
 	{
 		std::cout << "You have triggered a bug in Factor. Please report.\n";
-		std::cout << "early_error: ";
-		print_obj(error);
-		std::cout << std::endl;
+		std::cout << "error: " << error << std::endl;
+		std::cout << "arg 1: "; print_obj(arg1); std::cout << std::endl;
+		std::cout << "arg 2: "; print_obj(arg2); std::cout << std::endl;
 		factorbug();
 	}
 }
 
-void factor_vm::general_error(vm_error_type error, cell arg1, cell arg2)
-{
-	throw_error(allot_array_4(special_objects[OBJ_ERROR],
-		tag_fixnum(error),arg1,arg2));
-}
-
 void factor_vm::type_error(cell type, cell tagged)
 {
 	general_error(ERROR_TYPE,tag_fixnum(type),tagged);
diff --git a/vm/vm.hpp b/vm/vm.hpp
index 44ac81a70c..2cbedbcc7b 100755
--- a/vm/vm.hpp
+++ b/vm/vm.hpp
@@ -167,7 +167,6 @@ struct factor_vm
 	void primitive_profiling();
 
 	// errors
-	void throw_error(cell error);
 	void general_error(vm_error_type error, cell arg1, cell arg2);
 	void type_error(cell type, cell tagged);
 	void not_implemented_error();