[javaflow] instrumenting 'NEW' op

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[javaflow] instrumenting 'NEW' op

Kohsuke Kawaguchi

I've been playing with javaflow and noticed the following.

Suppose I have the following code:

        File root = new File("data");

This compiles into the following bytecode:

     // local variable 0 is "this"
     // local variable 1 is "root"

     0  new java/io/File
     3  dup
     4  ldc "data"
     6  invokespecial java/io/File/<init>(Ljava/lang/String;)V
     9  astore_1

After the instrumentation, this code expands to:

     118  ldc "data"
     120  aload 2
     122  swap
     123  invokevirtual o/a/c/j/b/StackRecorder/pushObject...
     126  new java/io/File
     129  dup
     130  aload 2
     132  invokevirtual o/a/c/j/b/StackRecorder/popObject...
     135  checkcast java/lang/String
     138  invokespecial java/io/File/<init>(Ljava/lang/String;)V
     141  astore_1

I don't think I understand the instrumentation logic completely, but
from a cursory look, the idea is to evaluate constructor parameters
before the 'new' op (and StackRecorder is used as a temporary place to
store evaluated objects.)

This is actually causing an NPE if I run the above code outside the
continuation environment (because StackRecorder is null.)

So here goes my questions:

Why is this necessary? I'd imagine it's related to restoring/capturing
stack frames when Continuation.suspend is invoked inside a constructor,
but it's not clear to me why we need to handle 'new's/'invokespecial's
differently from, say, 'invokevirtual'.

Also, I noticed that the BcelClassTransformer isn't actually generating
the stack capture/restore code for invokespecial. Is that a TODO? Is
this related to the following line in the TODO file?

>   o fix unintialized objects for method/constructor calls



--
Kohsuke Kawaguchi

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [javaflow] instrumenting 'NEW' op

Torsten Curdt
>
> I don't think I understand the instrumentation logic completely,  
> but from a cursory look, the idea is to evaluate constructor  
> parameters before the 'new' op (and StackRecorder is used as a  
> temporary place to store evaluated objects.)

That's true ...Stephan introduced that
to get rid of uninitialized objects on
the stack.

TBH I would like to find a different
way of doing that. Maybe we can save
the type of the object on the stack
and then create a new uninitialized
object on the restore.

>
> This is actually causing an NPE if I run the above code outside the  
> continuation environment (because StackRecorder is null.)

Well ...usually you should not run
this code outside the continuation
environment. Maybe we should try to
force that. So far that was the most
common problem people had getting
started with javaflow.

> Why is this necessary? I'd imagine it's related to restoring/
> capturing stack frames when Continuation.suspend is invoked inside  
> a constructor,

Yepp :)

> but it's not clear to me why we need to handle  
> 'new's/'invokespecial's differently from, say, 'invokevirtual'.

Hmmmm... not sure if we need to.
Usually I would say we don't need
to treat it differently, too.

> Also, I noticed that the BcelClassTransformer isn't actually  
> generating the stack capture/restore code for invokespecial. Is  
> that a TODO? Is this related to the following line in the TODO file?
>
>
>>   o fix unintialized objects for method/constructor calls

Yes, that's about that the "new" operator.

cheers
--
Torsten

PGP.sig (193 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [javaflow] instrumenting 'NEW' op

Kohsuke Kawaguchi-2
Torsten Curdt wrote:

>>
>> I don't think I understand the instrumentation logic completely,  
>> but from a cursory look, the idea is to evaluate constructor  
>> parameters before the 'new' op (and StackRecorder is used as a  
>> temporary place to store evaluated objects.)
>
> That's true ...Stephan introduced that
> to get rid of uninitialized objects on
> the stack.
>
> TBH I would like to find a different
> way of doing that. Maybe we can save
> the type of the object on the stack
> and then create a new uninitialized
> object on the restore.
I read the JVM spec and I understand the interaction between the
verifier and the uninitialized objects better. So I think I now
understand the reasoning behind this. When you got a Java code like this:

String v = new String(computeString());

String computeString() {
   Continuation.suspend();
   return "abc";
}

then the stack capturing gets problematic because the stack contains
uninitialized object. The transformer was trying to avoid this issue by
performing evaluations of parameters before the 'new' instruction.

Like you said, I think it's nicer to change this so that the capturing
would simply discard the uninitialized object and the restoration
creates a new instance. We can infer the type of the object that needs
to be 'new'ed from the byte code, so that information doesn't have to be
saved in the StackRecorder.

I'm interested in fixing this, if that's OK with you.


I also learned that the byte code verifier ensures that the constructor
can be only invoked on an uninitialized object. I guess this means that
we can't suspend if the call stack includes a constructor. This seems
like a rather serious limitation.

Did this limitation apply to the BRAKES project? Or did they find a way
to work around this? like maybe by copying constructor into an ordinary
method?


--
Kohsuke Kawaguchi
Sun Microsystems                   [hidden email]

smime.p7s (4K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [javaflow] instrumenting 'NEW' op

Kohsuke Kawaguchi
Kohsuke Kawaguchi wrote:

> Torsten Curdt wrote:
>>>
>>> I don't think I understand the instrumentation logic completely,  
>>> but from a cursory look, the idea is to evaluate constructor  
>>> parameters before the 'new' op (and StackRecorder is used as a  
>>> temporary place to store evaluated objects.)
>>
>> That's true ...Stephan introduced that
>> to get rid of uninitialized objects on
>> the stack.
>>
>> TBH I would like to find a different
>> way of doing that. Maybe we can save
>> the type of the object on the stack
>> and then create a new uninitialized
>> object on the restore.
>
> I read the JVM spec and I understand the interaction between the
> verifier and the uninitialized objects better. So I think I now
> understand the reasoning behind this. When you got a Java code like this:
>
> String v = new String(computeString());
>
> String computeString() {
>    Continuation.suspend();
>    return "abc";
> }
>
> then the stack capturing gets problematic because the stack contains
> uninitialized object. The transformer was trying to avoid this issue by
> performing evaluations of parameters before the 'new' instruction.
>
> Like you said, I think it's nicer to change this so that the capturing
> would simply discard the uninitialized object and the restoration
> creates a new instance. We can infer the type of the object that needs
> to be 'new'ed from the byte code, so that information doesn't have to be
> saved in the StackRecorder.
>
> I'm interested in fixing this, if that's OK with you.

I think I realized the problem with this approach. The code below
wouldn't pass the verifier even though it's safe.


    ldc 0
    ifeq x

    new java/lang/String
    goto y

x:
    new java/lang/String

y:
    invokespecial java/lang/String/<init>()V

When the stack operand type is compared for two incoming control flows
to "y:", those two uninitialized String types are considered different.

I also checked the BRAKES project and they do the transformation similar
to the current transformation of javaflow (evaluate arguments before
'new') The only difference is that they use local variables to store
evaluated results.

I have to say ... the bytecode stuff is dense!

--
Kohsuke Kawaguchi

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]