harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alexei Fedotov" <alexei.fedo...@gmail.com>
Subject Re: [drlvm][regression] jvmti single step is broken
Date Mon, 22 Oct 2007 16:31:17 GMT
Gregory,
Thank you for a quick resolution.

On 10/20/07, Gregory Shimansky <gshimansky@apache.org> wrote:
> Gregory Shimansky wrote:
> > Alexei Fedotov wrote:
> >> Hello Gregory,
> >> I have filed a bug [1] which may be caused by your recent changes.
> >
> > Hello Alexei. Thanks for noticing this. I am indeed in the the process
> > of changing JVMTI single step code, and this assertion failure did
> > happen because of my recent changes. I changed the prediction code for
> > invokevirtual and invokeinterface bytecodes.
> >
> > Previously JVMTI code contained a switch based on the call instruction's
> > 1st operand kind. If it was Kind_Mem, then the bytecode was treated as
> > invokevirtual. If instruction was Kind_Reg, then it was treated as
> > invokeinterface. But I changed the code to actually taking bytecode in
> > question from the method's bytecode array and switch now knows exactly
> > what type of invoke is in the method.
> >
> > For invokevirtual bytecode on x86 it is expected to have a call[base +
> > index*scale + disp] instruction that calls a virtual method, where
> > (base) contains the address of VTable, and (index*scale + disp) contain
> > offset in vtable (disp is usually null in the code generated by JET).
> >
> > But here is some strange situation, and I'd like to hear JIT gurus to
> > tell me what has happened. The method is
> >
> > org/apache/harmony/test/stress/jpda/jdwp/share/JDWPQALogWriter.println(Ljava/lang/String;)V
> >
> >
> > Its bytecode 15 looks like this in java:
> >
> >  15: invokevirtual #6=<Method java.lang.StringBuilder.append
> > (java.lang.String)java.lang.StringBuilder>
> >
> > But generated code this bytecode looks like this:
> >
> > 0282E164  mov         edx,dword ptr [ebp-0F0h]
> > 0282E16A  sub         esp,0Ch
> > 0282E16D  mov         dword ptr [esp],2C719A8h
> > 0282E174  mov         dword ptr [esp+4],6
> > 0282E17C  mov         dword ptr [esp+8],edx
> > 0282E180  mov         dword ptr [ebp-0F0h],edx
> > 0282E186  mov         dword ptr [ebp-0D4h],3
> > 0282E190  mov         dword ptr [ebp-0E0h],7
> > 0282E19A  mov         eax,12D1620h
> > 0282E19F  call        eax
> > 0282E1A1  sub         esp,8
> > 0282E1A4  mov         ecx,dword ptr [ebp-0F4h]
> > 0282E1AA  mov         dword ptr [esp],ecx
> > 0282E1AD  mov         ecx,dword ptr [ebp-0F0h]
> > 0282E1B3  mov         dword ptr [esp+4],ecx
> > 0282E1B7  mov         dword ptr [ebp-0D4h],1
> > 0282E1C1  mov         dword ptr [ebp-0E0h],1
> > 0282E1CB  mov         ecx,dword ptr [eax]
> > 0282E1CD  call        ecx
> > 0282E1CF  mov         dword ptr [ebp-0F0h],eax
> > 0282E1D5  mov         dword ptr [ebp-0D4h],2
> > 0282E1DF  mov         dword ptr [ebp-0E0h],3
> >
> > The addresses are generated by jvmti_dump_compiled_method function that
> > outputs this map:
> >
> > ...
> > %ecnfk@1192882950836| STDERR> bytecode 7: 15 = 0282E164
> > %ecnfk@1192882950836| STDERR>
> > %ecnfk@1192882950836| STDERR> bytecode 8: 18 = 0282E1E9
> > ...
> >
> > In the above assembly I see two calls, both of type Kind_Reg. But this
> > code is compiled for invokevirtual. Before my change this code would be
> > treated as invokeinterface, and an attempt would be to find a compiled
> > method based on the value of ECX.
>
> I made a fix at 586707 to treat all calls through a register as direct
> calls to method's compiled code. This means that register calls cannot
> be encountered to call a stub for method's compilation or resolution.
> The address has to be pointing to method's body. Otherwise it isn't
> possible for JVMTI code to figure out exactly what method is called by
> invokevirtual bytecode.
>
> > I wonder how to get VTable and offset out of the above code, and why is
> > the difference from the usual call [base + index*scale + disp]?
> >
> >> [1] http://issues.apache.org/jira/browse/HARMONY-4981
> >>
> >> On 10/19/07, gshimansky@apache.org <gshimansky@apache.org> wrote:
> >>> Author: gshimansky
> >>> Date: Fri Oct 19 04:09:47 2007
> >>> New Revision: 586379
> >>>
> >>> URL: http://svn.apache.org/viewvc?rev=586379&view=rev
> >>> Log:
> >>> Improved JVMTI for handling invokevirtual and invokeinterface on
> >>> x86_64 architecture.
> >>> Capabilities for breakpoint and single step are turned on.
> >>>
> >>>
> >>> Modified:
> >>>    harmony/enhanced/drlvm/trunk/vm/vmcore/include/jvmti_break_intf.h
> >>>    harmony/enhanced/drlvm/trunk/vm/vmcore/include/jvmti_internal.h
> >>>    harmony/enhanced/drlvm/trunk/vm/vmcore/src/jvmti/jvmti_break.cpp
> >>>    harmony/enhanced/drlvm/trunk/vm/vmcore/src/jvmti/jvmti_capability.cpp
> >>>    harmony/enhanced/drlvm/trunk/vm/vmcore/src/jvmti/jvmti_step.cpp
> >>>
> >>> Modified:
> >>> harmony/enhanced/drlvm/trunk/vm/vmcore/include/jvmti_break_intf.h
> >>> URL:
> >>> http://svn.apache.org/viewvc/harmony/enhanced/drlvm/trunk/vm/vmcore/include/jvmti_break_intf.h?rev=586379&r1=586378&r2=586379&view=diff
> >>>
> >>> ==============================================================================
> >>>
> >>> --- harmony/enhanced/drlvm/trunk/vm/vmcore/include/jvmti_break_intf.h
> >>> (original)
> >>> +++ harmony/enhanced/drlvm/trunk/vm/vmcore/include/jvmti_break_intf.h
> >>> Fri Oct 19 04:09:47 2007
> >>> @@ -78,7 +78,7 @@
> >>>  };
> >>>
> >>>  // Pointer to interface callback function
> >>> -typedef bool (*BPInterfaceCallBack)(TIEnv *env, VMBreakPoint* bp,
> >>> POINTER_SIZE_INT data);
> >>> +typedef bool (*BPInterfaceCallBack)(TIEnv *env, const VMBreakPoint*
> >>> bp, const POINTER_SIZE_INT data);
> >>>  typedef bool (*BPInterfaceProcedure) (VMBreakPoint *bp);
> >>>
> >>>  class VMBreakPoints
> >>>
> >>> Modified:
> >>> harmony/enhanced/drlvm/trunk/vm/vmcore/include/jvmti_internal.h
> >>> URL:
> >>> http://svn.apache.org/viewvc/harmony/enhanced/drlvm/trunk/vm/vmcore/include/jvmti_internal.h?rev=586379&r1=586378&r2=586379&view=diff
> >>>
> >>> ==============================================================================
> >>>
> >>> --- harmony/enhanced/drlvm/trunk/vm/vmcore/include/jvmti_internal.h
> >>> (original)
> >>> +++ harmony/enhanced/drlvm/trunk/vm/vmcore/include/jvmti_internal.h
> >>> Fri Oct 19 04:09:47 2007
> >>> @@ -423,6 +423,6 @@
> >>>     unsigned location, jvmti_StepLocation **next_step, unsigned *count);
> >>>
> >>>  // Callback function for JVMTI breakpoint processing
> >>> -bool jvmti_process_breakpoint_event(TIEnv *env, VMBreakPoint* bp,
> >>> POINTER_SIZE_INT data);
> >>> +bool jvmti_process_breakpoint_event(TIEnv *env, const VMBreakPoint*
> >>> bp, const POINTER_SIZE_INT data);
> >>>
> >>>  #endif /* _JVMTI_INTERNAL_H_ */
> >>>
> >>> Modified:
> >>> harmony/enhanced/drlvm/trunk/vm/vmcore/src/jvmti/jvmti_break.cpp
> >>> URL:
> >>> http://svn.apache.org/viewvc/harmony/enhanced/drlvm/trunk/vm/vmcore/src/jvmti/jvmti_break.cpp?rev=586379&r1=586378&r2=586379&view=diff
> >>>
> >>> ==============================================================================
> >>>
> >>> --- harmony/enhanced/drlvm/trunk/vm/vmcore/src/jvmti/jvmti_break.cpp
> >>> (original)
> >>> +++ harmony/enhanced/drlvm/trunk/vm/vmcore/src/jvmti/jvmti_break.cpp
> >>> Fri Oct 19 04:09:47 2007
> >>> @@ -39,7 +39,7 @@
> >>>
> >>>
> >>>  // Callback function for JVMTI breakpoint processing
> >>> -bool jvmti_process_breakpoint_event(TIEnv *env, VMBreakPoint* bp,
> >>> POINTER_SIZE_INT UNREF data)
> >>> +bool jvmti_process_breakpoint_event(TIEnv *env, const VMBreakPoint*
> >>> bp, const POINTER_SIZE_INT UNREF data)
> >>>  {
> >>>     assert(bp);
> >>>
> >>>
> >>> Modified:
> >>> harmony/enhanced/drlvm/trunk/vm/vmcore/src/jvmti/jvmti_capability.cpp
> >>> URL:
> >>> http://svn.apache.org/viewvc/harmony/enhanced/drlvm/trunk/vm/vmcore/src/jvmti/jvmti_capability.cpp?rev=586379&r1=586378&r2=586379&view=diff
> >>>
> >>> ==============================================================================
> >>>
> >>> ---
> >>> harmony/enhanced/drlvm/trunk/vm/vmcore/src/jvmti/jvmti_capability.cpp
> >>> (original)
> >>> +++
> >>> harmony/enhanced/drlvm/trunk/vm/vmcore/src/jvmti/jvmti_capability.cpp
> >>> Fri Oct 19 04:09:47 2007
> >>> @@ -87,10 +87,10 @@
> >>>     1, // can_get_source_debug_extension
> >>>     1, // can_access_local_variables
> >>>     0, // can_maintain_original_method_order
> >>> -    0, // can_generate_single_step_events
> >>> +    1, // can_generate_single_step_events
> >>>     1, // can_generate_exception_events
> >>>     1, // can_generate_frame_pop_events
> >>> -    0, // can_generate_breakpoint_events
> >>> +    1, // can_generate_breakpoint_events
> >>>     1, // can_suspend
> >>>     0, // can_redefine_any_class
> >>>     1, // can_get_current_thread_cpu_time
> >>>
> >>> Modified:
> >>> harmony/enhanced/drlvm/trunk/vm/vmcore/src/jvmti/jvmti_step.cpp
> >>> URL:
> >>> http://svn.apache.org/viewvc/harmony/enhanced/drlvm/trunk/vm/vmcore/src/jvmti/jvmti_step.cpp?rev=586379&r1=586378&r2=586379&view=diff
> >>>
> >>> ==============================================================================
> >>>
> >>> --- harmony/enhanced/drlvm/trunk/vm/vmcore/src/jvmti/jvmti_step.cpp
> >>> (original)
> >>> +++ harmony/enhanced/drlvm/trunk/vm/vmcore/src/jvmti/jvmti_step.cpp
> >>> Fri Oct 19 04:09:47 2007
> >>> @@ -494,8 +494,8 @@
> >>>     }
> >>>  }
> >>>
> >>> -static void jvmti_start_single_step_in_virtual_method(DebugUtilsTI
> >>> *ti, VMBreakPoint* bp,
> >>> -
> >>> POINTER_SIZE_INT data)
> >>> +static void jvmti_start_single_step_in_virtual_method(DebugUtilsTI
> >>> *ti, const VMBreakPoint* bp,
> >>> +    const POINTER_SIZE_INT data, const unsigned char bytecode)
> >>>  {
> >>>  #if (defined _IA32_) || (defined _EM64T_)
> >>>     VM_thread *vm_thread = p_TLS_vmthread;
> >>> @@ -517,11 +517,11 @@
> >>>
> >>>     const InstructionDisassembler::Opnd& op = disasm->get_opnd(0);
> >>>     Method *method;
> >>> -    if (op.kind == InstructionDisassembler::Kind_Mem)
> >>> +    if (bytecode == OPCODE_INVOKEVIRTUAL)
> >>>     {
> >>>         // Invokevirtual uses indirect call from VTable. The base
> >>>         // address is in the register, offset is in displacement *
> >>> -        // scale. This method is much faster than
> >>> +        // scale.
> >>>         VTable* vtable = (VTable*)disasm->get_reg_value(op.base, &regs);
> >>>         assert(vtable);
> >>>         // For x86 based architectures offset cannot be longer than 32
> >>> @@ -530,7 +530,7 @@
> >>>             op.scale + op.disp);
> >>>         method = class_get_method_from_vt_offset(vtable, offset);
> >>>     }
> >>> -    else if (op.kind == InstructionDisassembler::Kind_Reg)
> >>> +    else if (bytecode == OPCODE_INVOKEINTERFACE)
> >>>     {
> >>>         // This is invokeinterface bytecode which uses register
> >>>         // call so we need to search through all methods for this
> >>> @@ -577,7 +577,7 @@
> >>>
> >>>  // Callback function for JVMTI single step processing
> >>>  static bool jvmti_process_jit_single_step_event(TIEnv* UNREF
> >>> unused_env,
> >>> -                                                VMBreakPoint* bp,
> >>> POINTER_SIZE_INT data)
> >>> +    const VMBreakPoint* bp, const POINTER_SIZE_INT data)
> >>>  {
> >>>     assert(bp);
> >>>
> >>> @@ -610,7 +610,10 @@
> >>>
> >>>     if ((bool)data)
> >>>     {
> >>> -        jvmti_start_single_step_in_virtual_method(ti, bp, data);
> >>> +        const unsigned char *bytecode = reinterpret_cast<Method
> >>> *>(method)->get_byte_code_addr();
> >>> +        const unsigned char bc = bytecode[location];
> >>> +        assert(bc == OPCODE_INVOKEINTERFACE || bc ==
> >>> OPCODE_INVOKEVIRTUAL);
> >>> +        jvmti_start_single_step_in_virtual_method(ti, bp, data, bc);
> >>>         return true;
> >>>     }
> >>>
> >>>
> >>>
> >>>
> >>
> >>
> >
> >
>
>
> --
> Gregory
>
>


-- 
With best regards,
Alexei,
ESSD, Intel

Mime
View raw message