You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@harmony.apache.org by Alexey Varlamov <al...@gmail.com> on 2006/11/28 13:51:12 UTC
Re: svn commit: r479802 - /harmony/enhanced/drlvm/trunk/vm/vmcore/src/util/win/ia32/nt_exception_filter.cpp
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, gshimansky@apache.org <gs...@apache.org>:
> 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");
> +}
>
>
>
Re: svn commit: r479802 - /harmony/enhanced/drlvm/trunk/vm/vmcore/src/util/win/ia32/nt_exception_filter.cpp
Posted by Nina Rinskaya <ni...@gmail.com>.
Evgueni,
Thanks for explanation. I'm pretty sure I got "Assertion failed" popup
window with a recent build and -Dvm.assert_dialog=false, but I can't
reproduce it now.
Thanks,
Nina
On 11/28/06, Evgueni Brevnov <ev...@gmail.com> wrote:
> On 11/28/06, Nina Rinskaya <ni...@gmail.com> wrote:
> > 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.
>
> Current implementation doesn't open assertion dialog at all. So you
> should not have any troubles.
>
> Evgueni
>
> >
> > Thanks,
> > Nina
> >
> > On 11/28/06, Alexey Varlamov <al...@gmail.com> 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, gshimansky@apache.org <gs...@apache.org>:
> > > > 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");
> > > > +}
> > > >
> > > >
> > > >
> > >
> >
>
Re: svn commit: r479802 - /harmony/enhanced/drlvm/trunk/vm/vmcore/src/util/win/ia32/nt_exception_filter.cpp
Posted by Evgueni Brevnov <ev...@gmail.com>.
On 11/28/06, Nina Rinskaya <ni...@gmail.com> wrote:
> 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.
Current implementation doesn't open assertion dialog at all. So you
should not have any troubles.
Evgueni
>
> Thanks,
> Nina
>
> On 11/28/06, Alexey Varlamov <al...@gmail.com> 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, gshimansky@apache.org <gs...@apache.org>:
> > > 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");
> > > +}
> > >
> > >
> > >
> >
>
Re: svn commit: r479802 - /harmony/enhanced/drlvm/trunk/vm/vmcore/src/util/win/ia32/nt_exception_filter.cpp
Posted by Salikh Zakirov <Sa...@Intel.com>.
Nina Rinskaya wrote:
> 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.
Once again: the switch "vm.assert_dialog" was not dropped.
The thing that was removed was manual call to _CrtDbgBreak().
New code relies on the default handler instead.
Now, the default handler in launcher always catches exceptions,
and never shows debug dialog, so what we need now is improve
launcher to support debug dialogs.
Re: svn commit: r479802 - /harmony/enhanced/drlvm/trunk/vm/vmcore/src/util/win/ia32/nt_exception_filter.cpp
Posted by Nina Rinskaya <ni...@gmail.com>.
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 <al...@gmail.com> 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, gshimansky@apache.org <gs...@apache.org>:
> > 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");
> > +}
> >
> >
> >
>
Re: svn commit: r479802 - /harmony/enhanced/drlvm/trunk/vm/vmcore/src/util/win/ia32/nt_exception_filter.cpp
Posted by Evgueni Brevnov <ev...@gmail.com>.
On 11/28/06, Alexey Varlamov <al...@gmail.com> 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.
Alexey,
Definitely, this is important issue for many of us. I think this
problem should be resolved on the classlib's side. Namely, the
launcher should not override default handler in case we want a dialog
to appear.
Evgueni.
>
> --
> Regards,
> Alexey
>
>
> 2006/11/28, gshimansky@apache.org <gs...@apache.org>:
> > 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");
> > +}
> >
> >
> >
>
Re: svn commit: r479802 - /harmony/enhanced/drlvm/trunk/vm/vmcore/src/util/win/ia32/nt_exception_filter.cpp
Posted by Tim Ellison <t....@gmail.com>.
Geir Magnusson Jr. wrote:
> Yes, to be consistent. Because if vm.assert_dialog has anything to do
> with a _dialog_ for _asserts_, we should keep with DRLVM tradition and
> name it something only barely, tenuously similar.
>
> How about
>
> vm.checker_bannana
I'm good if you remove the vm prefix. The double-n is a nice touch. <g>
Regards,
Tim
--
Tim Ellison (t.p.ellison@gmail.com)
IBM Java technology centre, UK.
Re: svn commit: r479802 - /harmony/enhanced/drlvm/trunk/vm/vmcore/src/util/win/ia32/nt_exception_filter.cpp
Posted by "Geir Magnusson Jr." <ge...@pobox.com>.
Gregory Shimansky wrote:
> Salikh Zakirov wrote:
>> Alexey Varlamov 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?
>>
>>
>> This is in ia32/nt_exception_filter.cpp, so no portability concerns.
>> I agree that we may want to generalize it multiple architectures in
>> the future,
>> but it wasn't my concern now.
>
> Agreed. The register names are ia32 specific too, so 32-bitness is not
> the only unportable part of the file.
>
>>> 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.
>>
>> Despite the fact that "vm.assert_dialog" check was removed, the support
>> is still there: "vm.assert_dialog" controls the behaviour of the default
>> handler.
>>
>> However, you are right that now the overall behaviour is determined
>> by the launcher, so we need to change launcher correspondingly.
>
> Shall we agree that launcher should parse and handle this property? The
> name doesn't look very good to me (Geir will surely ask to change it).
Yes, to be consistent. Because if vm.assert_dialog has anything to do
with a _dialog_ for _asserts_, we should keep with DRLVM tradition and
name it something only barely, tenuously similar.
How about
vm.checker_bannana
?
> What about vm.windows.crt.debug.dialog ? AFAIK the property has no
> effect on linux, and the word "assert" doesn't reflect the purpose of
> this property too.
>
Sure you can't fit "checker" in there?
:)
geir
Re: [general] Winx86 platform support
Posted by Rana Dasgupta <rd...@gmail.com>.
OK, I will start pegging away at it.
On 11/30/06, Pavel Ozhdikhin <pa...@gmail.com> wrote:
>
> Rana,
>
> I think now it's just a right time to start this work. It would be good if
> you do preliminary examination of which areas need to be fixed for 64 bit
> Windows in VM/JIT. I don't think there will be many problems eith enabling
> on the JIT side - so far optimizations which do not work on EM64 (for
> example those requiring fs:14 for TLS access) are turned off there.
>
> Thanks,
> Pavel
>
>
> On 11/30/06, Rana Dasgupta <rd...@gmail.com> wrote:
> >
> > This port would be really good to have, but not urgent. Obviously, the
> > biggest change is in the JIT. At the moment, I think that we should
> focus
> > on
> > stabilizing and performing on the 3 major platforms we are functional
> on.
> > The IPF port is also in progress. However, with the Vista releases early
> > next year? ( 32 and 64 bits ), 64 bit Windows will be a major platform
> and
> > we will need to be on it. I am going to start going thru DRLVM to
> identify
> > functional areas and implementation that need to change, play around
> with
> > the Microsoft 64 bit toolsets( ML64 assembler, CL, LINK, Visual Studio
> 64
> > bit ) etc. and start putting together a Windows64 porting guide on the
> > Wiki.
> > Or is it too early to do this?
> >
> > Thanks.
> >
> >
> > On 11/29/06, Rana Dasgupta <rd...@gmail.com> wrote:
> > >
> > > In 32 bit mode or 64 bit mode? We are not ported/functional yet on 64
> > bit
> > > Windows for exception handling, JIT etc. No [fs:14] for TLS access,
> > windows
> > > handles are 8 bytes, all the inline asm{} needs to go away, most data
> > types
> > > are different...More changes than Linux 32 -> Linux 64. I would think
> > that
> > > the work involved would be somewhat similar to an IPF port. Certainly
> > worth
> > > doing though.
> > >
> > > On 11/29/06, Geir Magnusson Jr. <ge...@pobox.com> wrote:
> > > >
> > > > +1
> > > >
> > > > we do want to support this platform, but need an installed 64bit
> > windows
> > > > machine to work on - or at least someone to have it and give us
> > > > feedback.
> > > >
> > > >
> > > >
> > > > Tim Ellison wrote:
> > > > > Alexey Varlamov wrote:
> > > > >> BTW, are we interested in supporting Windows on x86_64? It is
> > listed
> > > > >> on [1] but seems nobody really tests.
> > > > >
> > > > > Yes! Do you have a machine to contribute to the build/test? I'm
> up
> > > > for
> > > > > helping to fix problems.
> > > > >
> > > > > Regards,
> > > > > Tim
> > > > >
> > > >
> > >
> > >
> >
> >
>
>
Re: [general] Winx86 platform support
Posted by "Geir Magnusson Jr." <ge...@pobox.com>.
+1
Pavel Ozhdikhin wrote:
> Rana,
>
> I think now it's just a right time to start this work. It would be good if
> you do preliminary examination of which areas need to be fixed for 64 bit
> Windows in VM/JIT. I don't think there will be many problems eith enabling
> on the JIT side - so far optimizations which do not work on EM64 (for
> example those requiring fs:14 for TLS access) are turned off there.
>
> Thanks,
> Pavel
>
>
> On 11/30/06, Rana Dasgupta <rd...@gmail.com> wrote:
>>
>> This port would be really good to have, but not urgent. Obviously, the
>> biggest change is in the JIT. At the moment, I think that we should focus
>> on
>> stabilizing and performing on the 3 major platforms we are functional on.
>> The IPF port is also in progress. However, with the Vista releases early
>> next year? ( 32 and 64 bits ), 64 bit Windows will be a major platform
>> and
>> we will need to be on it. I am going to start going thru DRLVM to
>> identify
>> functional areas and implementation that need to change, play around with
>> the Microsoft 64 bit toolsets( ML64 assembler, CL, LINK, Visual Studio 64
>> bit ) etc. and start putting together a Windows64 porting guide on the
>> Wiki.
>> Or is it too early to do this?
>>
>> Thanks.
>>
>>
>> On 11/29/06, Rana Dasgupta <rd...@gmail.com> wrote:
>> >
>> > In 32 bit mode or 64 bit mode? We are not ported/functional yet on 64
>> bit
>> > Windows for exception handling, JIT etc. No [fs:14] for TLS access,
>> windows
>> > handles are 8 bytes, all the inline asm{} needs to go away, most data
>> types
>> > are different...More changes than Linux 32 -> Linux 64. I would think
>> that
>> > the work involved would be somewhat similar to an IPF port. Certainly
>> worth
>> > doing though.
>> >
>> > On 11/29/06, Geir Magnusson Jr. <ge...@pobox.com> wrote:
>> > >
>> > > +1
>> > >
>> > > we do want to support this platform, but need an installed 64bit
>> windows
>> > > machine to work on - or at least someone to have it and give us
>> > > feedback.
>> > >
>> > >
>> > >
>> > > Tim Ellison wrote:
>> > > > Alexey Varlamov wrote:
>> > > >> BTW, are we interested in supporting Windows on x86_64? It is
>> listed
>> > > >> on [1] but seems nobody really tests.
>> > > >
>> > > > Yes! Do you have a machine to contribute to the build/test?
>> I'm up
>> > > for
>> > > > helping to fix problems.
>> > > >
>> > > > Regards,
>> > > > Tim
>> > > >
>> > >
>> >
>> >
>>
>>
>
Re: [general] Winx86 platform support
Posted by Pavel Ozhdikhin <pa...@gmail.com>.
Rana,
I think now it's just a right time to start this work. It would be good if
you do preliminary examination of which areas need to be fixed for 64 bit
Windows in VM/JIT. I don't think there will be many problems eith enabling
on the JIT side - so far optimizations which do not work on EM64 (for
example those requiring fs:14 for TLS access) are turned off there.
Thanks,
Pavel
On 11/30/06, Rana Dasgupta <rd...@gmail.com> wrote:
>
> This port would be really good to have, but not urgent. Obviously, the
> biggest change is in the JIT. At the moment, I think that we should focus
> on
> stabilizing and performing on the 3 major platforms we are functional on.
> The IPF port is also in progress. However, with the Vista releases early
> next year? ( 32 and 64 bits ), 64 bit Windows will be a major platform and
> we will need to be on it. I am going to start going thru DRLVM to identify
> functional areas and implementation that need to change, play around with
> the Microsoft 64 bit toolsets( ML64 assembler, CL, LINK, Visual Studio 64
> bit ) etc. and start putting together a Windows64 porting guide on the
> Wiki.
> Or is it too early to do this?
>
> Thanks.
>
>
> On 11/29/06, Rana Dasgupta <rd...@gmail.com> wrote:
> >
> > In 32 bit mode or 64 bit mode? We are not ported/functional yet on 64
> bit
> > Windows for exception handling, JIT etc. No [fs:14] for TLS access,
> windows
> > handles are 8 bytes, all the inline asm{} needs to go away, most data
> types
> > are different...More changes than Linux 32 -> Linux 64. I would think
> that
> > the work involved would be somewhat similar to an IPF port. Certainly
> worth
> > doing though.
> >
> > On 11/29/06, Geir Magnusson Jr. <ge...@pobox.com> wrote:
> > >
> > > +1
> > >
> > > we do want to support this platform, but need an installed 64bit
> windows
> > > machine to work on - or at least someone to have it and give us
> > > feedback.
> > >
> > >
> > >
> > > Tim Ellison wrote:
> > > > Alexey Varlamov wrote:
> > > >> BTW, are we interested in supporting Windows on x86_64? It is
> listed
> > > >> on [1] but seems nobody really tests.
> > > >
> > > > Yes! Do you have a machine to contribute to the build/test? I'm up
> > > for
> > > > helping to fix problems.
> > > >
> > > > Regards,
> > > > Tim
> > > >
> > >
> >
> >
>
>
Re: [general] Winx86 platform support
Posted by Rana Dasgupta <rd...@gmail.com>.
This port would be really good to have, but not urgent. Obviously, the
biggest change is in the JIT. At the moment, I think that we should focus on
stabilizing and performing on the 3 major platforms we are functional on.
The IPF port is also in progress. However, with the Vista releases early
next year? ( 32 and 64 bits ), 64 bit Windows will be a major platform and
we will need to be on it. I am going to start going thru DRLVM to identify
functional areas and implementation that need to change, play around with
the Microsoft 64 bit toolsets( ML64 assembler, CL, LINK, Visual Studio 64
bit ) etc. and start putting together a Windows64 porting guide on the Wiki.
Or is it too early to do this?
Thanks.
On 11/29/06, Rana Dasgupta <rd...@gmail.com> wrote:
>
> In 32 bit mode or 64 bit mode? We are not ported/functional yet on 64 bit
> Windows for exception handling, JIT etc. No [fs:14] for TLS access, windows
> handles are 8 bytes, all the inline asm{} needs to go away, most data types
> are different...More changes than Linux 32 -> Linux 64. I would think that
> the work involved would be somewhat similar to an IPF port. Certainly worth
> doing though.
>
> On 11/29/06, Geir Magnusson Jr. <ge...@pobox.com> wrote:
> >
> > +1
> >
> > we do want to support this platform, but need an installed 64bit windows
> > machine to work on - or at least someone to have it and give us
> > feedback.
> >
> >
> >
> > Tim Ellison wrote:
> > > Alexey Varlamov wrote:
> > >> BTW, are we interested in supporting Windows on x86_64? It is listed
> > >> on [1] but seems nobody really tests.
> > >
> > > Yes! Do you have a machine to contribute to the build/test? I'm up
> > for
> > > helping to fix problems.
> > >
> > > Regards,
> > > Tim
> > >
> >
>
>
Re: [general] Winx86 platform support
Posted by Rana Dasgupta <rd...@gmail.com>.
In 32 bit mode or 64 bit mode? We are not ported/functional yet on 64 bit
Windows for exception handling, JIT etc. No [fs:14] for TLS access, windows
handles are 8 bytes, all the inline asm{} needs to go away, most data types
are different...More changes than Linux 32 -> Linux 64. I would think that
the work involved would be somewhat similar to an IPF port. Certainly worth
doing though.
On 11/29/06, Geir Magnusson Jr. <ge...@pobox.com> wrote:
>
> +1
>
> we do want to support this platform, but need an installed 64bit windows
> machine to work on - or at least someone to have it and give us feedback.
>
>
>
> Tim Ellison wrote:
> > Alexey Varlamov wrote:
> >> BTW, are we interested in supporting Windows on x86_64? It is listed
> >> on [1] but seems nobody really tests.
> >
> > Yes! Do you have a machine to contribute to the build/test? I'm up for
> > helping to fix problems.
> >
> > Regards,
> > Tim
> >
>
Re: [general] Winx86 platform support
Posted by "Geir Magnusson Jr." <ge...@pobox.com>.
+1
we do want to support this platform, but need an installed 64bit windows
machine to work on - or at least someone to have it and give us feedback.
Tim Ellison wrote:
> Alexey Varlamov wrote:
>> BTW, are we interested in supporting Windows on x86_64? It is listed
>> on [1] but seems nobody really tests.
>
> Yes! Do you have a machine to contribute to the build/test? I'm up for
> helping to fix problems.
>
> Regards,
> Tim
>
[general] Winx86 platform support (was: Re: svn commit: r479802 -
/harmony/enhanced/drlvm/trunk/vm/vmcore/src/util/win/ia32/nt_exception_filter.cpp)
Posted by Tim Ellison <t....@gmail.com>.
Alexey Varlamov wrote:
> BTW, are we interested in supporting Windows on x86_64? It is listed
> on [1] but seems nobody really tests.
Yes! Do you have a machine to contribute to the build/test? I'm up for
helping to fix problems.
Regards,
Tim
--
Tim Ellison (t.p.ellison@gmail.com)
IBM Java technology centre, UK.
Re: svn commit: r479802 - /harmony/enhanced/drlvm/trunk/vm/vmcore/src/util/win/ia32/nt_exception_filter.cpp
Posted by Gregory Shimansky <gs...@gmail.com>.
Alexey Varlamov wrote:
>> > Anyway, if you don't mind, I'll try to change the launcher code to use
>> > "launcher.enable.crash.handler" which should be true by default.
>>
>> How about using -X<option> instead of -D<option>?
> I'm going to integrated HARMONY-1925 soon, which introduces
> distinction between Java and VM properties as discussed earlier. Then
> it should become -XD<option> :)
> Regarding the name, why "launcher" prefix? It may be used by VM as
> well, as I said above.
I thought that VM would simply use the default handler (installed or not
installed by the launcher) when VEH handler receives an exception that
VM does not know.
So the only thing which this property would control is launcher behavior
to install or not install the crash handler.
--
Gregory
Re: svn commit: r479802 - /harmony/enhanced/drlvm/trunk/vm/vmcore/src/util/win/ia32/nt_exception_filter.cpp
Posted by Alexey Varlamov <al...@gmail.com>.
Guys, you are right, I was fast and stupid, partially due to personal
time limit.
Still this inspired such an interesting discussion ;)
Really we have a lot of duplication in low-level platform code in
DRLVM, this may become onerous soon enough. BTW, are we interested in
supporting Windows on x86_64? It is listed on [1] but seems nobody
really tests.
[1] http://wiki.apache.org/harmony/Platforms_to_Run_Harmony_Development_Kit_on
2006/11/29, Evgueni Brevnov <ev...@gmail.com>:
> On 11/28/06, Gregory Shimansky <gs...@gmail.com> wrote:
> > Geir Magnusson Jr. wrote:
> >
> > > Gregory Shimansky wrote:
> > >> Gregory Shimansky wrote:
> > >>>>> 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.
> > >>>>
> > >>>> Despite the fact that "vm.assert_dialog" check was removed, the support
> > >>>> is still there: "vm.assert_dialog" controls the behaviour of the
> > >>>> default
> > >>>> handler.
> > >>>>
> > >>>> However, you are right that now the overall behaviour is determined
> > >>>> by the launcher, so we need to change launcher correspondingly.
> > >>>
> > >>> Shall we agree that launcher should parse and handle this property?
> > >>> The name doesn't look very good to me (Geir will surely ask to change
> > >>> it).
> > >>>
> > >>> What about vm.windows.crt.debug.dialog ? AFAIK the property has no
> > >>> effect on linux, and the word "assert" doesn't reflect the purpose of
> > >>> this property too.
> > >>
> > >> Now that I thought about it some more, what about controlling the
> > >> launcher default crash handler on all platforms? Specifying something
> > >> like -Dhy.launcher.crash.handler=false to disable crash handler
> > >> installation in launcher.
> > >
> > > I like it, except for the preceeding "hy"... also, will this be used by
> > > any part of the system other than the launcher?
> >
> > I don't think it will be used anywhere except for the launcher. It is
> > launcher specific and actually Harmony launcher specific.
>
> On Windows except crash handler you need to change the default
> behaviour of _CrtDbgReport so it doesn't popup debug window. Currently
> it's the VM how changes this behaviour. Do you propose to move this
> functionality into launcher as well?
I think VM needs to be able to handle this even w/o the shared launcher.
Just a side note: grepped drlvm, there are 3(!) identical code
snippets for the same:
if (!vm_get_boolean_property_value_with_default("vm.assert_dialog"))
{
disable_assert_dialogs();
}
vm/vmcore/src/init/vm_init.cpp:692
vm/interpreter/src/interp_exports.cpp:132
vm/jitrino/src/vm/drl/DrlJITInterface.cpp:92
:)
>
> >
> > Anyway, if you don't mind, I'll try to change the launcher code to use
> > "launcher.enable.crash.handler" which should be true by default.
>
> How about using -X<option> instead of -D<option>?
I'm going to integrated HARMONY-1925 soon, which introduces
distinction between Java and VM properties as discussed earlier. Then
it should become -XD<option> :)
Regarding the name, why "launcher" prefix? It may be used by VM as
well, as I said above.
--
Alexey
>
> Evgueni
> >
> > --
> > Gregory
> >
> >
>
Re: svn commit: r479802 - /harmony/enhanced/drlvm/trunk/vm/vmcore/src/util/win/ia32/nt_exception_filter.cpp
Posted by Evgueni Brevnov <ev...@gmail.com>.
On 11/30/06, Salikh Zakirov <Sa...@intel.com> wrote:
> Evgueni Brevnov wrote:
> > Let's leave the -D or -X dilemma for a while. Nina Rinskya and me
> > found more serious problem. The problem is that debug windows popups
> > even if -Dvm.assert_dialog=false is specified. It happens because each
> > dll has its own copy of the variable which controls the behaviour of
> > an assert macro. In other words when you change the report mode by
> > calling _CrtSetReportMode it affect the current dll only. So if
> > assertion fails in any other dll we will see debug window.
> >
> > Any ideas how to fix that?
>
> A question, just to verify understanding:
>
> does portlib-style signal catching, which is done in launcher,
> misses these application errors too?
Yes it does. When assert fails it creates debug window explicitly. So
there is no connection between exception handlers and assert
statements.
>
> I was under impression that stock version of launcher does not allow
> any uncaught exception to slip through.
No exception is generated if assert fails....
>
>
Re: svn commit: r479802 - /harmony/enhanced/drlvm/trunk/vm/vmcore/src/util/win/ia32/nt_exception_filter.cpp
Posted by Salikh Zakirov <Sa...@Intel.com>.
Evgueni Brevnov wrote:
> Let's leave the -D or -X dilemma for a while. Nina Rinskya and me
> found more serious problem. The problem is that debug windows popups
> even if -Dvm.assert_dialog=false is specified. It happens because each
> dll has its own copy of the variable which controls the behaviour of
> an assert macro. In other words when you change the report mode by
> calling _CrtSetReportMode it affect the current dll only. So if
> assertion fails in any other dll we will see debug window.
>
> Any ideas how to fix that?
A question, just to verify understanding:
does portlib-style signal catching, which is done in launcher,
misses these application errors too?
I was under impression that stock version of launcher does not allow
any uncaught exception to slip through.
Re: svn commit: r479802 - /harmony/enhanced/drlvm/trunk/vm/vmcore/src/util/win/ia32/nt_exception_filter.cpp
Posted by Evgueni Brevnov <ev...@gmail.com>.
Let's leave the -D or -X dilemma for a while. Nina Rinskya and me
found more serious problem. The problem is that debug windows popups
even if -Dvm.assert_dialog=false is specified. It happens because each
dll has its own copy of the variable which controls the behaviour of
an assert macro. In other words when you change the report mode by
calling _CrtSetReportMode it affect the current dll only. So if
assertion fails in any other dll we will see debug window.
Any ideas how to fix that?
Thanks
Evgueni
On 11/29/06, Geir Magnusson Jr. <ge...@pobox.com> wrote:
>
>
> Evgueni Brevnov wrote:
> > On 11/28/06, Gregory Shimansky <gs...@gmail.com> wrote:
> >> Geir Magnusson Jr. wrote:
> >>
> >> > Gregory Shimansky wrote:
> >> >> Gregory Shimansky wrote:
> >> >>>>> 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.
> >> >>>>
> >> >>>> Despite the fact that "vm.assert_dialog" check was removed, the
> >> support
> >> >>>> is still there: "vm.assert_dialog" controls the behaviour of the
> >> >>>> default
> >> >>>> handler.
> >> >>>>
> >> >>>> However, you are right that now the overall behaviour is determined
> >> >>>> by the launcher, so we need to change launcher correspondingly.
> >> >>>
> >> >>> Shall we agree that launcher should parse and handle this property?
> >> >>> The name doesn't look very good to me (Geir will surely ask to change
> >> >>> it).
> >> >>>
> >> >>> What about vm.windows.crt.debug.dialog ? AFAIK the property has no
> >> >>> effect on linux, and the word "assert" doesn't reflect the purpose of
> >> >>> this property too.
> >> >>
> >> >> Now that I thought about it some more, what about controlling the
> >> >> launcher default crash handler on all platforms? Specifying something
> >> >> like -Dhy.launcher.crash.handler=false to disable crash handler
> >> >> installation in launcher.
> >> >
> >> > I like it, except for the preceeding "hy"... also, will this be used by
> >> > any part of the system other than the launcher?
> >>
> >> I don't think it will be used anywhere except for the launcher. It is
> >> launcher specific and actually Harmony launcher specific.
> >
> > On Windows except crash handler you need to change the default
> > behaviour of _CrtDbgReport so it doesn't popup debug window. Currently
> > it's the VM how changes this behaviour. Do you propose to move this
> > functionality into launcher as well?
> >
> >>
> >> Anyway, if you don't mind, I'll try to change the launcher code to use
> >> "launcher.enable.crash.handler" which should be true by default.
> >
> > How about using -X<option> instead of -D<option>?
>
> -X ? That's for VM...
>
> geir
>
>
>
Re: svn commit: r479802 - /harmony/enhanced/drlvm/trunk/vm/vmcore/src/util/win/ia32/nt_exception_filter.cpp
Posted by "Geir Magnusson Jr." <ge...@pobox.com>.
Evgueni Brevnov wrote:
> On 11/28/06, Gregory Shimansky <gs...@gmail.com> wrote:
>> Geir Magnusson Jr. wrote:
>>
>> > Gregory Shimansky wrote:
>> >> Gregory Shimansky wrote:
>> >>>>> 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.
>> >>>>
>> >>>> Despite the fact that "vm.assert_dialog" check was removed, the
>> support
>> >>>> is still there: "vm.assert_dialog" controls the behaviour of the
>> >>>> default
>> >>>> handler.
>> >>>>
>> >>>> However, you are right that now the overall behaviour is determined
>> >>>> by the launcher, so we need to change launcher correspondingly.
>> >>>
>> >>> Shall we agree that launcher should parse and handle this property?
>> >>> The name doesn't look very good to me (Geir will surely ask to change
>> >>> it).
>> >>>
>> >>> What about vm.windows.crt.debug.dialog ? AFAIK the property has no
>> >>> effect on linux, and the word "assert" doesn't reflect the purpose of
>> >>> this property too.
>> >>
>> >> Now that I thought about it some more, what about controlling the
>> >> launcher default crash handler on all platforms? Specifying something
>> >> like -Dhy.launcher.crash.handler=false to disable crash handler
>> >> installation in launcher.
>> >
>> > I like it, except for the preceeding "hy"... also, will this be used by
>> > any part of the system other than the launcher?
>>
>> I don't think it will be used anywhere except for the launcher. It is
>> launcher specific and actually Harmony launcher specific.
>
> On Windows except crash handler you need to change the default
> behaviour of _CrtDbgReport so it doesn't popup debug window. Currently
> it's the VM how changes this behaviour. Do you propose to move this
> functionality into launcher as well?
>
>>
>> Anyway, if you don't mind, I'll try to change the launcher code to use
>> "launcher.enable.crash.handler" which should be true by default.
>
> How about using -X<option> instead of -D<option>?
-X ? That's for VM...
geir
Re: svn commit: r479802 - /harmony/enhanced/drlvm/trunk/vm/vmcore/src/util/win/ia32/nt_exception_filter.cpp
Posted by Evgueni Brevnov <ev...@gmail.com>.
On 11/28/06, Gregory Shimansky <gs...@gmail.com> wrote:
> Geir Magnusson Jr. wrote:
>
> > Gregory Shimansky wrote:
> >> Gregory Shimansky wrote:
> >>>>> 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.
> >>>>
> >>>> Despite the fact that "vm.assert_dialog" check was removed, the support
> >>>> is still there: "vm.assert_dialog" controls the behaviour of the
> >>>> default
> >>>> handler.
> >>>>
> >>>> However, you are right that now the overall behaviour is determined
> >>>> by the launcher, so we need to change launcher correspondingly.
> >>>
> >>> Shall we agree that launcher should parse and handle this property?
> >>> The name doesn't look very good to me (Geir will surely ask to change
> >>> it).
> >>>
> >>> What about vm.windows.crt.debug.dialog ? AFAIK the property has no
> >>> effect on linux, and the word "assert" doesn't reflect the purpose of
> >>> this property too.
> >>
> >> Now that I thought about it some more, what about controlling the
> >> launcher default crash handler on all platforms? Specifying something
> >> like -Dhy.launcher.crash.handler=false to disable crash handler
> >> installation in launcher.
> >
> > I like it, except for the preceeding "hy"... also, will this be used by
> > any part of the system other than the launcher?
>
> I don't think it will be used anywhere except for the launcher. It is
> launcher specific and actually Harmony launcher specific.
On Windows except crash handler you need to change the default
behaviour of _CrtDbgReport so it doesn't popup debug window. Currently
it's the VM how changes this behaviour. Do you propose to move this
functionality into launcher as well?
>
> Anyway, if you don't mind, I'll try to change the launcher code to use
> "launcher.enable.crash.handler" which should be true by default.
How about using -X<option> instead of -D<option>?
Evgueni
>
> --
> Gregory
>
>
Re: svn commit: r479802 - /harmony/enhanced/drlvm/trunk/vm/vmcore/src/util/win/ia32/nt_exception_filter.cpp
Posted by "Geir Magnusson Jr." <ge...@pobox.com>.
I'm in the middle of moving launcher - I guess I can just refresh again
later...
Gregory Shimansky wrote:
> Geir Magnusson Jr. wrote:
>
>> Gregory Shimansky wrote:
>>> Gregory Shimansky wrote:
>>>>>> 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.
>>>>>
>>>>> Despite the fact that "vm.assert_dialog" check was removed, the
>>>>> support
>>>>> is still there: "vm.assert_dialog" controls the behaviour of the
>>>>> default
>>>>> handler.
>>>>>
>>>>> However, you are right that now the overall behaviour is determined
>>>>> by the launcher, so we need to change launcher correspondingly.
>>>>
>>>> Shall we agree that launcher should parse and handle this property?
>>>> The name doesn't look very good to me (Geir will surely ask to
>>>> change it).
>>>>
>>>> What about vm.windows.crt.debug.dialog ? AFAIK the property has no
>>>> effect on linux, and the word "assert" doesn't reflect the purpose
>>>> of this property too.
>>>
>>> Now that I thought about it some more, what about controlling the
>>> launcher default crash handler on all platforms? Specifying something
>>> like -Dhy.launcher.crash.handler=false to disable crash handler
>>> installation in launcher.
>>
>> I like it, except for the preceeding "hy"... also, will this be used
>> by any part of the system other than the launcher?
>
> I don't think it will be used anywhere except for the launcher. It is
> launcher specific and actually Harmony launcher specific.
>
> Anyway, if you don't mind, I'll try to change the launcher code to use
> "launcher.enable.crash.handler" which should be true by default.
>
Re: svn commit: r479802 - /harmony/enhanced/drlvm/trunk/vm/vmcore/src/util/win/ia32/nt_exception_filter.cpp
Posted by Gregory Shimansky <gs...@gmail.com>.
Geir Magnusson Jr. wrote:
> Gregory Shimansky wrote:
>> Gregory Shimansky wrote:
>>>>> 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.
>>>>
>>>> Despite the fact that "vm.assert_dialog" check was removed, the support
>>>> is still there: "vm.assert_dialog" controls the behaviour of the
>>>> default
>>>> handler.
>>>>
>>>> However, you are right that now the overall behaviour is determined
>>>> by the launcher, so we need to change launcher correspondingly.
>>>
>>> Shall we agree that launcher should parse and handle this property?
>>> The name doesn't look very good to me (Geir will surely ask to change
>>> it).
>>>
>>> What about vm.windows.crt.debug.dialog ? AFAIK the property has no
>>> effect on linux, and the word "assert" doesn't reflect the purpose of
>>> this property too.
>>
>> Now that I thought about it some more, what about controlling the
>> launcher default crash handler on all platforms? Specifying something
>> like -Dhy.launcher.crash.handler=false to disable crash handler
>> installation in launcher.
>
> I like it, except for the preceeding "hy"... also, will this be used by
> any part of the system other than the launcher?
I don't think it will be used anywhere except for the launcher. It is
launcher specific and actually Harmony launcher specific.
Anyway, if you don't mind, I'll try to change the launcher code to use
"launcher.enable.crash.handler" which should be true by default.
--
Gregory
Re: svn commit: r479802 - /harmony/enhanced/drlvm/trunk/vm/vmcore/src/util/win/ia32/nt_exception_filter.cpp
Posted by "Geir Magnusson Jr." <ge...@pobox.com>.
Gregory Shimansky wrote:
> Gregory Shimansky wrote:
>>>> 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.
>>>
>>> Despite the fact that "vm.assert_dialog" check was removed, the support
>>> is still there: "vm.assert_dialog" controls the behaviour of the default
>>> handler.
>>>
>>> However, you are right that now the overall behaviour is determined
>>> by the launcher, so we need to change launcher correspondingly.
>>
>> Shall we agree that launcher should parse and handle this property?
>> The name doesn't look very good to me (Geir will surely ask to change
>> it).
>>
>> What about vm.windows.crt.debug.dialog ? AFAIK the property has no
>> effect on linux, and the word "assert" doesn't reflect the purpose of
>> this property too.
>
> Now that I thought about it some more, what about controlling the
> launcher default crash handler on all platforms? Specifying something
> like -Dhy.launcher.crash.handler=false to disable crash handler
> installation in launcher.
I like it, except for the preceeding "hy"... also, will this be used by
any part of the system other than the launcher?
geir
Re: svn commit: r479802 - /harmony/enhanced/drlvm/trunk/vm/vmcore/src/util/win/ia32/nt_exception_filter.cpp
Posted by Gregory Shimansky <gs...@gmail.com>.
Gregory Shimansky wrote:
>>> 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.
>>
>> Despite the fact that "vm.assert_dialog" check was removed, the support
>> is still there: "vm.assert_dialog" controls the behaviour of the default
>> handler.
>>
>> However, you are right that now the overall behaviour is determined
>> by the launcher, so we need to change launcher correspondingly.
>
> Shall we agree that launcher should parse and handle this property? The
> name doesn't look very good to me (Geir will surely ask to change it).
>
> What about vm.windows.crt.debug.dialog ? AFAIK the property has no
> effect on linux, and the word "assert" doesn't reflect the purpose of
> this property too.
Now that I thought about it some more, what about controlling the
launcher default crash handler on all platforms? Specifying something
like -Dhy.launcher.crash.handler=false to disable crash handler
installation in launcher.
--
Gregory
Re: svn commit: r479802 - /harmony/enhanced/drlvm/trunk/vm/vmcore/src/util/win/ia32/nt_exception_filter.cpp
Posted by Gregory Shimansky <gs...@gmail.com>.
Salikh Zakirov wrote:
> Alexey Varlamov 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?
>
>
> This is in ia32/nt_exception_filter.cpp, so no portability concerns.
> I agree that we may want to generalize it multiple architectures in the future,
> but it wasn't my concern now.
Agreed. The register names are ia32 specific too, so 32-bitness is not
the only unportable part of the file.
>> 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.
>
> Despite the fact that "vm.assert_dialog" check was removed, the support
> is still there: "vm.assert_dialog" controls the behaviour of the default
> handler.
>
> However, you are right that now the overall behaviour is determined
> by the launcher, so we need to change launcher correspondingly.
Shall we agree that launcher should parse and handle this property? The
name doesn't look very good to me (Geir will surely ask to change it).
What about vm.windows.crt.debug.dialog ? AFAIK the property has no
effect on linux, and the word "assert" doesn't reflect the purpose of
this property too.
--
Gregory
Re: svn commit: r479802 - /harmony/enhanced/drlvm/trunk/vm/vmcore/src/util/win/ia32/nt_exception_filter.cpp
Posted by Salikh Zakirov <Sa...@Intel.com>.
Alexey Varlamov 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?
This is in ia32/nt_exception_filter.cpp, so no portability concerns.
I agree that we may want to generalize it multiple architectures in the future,
but it wasn't my concern now.
> 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.
Despite the fact that "vm.assert_dialog" check was removed, the support
is still there: "vm.assert_dialog" controls the behaviour of the default
handler.
However, you are right that now the overall behaviour is determined
by the launcher, so we need to change launcher correspondingly.