vm: more defense against multi-faulting

* Clear faulting_p from a safepoint rather than inside general_error, because jumping into unwind-native-frames could blow up.
* Handle multiple faults from fatal_error by breakpointing. Is there anything else we can safely do at that point?
* Verify memory protection faults in the top half of the signal handlers because signal dispatch could fault. Treat memory faults during gc or fep as fatal errors.
* Add a function factor_vm::abort() that restores the default SIGABRT handler and ::abort()s. Use it from fatal_error() so we get useful context from gdb and so the user gets feedback from the system crash reporter that Factor blew up and didn't just disappear.
* In factorbug(), don't proceed with .s .r .c if it would be unsafe to do so.
* Don't pile on signals if we've already called fatal_error().

basis/cpu/x86/32/bootstrap.factor

@@ -140,12 +140,17 @@ IN: bootstrap.x86
 [ jit-jump-quot ]
 \ (call) define-combinator-primitive
 
+: (jit-safepoint) ( -- )
+    0 EAX MOVABS rc-absolute rel-safepoint ;
 [
     ! Load ds and rs registers
     jit-load-vm
     jit-load-context
     jit-restore-context
 
+    ! Safepoint to clear the faulting flag in the VM
+     (jit-safepoint)
+
     ! Windows-specific setup
     ctx-reg jit-update-seh
 
@@ -395,9 +400,7 @@ IN: bootstrap.x86
     EAX EDX [] MOV
     jit-jump-quot ;
 
-[
+[ (jit-safepoint) ] \ jit-safepoint jit-define
-    0 EAX MOVABS rc-absolute rel-safepoint
-] \ jit-safepoint jit-define
 
 [
     jit-start-context-and-delete

basis/cpu/x86/64/bootstrap.factor

@@ -122,6 +122,9 @@ IN: bootstrap.x86
 [ jit-jump-quot ]
 \ (call) define-combinator-primitive
 
+: (jit-safepoint)
+    0 [RIP+] EAX MOV rc-relative rel-safepoint ;
+
 [
     ! Unwind stack frames
     RSP arg2 MOV
@@ -134,6 +137,9 @@ IN: bootstrap.x86
     jit-load-context
     jit-restore-context
 
+    ! Safepoint to clear the faulting flag in the VM
+    (jit-safepoint)
+
     ! Call quotation
     jit-jump-quot
 ] \ unwind-native-frames define-sub-primitive
@@ -336,9 +342,7 @@ IN: bootstrap.x86
     jit-push-param
     jit-jump-quot ;
 
-[
+[ (jit-safepoint) ] \ jit-safepoint jit-define
-    0 [RIP+] EAX MOV rc-relative rel-safepoint
-] \ jit-safepoint jit-define
 
 [
     jit-start-context-and-delete

core/kernel/kernel-tests.factor

@@ -2,16 +2,16 @@ USING: arrays byte-arrays kernel kernel.private math memory
 namespaces sequences tools.test math.private quotations
 continuations prettyprint io.streams.string debugger assocs
 sequences.private accessors locals.backend grouping words
-system ;
+system alien alien.accessors ;
 IN: kernel.tests
 
 [ 0 ] [ f size ] unit-test
 [ t ] [ [ \ = \ = ] all-equal? ] unit-test
 
 ! Don't leak extra roots if error is thrown
-[ ] [ 10000 [ [ 3 throw ] ignore-errors ] times ] unit-test
+[ ] [ 1000 [ [ 3 throw ] ignore-errors ] times ] unit-test
 
-[ ] [ 10000 [ [ -1 f <array> ] ignore-errors ] times ] unit-test
+[ ] [ 1000 [ [ -1 f <array> ] ignore-errors ] times ] unit-test
 
 ! Make sure we report the correct error on stack underflow
 [ clear drop ] [ { "kernel-error" 10 f f } = ] must-fail-with
@@ -183,3 +183,7 @@ os windows? [
 [ t ] [ { } identity-hashcode fixnum? ] unit-test
 [ 123 ] [ 123 identity-hashcode ] unit-test
 [ t ] [ f identity-hashcode fixnum? ] unit-test
+
+! Make sure memory protection faults work
+[ f 0 alien-unsigned-1 ] [ vm-error? ] must-fail-with
+[ 1 <alien> 0 alien-unsigned-1 ] [ vm-error? ] must-fail-with

vm/debug.cpp

@@ -201,13 +201,19 @@ void factor_vm::print_objects(cell *start, cell *end)
 void factor_vm::print_datastack()
 {
 	std::cout << "==== DATA STACK:" << std::endl;
+	if (ctx)
 		print_objects((cell *)ctx->datastack_seg->start,(cell *)ctx->datastack);
+	else
+		std::cout << "*** Context not initialized" << std::endl;
 }
 
 void factor_vm::print_retainstack()
 {
 	std::cout << "==== RETAIN STACK:" << std::endl;
+	if (ctx)
 		print_objects((cell *)ctx->retainstack_seg->start,(cell *)ctx->retainstack);
+	else
+		std::cout << "*** Context not initialized" << std::endl;
 }
 
 struct stack_frame_printer {
@@ -238,9 +244,14 @@ struct stack_frame_printer {
 void factor_vm::print_callstack()
 {
 	std::cout << "==== CALL STACK:" << std::endl;
+	if (ctx)
+	{
 		stack_frame_printer printer(this);
 		iterate_callstack(ctx,printer);
 	}
+	else
+		std::cout << "*** Context not initialized" << std::endl;
+}
 
 struct padded_address {
 	cell value;
@@ -450,6 +461,8 @@ void factor_vm::factorbug_usage(bool advanced_p)
 		std::cout << "  push <addr>      -- push object on data stack - NOT SAFE" << std::endl;
 		std::cout << "  gc               -- trigger full GC - NOT SAFE" << std::endl;
 		std::cout << "  code             -- code heap dump" << std::endl;
+		std::cout << "  abort            -- call abort()" << std::endl;
+		std::cout << "  breakpoint       -- trigger system breakpoint" << std::endl;
 	}
 	else
 	{
@@ -599,6 +612,10 @@ void factor_vm::factorbug()
 			primitive_full_gc();
 		else if(strcmp(cmd,"help") == 0)
 			factorbug_usage(true);
+		else if(strcmp(cmd,"abort") == 0)
+			abort();
+		else if(strcmp(cmd,"breakpoint") == 0)
+			breakpoint();
 		else
 			std::cout << "unknown command" << std::endl;
 	}

vm/errors.cpp

@@ -3,12 +3,26 @@
 namespace factor
 {
 
+bool factor_vm::fatal_erroring_p;
+
+static inline void fa_diddly_atal_error()
+{
+	printf("fatal_error in fatal_error!\n");
+	breakpoint();
+	exit(86);
+}
+
 void fatal_error(const char *msg, cell tagged)
 {
+	if (factor_vm::fatal_erroring_p)
+		fa_diddly_atal_error();
+
+	factor_vm::fatal_erroring_p = true;
+
 	std::cout << "fatal_error: " << msg;
-	std::cout << ": " << std::hex << tagged << std::dec;
+	std::cout << ": " << (void*)tagged;
 	std::cout << std::endl;
-	exit(1);
+	abort();
 }
 
 void critical_error(const char *msg, cell tagged)
@@ -59,7 +73,9 @@ void factor_vm::general_error(vm_error_type error, cell arg1, cell arg2)
 
 		ctx->push(error_object);
 
-		faulting_p = false;
+		/* Guard the safepoint, which will clear faulting_p if unwind-native-frames
+		succeeds */
+		code->guard_safepoint();
 		unwind_native_frames(special_objects[ERROR_HANDLER_QUOT],
 			ctx->callstack_top);
 	}
@@ -71,8 +87,8 @@ void factor_vm::general_error(vm_error_type error, cell arg1, cell arg2)
 		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;
-		faulting_p = false;
 		factorbug();
+		abort();
 	}
 }
 
@@ -86,12 +102,24 @@ void factor_vm::not_implemented_error()
 	general_error(ERROR_NOT_IMPLEMENTED,false_object,false_object);
 }
 
+void factor_vm::verify_memory_protection_error(cell addr)
+{
+	/* Called from the OS-specific top halves of the signal handlers to
+	make sure it's safe to dispatch to memory_protection_error */
+	if(fatal_erroring_p)
+		fa_diddly_atal_error();
+	if(faulting_p && !code->safepoint_p(addr))
+		fatal_error("Double fault", addr);
+	else if(fep_p)
+		fatal_error("Memory protection fault during low-level debugger", addr);
+	else if(atomic::load(&current_gc_p))
+		fatal_error("Memory protection fault during gc", addr);
+}
+
 void factor_vm::memory_protection_error(cell addr)
 {
 	if(code->safepoint_p(addr))
 		safepoint.handle_safepoint(this);
-	else if(faulting_p)
-		fatal_error("Double fault", 0);
 	else if(ctx->datastack_seg->underflow_p(addr))
 		general_error(ERROR_DATASTACK_UNDERFLOW,false_object,false_object);
 	else if(ctx->datastack_seg->overflow_p(addr))

vm/mach_signal.cpp

@@ -40,6 +40,7 @@ void factor_vm::call_fault_handler(
 	if(exception == EXC_BAD_ACCESS)
 	{
 		signal_fault_addr = MACH_EXC_STATE_FAULT(exc_state);
+		verify_memory_protection_error(signal_fault_addr);
 		handler = (cell)factor::memory_signal_handler_impl;
 	}
 	else if(exception == EXC_ARITHMETIC && code != MACH_EXC_INTEGER_DIV)

vm/os-unix.cpp

@@ -165,18 +165,23 @@ void factor_vm::end_sampling_profiler_timer()
 void memory_signal_handler(int signal, siginfo_t *siginfo, void *uap)
 {
 	factor_vm *vm = current_vm();
+	vm->verify_memory_protection_error((cell)siginfo->si_addr);
 	vm->signal_fault_addr = (cell)siginfo->si_addr;
 	vm->dispatch_signal(uap,factor::memory_signal_handler_impl);
 }
 
 void synchronous_signal_handler(int signal, siginfo_t *siginfo, void *uap)
 {
+	if (factor_vm::fatal_erroring_p)
+		return;
+
 	factor_vm *vm = current_vm_p();
 	if (vm)
 	{
 		vm->signal_number = signal;
 		vm->dispatch_signal(uap,factor::synchronous_signal_handler_impl);
-	} else
+	}
+	else
 		fatal_error("Foreign thread received signal", signal);
 }
 
@@ -190,6 +195,9 @@ static void enqueue_signal(factor_vm *vm, int signal)
 
 void enqueue_signal_handler(int signal, siginfo_t *siginfo, void *uap)
 {
+	if (factor_vm::fatal_erroring_p)
+		return;
+
 	factor_vm *vm = current_vm_p();
 	if (vm)
 		enqueue_signal(vm, signal);
@@ -199,6 +207,9 @@ void enqueue_signal_handler(int signal, siginfo_t *siginfo, void *uap)
 
 void fep_signal_handler(int signal, siginfo_t *siginfo, void *uap)
 {
+	if (factor_vm::fatal_erroring_p)
+		return;
+
 	factor_vm *vm = current_vm_p();
 	if (vm)
 	{
@@ -251,7 +262,7 @@ static void sigaction_safe(int signum, const struct sigaction *act, struct sigac
 	while(ret == -1 && errno == EINTR);
 
 	if(ret == -1)
-		fatal_error("sigaction failed", 0);
+		fatal_error("sigaction failed", errno);
 }
 
 static void init_sigaction_with_handler(struct sigaction *act,
@@ -499,4 +510,16 @@ void factor_vm::unlock_console()
 	pthread_mutex_unlock(&stdin_mutex);
 }
 
+void factor_vm::abort()
+{
+	sig_t ret;
+	do
+	{
+		ret = signal(SIGABRT, SIG_DFL);
+	}
+	while(ret == SIG_ERR && errno == EINTR);
+	
+	::abort();
+}
+
 }

vm/os-unix.hpp

@@ -45,4 +45,9 @@ void sleep_nanos(u64 nsec);
 
 void move_file(const vm_char *path1, const vm_char *path2);
 
+static inline void breakpoint()
+{
+    __builtin_trap();
+}
+
 }

vm/os-windows.cpp

@@ -214,12 +214,21 @@ void sleep_nanos(u64 nsec)
 	Sleep((DWORD)(nsec/1000000));
 }
 
+typedef enum _EXCEPTION_DISPOSITION
+{
+	ExceptionContinueExecution = 0,
+	ExceptionContinueSearch = 1,
+	ExceptionNestedException = 2,
+	ExceptionCollidedUnwind = 3
+} EXCEPTION_DISPOSITION;
+
 LONG factor_vm::exception_handler(PEXCEPTION_RECORD e, void *frame, PCONTEXT c, void *dispatch)
 {
 	switch (e->ExceptionCode)
 	{
 	case EXCEPTION_ACCESS_VIOLATION:
 		signal_fault_addr = e->ExceptionInformation[1];
+		verify_memory_protection_error(signal_fault_addr);
 		dispatch_signal_handler(
 			(cell*)&c->ESP,
 			(cell*)&c->EIP,
@@ -261,19 +270,19 @@ LONG factor_vm::exception_handler(PEXCEPTION_RECORD e, void *frame, PCONTEXT c,
 		break;
 	}
 
-	return 0;
+	return ExceptionContinueExecution;
 }
 
 VM_C_API LONG exception_handler(PEXCEPTION_RECORD e, void *frame, PCONTEXT c, void *dispatch)
 {
+	if (factor_vm::fatal_erroring_p)
+		return ExceptionContinueSearch;
+
 	factor_vm *vm = current_vm_p();
 	if (vm)
 		return vm->exception_handler(e,frame,c,dispatch);
 	else
-	{
+		return ExceptionContinueSearch;
-		fatal_error("Foreign thread received exception", e->ExceptionCode);
-		return 0; // to placate MSVC
-	}
 }
 
 static BOOL WINAPI ctrl_handler(DWORD dwCtrlType)
@@ -380,4 +389,9 @@ void factor_vm::end_sampling_profiler_timer()
 	sampler_thread = NULL;
 }
 
+void factor_vm::abort()
+{
+	::abort();
+}
+
 }

vm/os-windows.hpp

@@ -86,6 +86,11 @@ inline static THREADHANDLE thread_id()
 	return threadHandle;
 }
 
+inline static breakpoint()
+{
+	DebugBreak();
+}
+
 #define CODE_TO_FUNCTION_POINTER(code) (void)0
 #define CODE_TO_FUNCTION_POINTER_CALLBACK(vm, code) (void)0
 #define FUNCTION_CODE_POINTER(ptr) ptr

vm/safepoints.cpp

@@ -38,6 +38,7 @@ void safepoint_state::enqueue_samples(factor_vm *parent, cell samples, cell pc,
 void safepoint_state::handle_safepoint(factor_vm *parent) volatile
 {
 	parent->code->unguard_safepoint();
+	parent->faulting_p = false;
 
 	if (atomic::load(&fep_p))
 	{

vm/vm.hpp

@@ -143,6 +143,7 @@ struct factor_vm
 
 	/* Are we already handling a fault? Used to catch double memory faults */
 	bool faulting_p;
+	static bool fatal_erroring_p;
 
 	/* Safepoint state */
 	volatile safepoint_state safepoint;
@@ -213,6 +214,7 @@ struct factor_vm
 	void general_error(vm_error_type error, cell arg1, cell arg2);
 	void type_error(cell type, cell tagged);
 	void not_implemented_error();
+	void verify_memory_protection_error(cell addr);
 	void memory_protection_error(cell addr);
 	void signal_error(cell signal);
 	void divide_by_zero_error();
@@ -730,6 +732,7 @@ struct factor_vm
 	void open_console();
 	void lock_console();
 	void unlock_console();
+	static void abort();
 
 	// os-windows
   #if defined(WINDOWS)