harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gregory Shimansky <gshiman...@apache.org>
Subject Re: [drlvm][regression] jvmti single step is broken
Date Sat, 20 Oct 2007 13:03:56 GMT
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


Mime
View raw message