From 11ddbc03a4d56763d8eec123ddce61083db1a661 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Sat, 27 Mar 2010 07:33:28 -0400 Subject: [PATCH] vm: signal handling cleanup --- vm/alien.cpp | 2 +- vm/callstack.cpp | 12 +++++-- vm/code_blocks.cpp | 2 +- vm/contexts.cpp | 15 ++++++-- vm/contexts.hpp | 16 ++------- vm/errors.cpp | 76 +++++++++++++++++------------------------ vm/errors.hpp | 8 ++--- vm/io.cpp | 2 +- vm/mach_signal.cpp | 14 ++------ vm/math.cpp | 2 +- vm/os-genunix.hpp | 5 --- vm/os-macosx-ppc.hpp | 5 --- vm/os-macosx-x86.32.hpp | 5 --- vm/os-macosx-x86.64.hpp | 5 --- vm/os-unix.cpp | 15 ++------ vm/os-windows-nt.cpp | 2 ++ vm/os-windows.cpp | 2 +- vm/segments.hpp | 10 ++++++ vm/vm.hpp | 16 ++++----- 19 files changed, 90 insertions(+), 124 deletions(-) diff --git a/vm/alien.cpp b/vm/alien.cpp index 44365859e2..da70fa134e 100755 --- a/vm/alien.cpp +++ b/vm/alien.cpp @@ -13,7 +13,7 @@ char *factor_vm::pinned_alien_offset(cell obj) { alien *ptr = untag(obj); if(to_boolean(ptr->expired)) - general_error(ERROR_EXPIRED,obj,false_object,NULL); + general_error(ERROR_EXPIRED,obj,false_object); if(to_boolean(ptr->base)) type_error(ALIEN_TYPE,obj); else diff --git a/vm/callstack.cpp b/vm/callstack.cpp index 8389ff8d90..7268d6ab91 100755 --- a/vm/callstack.cpp +++ b/vm/callstack.cpp @@ -18,11 +18,17 @@ callstack *factor_vm::allot_callstack(cell size) return stack; } -stack_frame *factor_vm::fix_callstack_top(stack_frame *top, stack_frame *bottom) +/* If 'stack' points into the middle of the frame, find the nearest valid stack +pointer where we can resume execution and hope to capture the call trace without +crashing. Also, make sure we have at least 'stack_reserved' bytes available so +that we don't run out of callstack space while handling the error. */ +stack_frame *factor_vm::fix_callstack_top(stack_frame *stack) { - stack_frame *frame = bottom - 1; + stack_frame *frame = ctx->callstack_bottom - 1; - while(frame >= top) + while(frame >= stack + && frame >= ctx->callstack_top + && (cell)frame >= ctx->callstack_seg->start + stack_reserved) frame = frame_successor(frame); return frame + 1; diff --git a/vm/code_blocks.cpp b/vm/code_blocks.cpp index 4741a68c54..894e49846d 100755 --- a/vm/code_blocks.cpp +++ b/vm/code_blocks.cpp @@ -144,7 +144,7 @@ void factor_vm::update_word_references(code_block *compiled, bool reset_inline_c image load */ void factor_vm::undefined_symbol() { - general_error(ERROR_UNDEFINED_SYMBOL,false_object,false_object,NULL); + general_error(ERROR_UNDEFINED_SYMBOL,false_object,false_object); } void undefined_symbol() diff --git a/vm/contexts.cpp b/vm/contexts.cpp index f21d9c948d..8734ff8486 100644 --- a/vm/contexts.cpp +++ b/vm/contexts.cpp @@ -44,6 +44,17 @@ void context::reset() reset_context_objects(); } +void context::fix_stacks() +{ + if(datastack + sizeof(cell) < datastack_seg->start + || datastack + stack_reserved >= datastack_seg->end) + reset_datastack(); + + if(retainstack + sizeof(cell) < retainstack_seg->start + || retainstack + stack_reserved >= retainstack_seg->end) + reset_retainstack(); +} + context::~context() { delete datastack_seg; @@ -167,13 +178,13 @@ bool factor_vm::stack_to_array(cell bottom, cell top) void factor_vm::primitive_datastack() { if(!stack_to_array(ctx->datastack_seg->start,ctx->datastack)) - general_error(ERROR_DS_UNDERFLOW,false_object,false_object,NULL); + general_error(ERROR_DATASTACK_UNDERFLOW,false_object,false_object); } void factor_vm::primitive_retainstack() { if(!stack_to_array(ctx->retainstack_seg->start,ctx->retainstack)) - general_error(ERROR_RS_UNDERFLOW,false_object,false_object,NULL); + general_error(ERROR_RETAINSTACK_UNDERFLOW,false_object,false_object); } /* returns pointer to top of stack */ diff --git a/vm/contexts.hpp b/vm/contexts.hpp index 3adabd9e5d..441b5916c8 100644 --- a/vm/contexts.hpp +++ b/vm/contexts.hpp @@ -8,6 +8,8 @@ enum context_object { OBJ_CATCHSTACK, }; +static const cell stack_reserved = 1024; + struct context { // First 4 fields accessed directly by compiler. See basis/vm/vm.factor @@ -41,6 +43,7 @@ struct context { void reset_callstack(); void reset_context_objects(); void reset(); + void fix_stacks(); cell peek() { @@ -64,19 +67,6 @@ struct context { datastack += sizeof(cell); replace(tagged); } - - static const cell stack_reserved = (64 * sizeof(cell)); - - void fix_stacks() - { - if(datastack + sizeof(cell) < datastack_seg->start - || datastack + stack_reserved >= datastack_seg->end) - reset_datastack(); - - if(retainstack + sizeof(cell) < retainstack_seg->start - || retainstack + stack_reserved >= retainstack_seg->end) - reset_retainstack(); - } }; VM_C_API context *new_context(factor_vm *parent); diff --git a/vm/errors.cpp b/vm/errors.cpp index 37a9452744..d0c72989a3 100755 --- a/vm/errors.cpp +++ b/vm/errors.cpp @@ -27,8 +27,10 @@ void out_of_memory() exit(1); } -void factor_vm::throw_error(cell error, stack_frame *callstack_top) +void factor_vm::throw_error(cell error, stack_frame *stack) { + assert(stack); + /* 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])) @@ -41,22 +43,13 @@ void factor_vm::throw_error(cell error, stack_frame *callstack_top) bignum_roots.clear(); code_roots.clear(); - /* If we had an underflow or overflow, stack pointers might be - out of bounds */ + /* If we had an underflow or overflow, data or retain stack + pointers might be out of bounds */ ctx->fix_stacks(); ctx->push(error); - /* Errors thrown from C code pass NULL for this parameter. - Errors thrown from Factor code, or signal handlers, pass the - actual stack pointer at the time, since the saved pointer is - not necessarily up to date at that point. */ - if(callstack_top) - callstack_top = fix_callstack_top(callstack_top,ctx->callstack_bottom); - else - callstack_top = ctx->callstack_top; - - unwind_native_frames(special_objects[ERROR_HANDLER_QUOT],callstack_top); + unwind_native_frames(special_objects[ERROR_HANDLER_QUOT],stack); } /* Error was thrown in early startup before error handler is set, just crash. */ @@ -70,62 +63,55 @@ void factor_vm::throw_error(cell error, stack_frame *callstack_top) } } -void factor_vm::general_error(vm_error_type error, cell arg1, cell arg2, stack_frame *callstack_top) +void factor_vm::general_error(vm_error_type error, cell arg1, cell arg2, stack_frame *stack) { throw_error(allot_array_4(special_objects[OBJ_ERROR], - tag_fixnum(error),arg1,arg2),callstack_top); + tag_fixnum(error),arg1,arg2),stack); +} + +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),ctx->callstack_top); } void factor_vm::type_error(cell type, cell tagged) { - general_error(ERROR_TYPE,tag_fixnum(type),tagged,NULL); + general_error(ERROR_TYPE,tag_fixnum(type),tagged); } void factor_vm::not_implemented_error() { - general_error(ERROR_NOT_IMPLEMENTED,false_object,false_object,NULL); + general_error(ERROR_NOT_IMPLEMENTED,false_object,false_object); } -/* Test if 'fault' is in the guard page at the top or bottom (depending on -offset being 0 or -1) of area+area_size */ -bool factor_vm::in_page(cell fault, cell area, cell area_size, int offset) +void factor_vm::memory_protection_error(cell addr, stack_frame *stack) { - int pagesize = getpagesize(); - area += area_size; - area += offset * pagesize; - - return fault >= area && fault <= area + pagesize; -} - -void factor_vm::memory_protection_error(cell addr, stack_frame *native_stack) -{ - if(in_page(addr, ctx->datastack_seg->start, 0, -1)) - general_error(ERROR_DS_UNDERFLOW,false_object,false_object,native_stack); - else if(in_page(addr, ctx->datastack_seg->start, datastack_size, 0)) - general_error(ERROR_DS_OVERFLOW,false_object,false_object,native_stack); - else if(in_page(addr, ctx->retainstack_seg->start, 0, -1)) - general_error(ERROR_RS_UNDERFLOW,false_object,false_object,native_stack); - else if(in_page(addr, ctx->retainstack_seg->start, retainstack_size, 0)) - general_error(ERROR_RS_OVERFLOW,false_object,false_object,native_stack); - else if(in_page(addr, nursery.end, 0, 0)) - critical_error("allot_object() missed GC check",0); + if(ctx->datastack_seg->underflow_p(addr)) + general_error(ERROR_DATASTACK_UNDERFLOW,false_object,false_object,stack); + else if(ctx->datastack_seg->overflow_p(addr)) + general_error(ERROR_DATASTACK_OVERFLOW,false_object,false_object,stack); + else if(ctx->retainstack_seg->underflow_p(addr)) + general_error(ERROR_RETAINSTACK_UNDERFLOW,false_object,false_object,stack); + else if(ctx->retainstack_seg->overflow_p(addr)) + general_error(ERROR_RETAINSTACK_OVERFLOW,false_object,false_object,stack); else - general_error(ERROR_MEMORY,allot_cell(addr),false_object,native_stack); + general_error(ERROR_MEMORY,allot_cell(addr),false_object,stack); } -void factor_vm::signal_error(cell signal, stack_frame *native_stack) +void factor_vm::signal_error(cell signal, stack_frame *stack) { - general_error(ERROR_SIGNAL,allot_cell(signal),false_object,native_stack); + general_error(ERROR_SIGNAL,allot_cell(signal),false_object,stack); } void factor_vm::divide_by_zero_error() { - general_error(ERROR_DIVIDE_BY_ZERO,false_object,false_object,NULL); + general_error(ERROR_DIVIDE_BY_ZERO,false_object,false_object); } -void factor_vm::fp_trap_error(unsigned int fpu_status, stack_frame *signal_callstack_top) +void factor_vm::fp_trap_error(unsigned int fpu_status, stack_frame *stack) { - general_error(ERROR_FP_TRAP,tag_fixnum(fpu_status),false_object,signal_callstack_top); + general_error(ERROR_FP_TRAP,tag_fixnum(fpu_status),false_object,stack); } void factor_vm::primitive_call_clear() diff --git a/vm/errors.hpp b/vm/errors.hpp index 4b237e03a0..0ce5957aef 100755 --- a/vm/errors.hpp +++ b/vm/errors.hpp @@ -14,10 +14,10 @@ enum vm_error_type ERROR_C_STRING, ERROR_FFI, ERROR_UNDEFINED_SYMBOL, - ERROR_DS_UNDERFLOW, - ERROR_DS_OVERFLOW, - ERROR_RS_UNDERFLOW, - ERROR_RS_OVERFLOW, + ERROR_DATASTACK_UNDERFLOW, + ERROR_DATASTACK_OVERFLOW, + ERROR_RETAINSTACK_UNDERFLOW, + ERROR_RETAINSTACK_OVERFLOW, ERROR_MEMORY, ERROR_FP_TRAP, }; diff --git a/vm/io.cpp b/vm/io.cpp index fdd872457e..8ce7ff5256 100755 --- a/vm/io.cpp +++ b/vm/io.cpp @@ -28,7 +28,7 @@ void factor_vm::io_error() return; #endif - general_error(ERROR_IO,tag_fixnum(errno),false_object,NULL); + general_error(ERROR_IO,tag_fixnum(errno),false_object); } FILE *factor_vm::safe_fopen(char *filename, char *mode) diff --git a/vm/mach_signal.cpp b/vm/mach_signal.cpp index 6295381b1c..af14c3a49a 100644 --- a/vm/mach_signal.cpp +++ b/vm/mach_signal.cpp @@ -35,19 +35,9 @@ void factor_vm::call_fault_handler( MACH_THREAD_STATE_TYPE *thread_state, MACH_FLOAT_STATE_TYPE *float_state) { - /* There is a race condition here, but in practice an exception - delivered during stack frame setup/teardown or while transitioning - from Factor to C is a sign of things seriously gone wrong, not just - a divide by zero or stack underflow in the listener */ + MACH_STACK_POINTER(thread_state) = (cell)fix_callstack_top((stack_frame *)MACH_STACK_POINTER(thread_state)); - /* Are we in compiled Factor code? Then use the current stack pointer */ - if(in_code_heap_p(MACH_PROGRAM_COUNTER(thread_state))) - signal_callstack_top = (stack_frame *)MACH_STACK_POINTER(thread_state); - /* Are we in C? Then use the saved callstack top */ - else - signal_callstack_top = NULL; - - MACH_STACK_POINTER(thread_state) = align_stack_pointer(MACH_STACK_POINTER(thread_state)); + signal_callstack_top = (stack_frame *)MACH_STACK_POINTER(thread_state); /* Now we point the program counter at the right handler function. */ if(exception == EXC_BAD_ACCESS) diff --git a/vm/math.cpp b/vm/math.cpp index bb5d9c13c4..a462232344 100755 --- a/vm/math.cpp +++ b/vm/math.cpp @@ -246,7 +246,7 @@ cell factor_vm::unbox_array_size_slow() } } - general_error(ERROR_ARRAY_SIZE,ctx->pop(),tag_fixnum(array_size_max),NULL); + general_error(ERROR_ARRAY_SIZE,ctx->pop(),tag_fixnum(array_size_max)); return 0; /* can't happen */ } diff --git a/vm/os-genunix.hpp b/vm/os-genunix.hpp index ff5d29ecd7..1972a728e6 100644 --- a/vm/os-genunix.hpp +++ b/vm/os-genunix.hpp @@ -10,9 +10,4 @@ void early_init(); const char *vm_executable_path(); const char *default_image_path(); -template Type align_stack_pointer(Type sp) -{ - return sp; -} - } diff --git a/vm/os-macosx-ppc.hpp b/vm/os-macosx-ppc.hpp index 30fd4b2081..90da9a26f3 100644 --- a/vm/os-macosx-ppc.hpp +++ b/vm/os-macosx-ppc.hpp @@ -62,11 +62,6 @@ inline static unsigned int uap_fpu_status(void *uap) return mach_fpu_status(UAP_FS(uap)); } -template Type align_stack_pointer(Type sp) -{ - return sp; -} - inline static void mach_clear_fpu_status(ppc_float_state_t *float_state) { FPSCR(float_state) &= 0x0007f8ff; diff --git a/vm/os-macosx-x86.32.hpp b/vm/os-macosx-x86.32.hpp index a6fe8e2703..3d754ae9e4 100644 --- a/vm/os-macosx-x86.32.hpp +++ b/vm/os-macosx-x86.32.hpp @@ -64,11 +64,6 @@ inline static unsigned int uap_fpu_status(void *uap) return mach_fpu_status(UAP_FS(uap)); } -template Type align_stack_pointer(Type sp) -{ - return (Type)((((cell)sp + 4) & ~15) - 4); -} - inline static void mach_clear_fpu_status(i386_float_state_t *float_state) { MXCSR(float_state) &= 0xffffffc0; diff --git a/vm/os-macosx-x86.64.hpp b/vm/os-macosx-x86.64.hpp index cb1980ddbf..7cef436327 100644 --- a/vm/os-macosx-x86.64.hpp +++ b/vm/os-macosx-x86.64.hpp @@ -62,11 +62,6 @@ inline static unsigned int uap_fpu_status(void *uap) return mach_fpu_status(UAP_FS(uap)); } -template Type align_stack_pointer(Type sp) -{ - return (Type)((((cell)sp + 8) & ~15) - 8); -} - inline static void mach_clear_fpu_status(x86_float_state64_t *float_state) { MXCSR(float_state) &= 0xffffffc0; diff --git a/vm/os-unix.cpp b/vm/os-unix.cpp index f63b509cb5..94d2a31839 100644 --- a/vm/os-unix.cpp +++ b/vm/os-unix.cpp @@ -85,7 +85,7 @@ void *factor_vm::ffi_dlsym(dll *dll, symbol_char *symbol) void factor_vm::ffi_dlclose(dll *dll) { if(dlclose(dll->handle)) - general_error(ERROR_FFI,false_object,false_object,NULL); + general_error(ERROR_FFI,false_object,false_object); dll->handle = NULL; } @@ -103,7 +103,7 @@ void factor_vm::move_file(const vm_char *path1, const vm_char *path2) ret = rename((path1),(path2)); } while(ret < 0 && errno == EINTR); if(ret < 0) - general_error(ERROR_IO,tag_fixnum(errno),false_object,NULL); + general_error(ERROR_IO,tag_fixnum(errno),false_object); } segment::segment(cell size_, bool executable_p) @@ -141,16 +141,7 @@ segment::~segment() void factor_vm::dispatch_signal(void *uap, void (handler)()) { - if(in_code_heap_p(UAP_PROGRAM_COUNTER(uap))) - { - stack_frame *ptr = (stack_frame *)UAP_STACK_POINTER(uap); - assert(ptr); - signal_callstack_top = ptr; - } - else - signal_callstack_top = NULL; - - UAP_STACK_POINTER(uap) = align_stack_pointer(UAP_STACK_POINTER(uap)); + UAP_STACK_POINTER(uap) = fix_callstack_top((stack_frame *)UAP_STACK_POINTER(uap)); UAP_PROGRAM_COUNTER(uap) = (cell)handler; } diff --git a/vm/os-windows-nt.cpp b/vm/os-windows-nt.cpp index d33a935f7f..e063fe3db3 100755 --- a/vm/os-windows-nt.cpp +++ b/vm/os-windows-nt.cpp @@ -74,6 +74,8 @@ LONG factor_vm::exception_handler(PEXCEPTION_POINTERS pe) PEXCEPTION_RECORD e = (PEXCEPTION_RECORD)pe->ExceptionRecord; CONTEXT *c = (CONTEXT*)pe->ContextRecord; + c->ESP = (cell)fix_callstack_top((stack_frame *)c->ESP); + if(in_code_heap_p(c->EIP)) signal_callstack_top = (stack_frame *)c->ESP; else diff --git a/vm/os-windows.cpp b/vm/os-windows.cpp index 08f5932172..d69966567a 100755 --- a/vm/os-windows.cpp +++ b/vm/os-windows.cpp @@ -140,7 +140,7 @@ long getpagesize() void factor_vm::move_file(const vm_char *path1, const vm_char *path2) { if(MoveFileEx((path1),(path2),MOVEFILE_REPLACE_EXISTING) == false) - general_error(ERROR_IO,tag_fixnum(GetLastError()),false_object,NULL); + general_error(ERROR_IO,tag_fixnum(GetLastError()),false_object); } } diff --git a/vm/segments.hpp b/vm/segments.hpp index 5cedada578..7f86c35485 100644 --- a/vm/segments.hpp +++ b/vm/segments.hpp @@ -15,6 +15,16 @@ struct segment { explicit segment(cell size, bool executable_p); ~segment(); + + bool underflow_p(cell addr) + { + return (addr >= start - getpagesize() && addr < start); + } + + bool overflow_p(cell addr) + { + return (addr >= end && addr < end + getpagesize()); + } }; } diff --git a/vm/vm.hpp b/vm/vm.hpp index defe4f24eb..7a0b0fcd33 100755 --- a/vm/vm.hpp +++ b/vm/vm.hpp @@ -160,20 +160,20 @@ struct factor_vm void primitive_profiling(); // errors - void throw_error(cell error, stack_frame *callstack_top); + void throw_error(cell error, stack_frame *stack); + void general_error(vm_error_type error, cell arg1, cell arg2, stack_frame *stack); + void general_error(vm_error_type error, cell arg1, cell arg2); + void type_error(cell type, cell tagged); void not_implemented_error(); - bool in_page(cell fault, cell area, cell area_size, int offset); - void memory_protection_error(cell addr, stack_frame *native_stack); - void signal_error(cell signal, stack_frame *native_stack); + void memory_protection_error(cell addr, stack_frame *stack); + void signal_error(cell signal, stack_frame *stack); void divide_by_zero_error(); - void fp_trap_error(unsigned int fpu_status, stack_frame *signal_callstack_top); + void fp_trap_error(unsigned int fpu_status, stack_frame *stack); void primitive_call_clear(); void primitive_unimplemented(); void memory_signal_handler_impl(); void misc_signal_handler_impl(); void fp_signal_handler_impl(); - void type_error(cell type, cell tagged); - void general_error(vm_error_type error, cell arg1, cell arg2, stack_frame *native_stack); // bignum int bignum_equal_p(bignum * x, bignum * y); @@ -582,7 +582,7 @@ struct factor_vm template void iterate_callstack_object(callstack *stack_, Iterator &iterator); void check_frame(stack_frame *frame); callstack *allot_callstack(cell size); - stack_frame *fix_callstack_top(stack_frame *top, stack_frame *bottom); + stack_frame *fix_callstack_top(stack_frame *top); stack_frame *second_from_top_stack_frame(); void primitive_callstack(); code_block *frame_code(stack_frame *frame);