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().

diff --git a/basis/cpu/x86/32/bootstrap.factor b/basis/cpu/x86/32/bootstrap.factor
index 96eef9864dd51ca6ccb4c9bae44dcd571365d1fe..c234d03a441f653be307c3d026f27d87eb19fc3f 100755 (executable)
--- a/basis/cpu/x86/32/bootstrap.factor
+++ b/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 ;
 
-[
-    0 EAX MOVABS rc-absolute rel-safepoint
-] \ jit-safepoint jit-define
+[ (jit-safepoint) ] \ jit-safepoint jit-define
 
 [
     jit-start-context-and-delete

diff --git a/basis/cpu/x86/64/bootstrap.factor b/basis/cpu/x86/64/bootstrap.factor
index a6919e4560735b42e49a8c44f7c3645b1751ea84..77e76db9b7c1623b934a9c4180f8f03bbcf8a200 100755 (executable)
--- a/basis/cpu/x86/64/bootstrap.factor
+++ b/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 ;
 
-[
-    0 [RIP+] EAX MOV rc-relative rel-safepoint
-] \ jit-safepoint jit-define
+[ (jit-safepoint) ] \ jit-safepoint jit-define
 
 [
     jit-start-context-and-delete

diff --git a/core/kernel/kernel-tests.factor b/core/kernel/kernel-tests.factor
index 3432026e9214a70a8985ced56f0d87901f846300..5aa690788eb229d7431562eaafb4d3eaa342a35e 100644 (file)
--- a/core/kernel/kernel-tests.factor
+++ b/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

diff --git a/vm/debug.cpp b/vm/debug.cpp
index 1d057ef65145c1ae838ba52625be20f206c3eb99..3698fb1dc8621955160d1799eedc0220041c69e6 100755 (executable)
--- a/vm/debug.cpp
+++ b/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;
-       print_objects((cell *)ctx->datastack_seg->start,(cell *)ctx->datastack);
+       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;
-       print_objects((cell *)ctx->retainstack_seg->start,(cell *)ctx->retainstack);
+       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,8 +244,13 @@ struct stack_frame_printer {
 void factor_vm::print_callstack()
 {
        std::cout << "==== CALL STACK:" << std::endl;
-       stack_frame_printer printer(this);
-       iterate_callstack(ctx,printer);
+       if (ctx)
+       {
+               stack_frame_printer printer(this);
+               iterate_callstack(ctx,printer);
+       }
+       else
+               std::cout << "*** Context not initialized" << std::endl;
 }
 
 struct padded_address {
@@ -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;
        }

diff --git a/vm/errors.cpp b/vm/errors.cpp
index 5f3188abcb14526b88848046025a47c91381891a..14a8700dcff6172b912a8fa189cd77dd24fc29d0 100755 (executable)
--- a/vm/errors.cpp
+++ b/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))

diff --git a/vm/mach_signal.cpp b/vm/mach_signal.cpp
index d393aa307b2c4eed424164c388d58d4fc5130c9b..52148676c5d0935730e5ae0f03b96ae25c64b7c9 100755 (executable)
--- a/vm/mach_signal.cpp
+++ b/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)

diff --git a/vm/os-unix.cpp b/vm/os-unix.cpp
index 085745ce37e693eccb0512d7be267074c71bfd04..8c6f05a920aca07b3544742845cbb405e7d7aa75 100755 (executable)
--- a/vm/os-unix.cpp
+++ b/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();
+}
+
 }

diff --git a/vm/os-unix.hpp b/vm/os-unix.hpp
index c0cce24a06765a42ca32bc3a903385fe5f50f256..e8008bf5eb8c396eb2ebdafe1f3b78f190c4d19f 100644 (file)
--- a/vm/os-unix.hpp
+++ b/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();
+}
+
 }

diff --git a/vm/os-windows.cpp b/vm/os-windows.cpp
index 42a9dc4da7b4da7cecac32b3b66cc956d1c7c6d3..9940850d3789536d35d89d4bcaae1af13a25d9ea 100755 (executable)
--- a/vm/os-windows.cpp
+++ b/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
-       {
-               fatal_error("Foreign thread received exception", e->ExceptionCode);
-               return 0; // to placate MSVC
-       }
+               return ExceptionContinueSearch;
 }
 
 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();
+}
+
 }

diff --git a/vm/os-windows.hpp b/vm/os-windows.hpp
index ba820d32e9b7375a206513dec0f6faa42f5b1cf6..d382fdcb936fdb461bfe09e143760faf4f576b2d 100755 (executable)
--- a/vm/os-windows.hpp
+++ b/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

diff --git a/vm/safepoints.cpp b/vm/safepoints.cpp
index 83621e477fd1d55fc71a557079502662f66a3cf1..2353157cab88b02c3d9293750ae788ea9c866e50 100644 (file)
--- a/vm/safepoints.cpp
+++ b/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))
        {

diff --git a/vm/vm.hpp b/vm/vm.hpp
index 61ab74ec6a21173a0b2e9770a6992266a6cee5f7..5cd8b1f05c2308f4610517bf75f303238c7616e7 100755 (executable)
--- a/vm/vm.hpp
+++ b/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)