You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@harmony.apache.org by Alexei Fedotov <al...@gmail.com> on 2007/10/19 21:14:53 UTC
[drlvm][regression] jvmti single step is broken
Hello Gregory,
I have filed a bug [1] which may be caused by your recent changes.
Thanks.
[1] http://issues.apache.org/jira/browse/HARMONY-4981
On 10/19/07, gshimansky@apache.org <gs...@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, ®s);
> 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;
> }
>
>
>
>
--
With best regards,
Alexei,
ESSD, Intel
Re: [drlvm][regression] jvmti single step is broken
Posted by Alexei Fedotov <al...@gmail.com>.
Gregory,
Thank you for a quick resolution.
On 10/20/07, Gregory Shimansky <gs...@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 <gs...@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, ®s);
> >>> 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
Re: [drlvm][regression] jvmti single step is broken
Posted by Gregory Shimansky <gs...@apache.org>.
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 <gs...@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, ®s);
>>> 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
Re: [drlvm][regression] jvmti single step is broken
Posted by Gregory Shimansky <gs...@apache.org>.
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 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 <gs...@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, ®s);
>> 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