From beb0f48da9c23e5113f215a99ac8b5a26691e7a1 Mon Sep 17 00:00:00 2001 From: Joe Groff Date: Fri, 11 Nov 2011 13:29:46 -0800 Subject: [PATCH] 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 | 9 ++++--- basis/cpu/x86/64/bootstrap.factor | 10 +++++--- core/kernel/kernel-tests.factor | 10 +++++--- vm/debug.cpp | 25 +++++++++++++++---- vm/errors.cpp | 40 ++++++++++++++++++++++++++----- vm/mach_signal.cpp | 1 + vm/os-unix.cpp | 27 +++++++++++++++++++-- vm/os-unix.hpp | 5 ++++ vm/os-windows.cpp | 24 +++++++++++++++---- vm/os-windows.hpp | 5 ++++ vm/safepoints.cpp | 1 + vm/vm.hpp | 3 +++ 12 files changed, 134 insertions(+), 26 deletions(-) diff --git a/basis/cpu/x86/32/bootstrap.factor b/basis/cpu/x86/32/bootstrap.factor index 96eef9864d..c234d03a44 100755 --- 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 a6919e4560..77e76db9b7 100755 --- 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 3432026e92..5aa690788e 100644 --- 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 ] ignore-errors ] times ] unit-test +[ ] [ 1000 [ [ -1 f ] 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 0 alien-unsigned-1 ] [ vm-error? ] must-fail-with diff --git a/vm/debug.cpp b/vm/debug.cpp index 1d057ef651..3698fb1dc8 100755 --- 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 -- 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 5f3188abcb..14a8700dcf 100755 --- 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(¤t_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 d393aa307b..52148676c5 100755 --- 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 085745ce37..8c6f05a920 100755 --- 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 c0cce24a06..e8008bf5eb 100644 --- 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 42a9dc4da7..9940850d37 100755 --- 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 ba820d32e9..d382fdcb93 100755 --- 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 83621e477f..2353157cab 100644 --- 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 61ab74ec6a..5cd8b1f05c 100755 --- 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)