harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Evgueni Brevnov" <evgueni.brev...@gmail.com>
Subject Re: svn commit: r479802 - /harmony/enhanced/drlvm/trunk/vm/vmcore/src/util/win/ia32/nt_exception_filter.cpp
Date Tue, 28 Nov 2006 13:38:17 GMT
On 11/28/06, Alexey Varlamov <alexey.v.varlamov@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 <gshimansky@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, &regs);
> > -            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(&regs);
> >             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, &regs);
> > +    // 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(&regs, exc_clss, java_code);
> > +    M2nFrame* prev_m2n = m2n_get_last_frame();
> > +    M2nFrame* m2n = NULL;
> > +    if (in_java)
> > +        m2n = m2n_push_suspended_frame(&regs);
> > +
> > +    TRACE2("signals", ("should throw exception %p at EIP=%p, ESP=%p",
> > +                exn_class, regs.eip, regs.esp));
> > +    exn_athrow_regs(&regs, 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(&regs, context);
> > -
> > -    return EXCEPTION_CONTINUE_EXECUTION;
> > -} //vectored_exception_handler
> > +    StackIterator *si =
> > +        si_create_from_registers(&regs, false, prev_m2n);
> > +    if (m2n)
> > +        STD_FREE(m2n);
> > +    si_transfer_control(si);
> > +    assert(!"si_transfer_control should not return");
> > +}
> >
> >
> >
>

Mime
View raw message