bytecode rewrite and breakpoint

[Coleen Phillimore]

Hi,

This is an interesting question actually.

On 1/12/15, 7:33 AM, San Hong Li wrote:
> Hi Coleen:
> Thanks for your attention and reply. I would like to clarify my 
> question again:
> Given that we have the following bytecde stream:
> |bp|
> -------------------------------------------------
> | x | x   | aload_0 | x | x | x |
> -------------------------------------------------
> and we suppose that the breakpoint gets inserted  at  the position of 
> "aload_0 ",   the steam would be updated as follows:
>
> -------------------------------------------------
> | x | x   | bp(202)| x | x | x |
> -------------------------------------------------
>          (original: aload_0 )
>
> The key point here is : bp and aload_0 have the same bcp index -  I 
> just deduced this from the implementation of TemplateTable::_*breakpoint.*
> *
> *
> *
> *
> Based on this layout,  when  the *TemplateTable::patch_bytecode *was 
> called for **aload_0
> The interpreter will see that the bp will be at bcp(0)   (*NOT aload_0 
> *at this time)

If we are executing the code TemplateTable::patch_bytecode, then the 
bytecode stream had an aload_0 in it *not* _bp, so I'm not sure why we'd 
all of a sudden find a _bp in the bytecode stream at this point.  The 
JVMTI set_breakpoint code seems to only set breakpoints at safepoints, 
and while there's a safepoint at each bytecode, there is not one after 
we've started executing the bytecode in the template interpreter.  It 
appears that a different thread cannot change the bytecode to _bp after 
the _aload_0 is read from the stream.

So I don't see how we need this code.  It's been there forever. Maybe 
the breakpoint code used to work differently in the past which made this 
code necessary.  But a word of warning, this is just my reading of this 
today, I could be surprised by some other reason that this code is needed.

Did I understand your question this time?  It's a good one that I don't 
know the answer to.

Thanks,
Coleen

>
> This explained that why the HotSpot must have the  below block in 
> *TemplateTable::patch_bytecode *
> to handle the case when the  at bcp(0) is bp.
> *(Actually this was the intention of my original question)*
>
>   if (JvmtiExport::can_post_breakpoint()) {
>     Label L_fast_patch;
>     // if a breakpoint is present we can't rewrite the stream directly
>     __ movzbl(temp_reg, at_bcp(0));
>     __ cmpl(temp_reg, Bytecodes::_breakpoint);
>     __ jcc(Assembler::notEqual, L_fast_patch);
>     __ get_method(temp_reg);
>     // Let breakpoint table handling rewrite to quicker bytecode
>     __ call_VM(noreg, CAST_FROM_FN_PTR(address,
> InterpreterRuntime::set_original_bytecode_at), temp_reg, r13, bc_reg);
> #ifndef ASSERT
>     __ jmpb(L_patch_done);
> #else
>     __ jmp(L_patch_done);
> #endif
>     __ bind(L_fast_patch);
>   }
>
> Hopefully I  explain my question and answer clearly  :-)
>
> On Fri, Jan 9, 2015 at 11:35 AM, San Hong Li <sherrylso at gmail.com 
> <mailto:sherrylso at gmail.com>> wrote:
>
>     Hmmm, finally I got the answer for this question.
>     The  at_bcp(0) would be the Bytecodes::_breakpoint if the
>     patch_code happened after the  break point is set.
>     The following example shows this:
>
>        33:  getstatic       #8; //Field
>     java/lang/System.in:Ljava/io/InputStream;
>        36:  invokevirtual   #9; //Method java/io/InputStream.read:()I
>        39:  pop
>        40:  aload_0  <---------------------- set break point here.
>        41:  getfield        #2; //Field myInt:I
>        44:  istore  6
>        46:  iload   6
>
>     If we  set the breakpoint at 40 , when the  patch_code  in aload
>      happened later, the interpreter will see the
>     Bytecodes::_breakpoint  at bcp(0).
>
>     hmmm, actually i just missed the info present
>     in TemplateTable::_breakpoint - the breakpoint  used the same
>     index with the original bytecode.
>
>     Anyway, thanks every one.
>
>     On Tue, Jan 6, 2015 at 10:18 PM, San Hong Li <sherrylso at gmail.com
>     <mailto:sherrylso at gmail.com>> wrote:
>
>         Hi All:
>
>         For the bytecode patch path, the  interpreter does have some
>         special handling for breakpoint bytecode, that is,  if a
>         breakpoint is present at_bcp(0),   the bytecode patching will
>         be skipped and  handled by breakpoint table later.
>
>         The related implementation is in  TemplateTable::patch_bytecode:
>
>           if (JvmtiExport::can_post_breakpoint()) {
>             Label L_fast_patch;
>             // if a breakpoint is present we can't rewrite the stream
>         directly
>             __ movzbl(temp_reg, at_bcp(0));
>             __ cmpl(temp_reg, Bytecodes::_breakpoint);
>             __ jcc(Assembler::notEqual, L_fast_patch);
>             __ get_method(temp_reg);
>             // Let breakpoint table handling rewrite to quicker bytecode
>             __ call_VM(noreg, CAST_FROM_FN_PTR(address,
>         InterpreterRuntime::set_original_bytecode_at), temp_reg, r13,
>         bc_reg);
>         #ifndef ASSERT
>             __ jmpb(L_patch_done);
>         #else
>             __ jmp(L_patch_done);
>         #endif
>             __ bind(L_fast_patch);
>           }
>
>         My understanding is the breakpoint can only be set in safepoint,
>         when the interpreter thread resumes from safepoint, because it
>         will retrieve the next bytecode for executing again，the
>         breakpoint bytecode which is set in safepoint before will get
>         chance to be executed. I can not imagine when the bcp(0) could
>         be changed as breakpoint in above code?
>         So I am curious whether the above code is already dead?
>         If not,  anyone can help to me to clarify that  how the
>         bytecode which is currently executed by interpreter could be
>         replaced with breakpoint?   which case could trigger  the
>         "InterpreterRuntime::set_original_bytecode_at" to be called?
>
>         Appreciated for your help in advance.
>
>
>