Alexey,
2) It dropped support for "vm.assert_dialog" switch completely, which is proved to be quite useful for various kinds of automated testing.
Thank you for raising this issue, in particular, it affects Eclipse Unit Tests automated runs on Windows -- the tests execution is just interrupted when "Assertion failed" dialog window appears and waits for me to click "OK" button. So it would be great to keep "vm.assert_dialog" option if it is possible. Thanks, Nina On 11/28/06, Alexey Varlamov <[EMAIL PROTECTED]> wrote:
Gregory, A couple of spotted issues in the change: 1) Portablity: code is ia32-specific due to unit32-casted assignments to Registers fields. Why don't use POINTER_SIZE_INT or UDATA instead? 2) It dropped support for "vm.assert_dialog" switch completely, which is proved to be quite useful for various kinds of automated testing. We even discussed recently that launcher lacks similar feature, and I anticipate other developers will raise complains soon. It would be nice to address these while we are hot on the trail. -- Regards, Alexey 2006/11/28, [EMAIL PROTECTED] <[EMAIL PROTECTED]>: > Author: gshimansky > Date: Mon Nov 27 15:23:48 2006 > New Revision: 479802 > > URL: http://svn.apache.org/viewvc?view=rev&rev=479802 > Log: > Applied HARMONY-2320 [drlvm] Throw exception outside of HWE handler to avoid deadlocks on Windows XP > > Tests passed on windows, ubuntu and suse 10 x86_64 > > > Modified: > harmony/enhanced/drlvm/trunk/vm/vmcore/src/util/win/ia32/nt_exception_filter.cpp > > Modified: harmony/enhanced/drlvm/trunk/vm/vmcore/src/util/win/ia32/nt_exception_filter.cpp > URL: http://svn.apache.org/viewvc/harmony/enhanced/drlvm/trunk/vm/vmcore/src/util/win/ia32/nt_exception_filter.cpp?view=diff&rev=479802&r1=479801&r2=479802 > ============================================================================== > --- harmony/enhanced/drlvm/trunk/vm/vmcore/src/util/win/ia32/nt_exception_filter.cpp (original) > +++ harmony/enhanced/drlvm/trunk/vm/vmcore/src/util/win/ia32/nt_exception_filter.cpp Mon Nov 27 15:23:48 2006 > @@ -19,14 +19,16 @@ > * @version $Revision: 1.1.2.1.4.4 $ > */ > > -#include "cxxlog.h" > +#include "clog.h" > #include "method_lookup.h" > #include "Environment.h" > #include "exceptions.h" > #include "exceptions_jit.h" > #include "interpreter_exports.h" > +#include "stack_iterator.h" > #include "stack_dump.h" > #include "jvmti_break_intf.h" > +#include "m2n.h" > > // Windows specific > #include <string> > @@ -277,6 +279,7 @@ > } > > static LONG NTAPI vectored_exception_handler_internal(LPEXCEPTION_POINTERS nt_exception); > +static void __cdecl c_exception_handler(Class*, bool); > > LONG __declspec(naked) NTAPI vectored_exception_handler(LPEXCEPTION_POINTERS nt_exception) > { > @@ -297,94 +300,80 @@ > static LONG NTAPI vectored_exception_handler_internal(LPEXCEPTION_POINTERS nt_exception) > { > DWORD code = nt_exception->ExceptionRecord->ExceptionCode; > - > - bool run_default_handler = true; > PCONTEXT context = nt_exception->ContextRecord; > - > - TRACE2("signals", "VEH received an exception: code = 0x" << > - ((void*)nt_exception->ExceptionRecord->ExceptionCode) << > - " location IP = 0x" << ((void*)context->Eip)); > - > - // this filter catches _all_ hardware exceptions including those caused by > - // VM internal code. To elimate confusion over what caused the > - // exception, we first make sure the exception was thrown inside a Java > - // method else crash handler or default handler is executed, this means that > - // it was thrown by VM C/C++ code. > - if (((code == STATUS_ACCESS_VIOLATION || > - code == STATUS_INTEGER_DIVIDE_BY_ZERO || > - code == STATUS_STACK_OVERFLOW) && > - vm_identify_eip((void *)context->Eip) == VM_TYPE_JAVA) || > - code == JVMTI_EXCEPTION_STATUS) { > - run_default_handler = false; > - } else if (code == STATUS_STACK_OVERFLOW) { > - if (is_unwindable()) { > - if (hythread_is_suspend_enabled()) { > - tmn_suspend_disable(); > - } > - run_default_handler = false; > - } else { > - Global_Env *env = VM_Global_State::loader_env; > - exn_raise_by_class(env->java_lang_StackOverflowError_Class); > - p_TLS_vmthread->restore_guard_page = true; > - return EXCEPTION_CONTINUE_EXECUTION; > - } > - } > > - if (run_default_handler) { > -#ifndef NDEBUG > - if (vm_get_boolean_property_value_with_default("vm.assert_dialog")) { > - > - if (IS_ERROR(code)) { > - if (UnhandledExceptionFilter(nt_exception) == EXCEPTION_CONTINUE_SEARCH) > - DebugBreak(); > - } > - } > -#endif > + TRACE2("signals", ("VEH received an exception: code = %x, eip = %p, esp = %p", > + nt_exception->ExceptionRecord->ExceptionCode, > + context->Eip, context->Esp)); > + > + // the possible reasons for hardware exception are > + // - segfault or division by zero in java code > + // => NullPointerException or ArithmeticException > + // > + // - breakpoint or privileged instruction in java code > + // => send jvmti breakpoint event > + // > + // - stack overflow, either in java or in native > + // => StackOverflowError > + // > + // - other (internal VM error or debugger breakpoint) > + // => delegate to default handler > + > + bool in_java = (vm_identify_eip((void*)context->Eip) == VM_TYPE_JAVA); > + > + // delegate "other" cases to default handler > + if (!in_java && code != STATUS_STACK_OVERFLOW) > return EXCEPTION_CONTINUE_SEARCH; > - } > > - // since we are now sure HWE occured in java code, gc should also have been disabled > + // if HWE occured in java code, suspension should also have been disabled > + assert(!in_java || !hythread_is_suspend_enabled()); > > - // gregory - this is not true since for debugging we may use int3 > - // in VM code which produces BREAKPOINT exception. JVMTI has > - // assertions for breakpoints which it has set in Java inside of > - // breakpoint handling function. Otherwise this assert should not > - // fail in case _CrtDbgBreak() was added somewhere in VM. > - assert(!hythread_is_suspend_enabled() || code == JVMTI_EXCEPTION_STATUS); > - > Global_Env *env = VM_Global_State::loader_env; > - Class *exc_clss = 0; > + // the actual exception object will be created lazily, > + // we determine only exception class here > + Class *exn_class = 0; > > switch(nt_exception->ExceptionRecord->ExceptionCode) > { > case STATUS_STACK_OVERFLOW: > - // stack overflow exception -- see ...\vc\include\winnt.h > { > - TRACE2("signals", "StackOverflowError detected at " > - << (void *) context->Eip << " on the stack at " > - << (void *) context->Esp); > - // Lazy exception object creation > - exc_clss = env->java_lang_StackOverflowError_Class; > + TRACE2("signals", > + ("StackOverflowError detected at eip = %p, esp = %p", > + context->Eip,context->Esp)); > + > p_TLS_vmthread->restore_guard_page = true; > + exn_class = env->java_lang_StackOverflowError_Class; > + if (in_java) { > + // stack overflow occured in java code: > + // nothing special to do > + } else if (is_unwindable()) { > + // stack overflow occured in native code that can be unwound > + // safely. > + // Throwing exception requires suspend disabled status > + if (hythread_is_suspend_enabled()) > + hythread_suspend_disable(); > + } else { > + // stack overflow occured in native code that > + // cannot be unwound. > + // Mark raised exception in TLS and resume execution > + exn_raise_by_class(env->java_lang_StackOverflowError_Class); > + return EXCEPTION_CONTINUE_EXECUTION; > + } > } > break; > case STATUS_ACCESS_VIOLATION: > - // null pointer exception -- see ...\vc\include\winnt.h > { > - TRACE2("signals", "NullPointerException detected at " > - << (void *) context->Eip); > - // Lazy exception object creation > - exc_clss = env->java_lang_NullPointerException_Class; > + TRACE2("signals", > + ("NullPointerException detected at eip = %p", context->Eip)); > + exn_class = env->java_lang_NullPointerException_Class; > } > break; > > case STATUS_INTEGER_DIVIDE_BY_ZERO: > - // divide by zero exception -- see ...\vc\include\winnt.h > { > - TRACE2("signals", "ArithmeticException detected at " > - << (void *) context->Eip); > - // Lazy exception object creation > - exc_clss = env->java_lang_ArithmeticException_Class; > + TRACE2("signals", > + ("ArithmeticException detected at eip = %p", context->Eip)); > + exn_class = env->java_lang_ArithmeticException_Class; > } > break; > case JVMTI_EXCEPTION_STATUS: > @@ -392,8 +381,8 @@ > { > Registers regs; > nt_to_vm_context(context, ®s); > - TRACE2("signals", "JVMTI breakpoint detected at " << > - (void *)regs.eip); > + TRACE2("signals", > + ("JVMTI breakpoint detected at eip = %p", regs.eip)); > bool handled = jvmti_jit_breakpoint_handler(®s); > if (handled) > { > @@ -403,21 +392,59 @@ > else > return EXCEPTION_CONTINUE_SEARCH; > } > - default: assert(false); > + default: > + // unexpected hardware exception occured in java code > + return EXCEPTION_CONTINUE_SEARCH; > } > > - Registers regs; > + // we must not call potentially blocking or suspendable code > + // (i.e. java code of exception constructor) from exception > + // handler, because this handler may hold a system-wide lock, > + // and this may result in a deadlock. > + > + // it was reported that exception handler grabs a system > + // lock on Windows XPsp2 and 2003sp0, but not on a 2003sp1 > + > + // save register context of hardware exception site > + // into thread-local registers snapshot > + assert(p_TLS_vmthread); > + nt_to_vm_context(context, &p_TLS_vmthread->regs); > + > + // __cdecl <=> push parameters in the reversed order > + // push in_java argument onto stack > + context->Esp -= 4; > + *((uint32*) context->Esp) = (uint32)in_java; > + // push the exn_class argument onto stack > + context->Esp -= 4; > + assert(exn_class); > + *((uint32*) context->Esp) = (uint32)exn_class; > + // imitate return IP on stack > + context->Esp -= 4; > > - nt_to_vm_context(context, ®s); > + // set up the real exception handler address > + context->Eip = (uint32)c_exception_handler; > > - uint32 exception_esp = regs.esp; > + // exit NT exception handler and transfer > + // control to VM exception handler > + return EXCEPTION_CONTINUE_EXECUTION; > +} > + > + > +static void __cdecl c_exception_handler(Class *exn_class, bool in_java) > +{ > + // this exception handler is executed *after* NT exception handler returned > DebugUtilsTI* ti = VM_Global_State::loader_env->TI; > + Registers & regs = p_TLS_vmthread->regs; > > - // TODO: We already checked that above. Is it possible to reuse the result? > - bool java_code = (vm_identify_eip((void *)regs.eip) == VM_TYPE_JAVA); > - exn_athrow_regs(®s, exc_clss, java_code); > + M2nFrame* prev_m2n = m2n_get_last_frame(); > + M2nFrame* m2n = NULL; > + if (in_java) > + m2n = m2n_push_suspended_frame(®s); > + > + TRACE2("signals", ("should throw exception %p at EIP=%p, ESP=%p", > + exn_class, regs.eip, regs.esp)); > + exn_athrow_regs(®s, exn_class, false); > > - assert(exception_esp <= regs.esp); > if (ti->get_global_capability(DebugUtilsTI::TI_GC_ENABLE_EXCEPTION_EVENT)) { > regs.esp = regs.esp - 4; > *((uint32*) regs.esp) = regs.eip; > @@ -428,7 +455,10 @@ > regs.eip = ((uint32)asm_exception_catch_callback); > } > > - vm_to_nt_context(®s, context); > - > - return EXCEPTION_CONTINUE_EXECUTION; > -} //vectored_exception_handler > + StackIterator *si = > + si_create_from_registers(®s, false, prev_m2n); > + if (m2n) > + STD_FREE(m2n); > + si_transfer_control(si); > + assert(!"si_transfer_control should not return"); > +} > > >
