[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-runtime-dev
Subject:    =?UTF-8?Q?=E7=AD=94=E5=A4=8D:_=E7=AD=94=E5=A4=8D:_=E7=AD=94=E5=A4=8D:_?= =?UTF-8?Q?=E7=AD=94=E5=A4=8
From:       陈雨亭 <chenyt () cs ! sjtu ! edu ! cn>
Date:       2017-05-22 1:09:30
Message-ID: 000001d2d298$1275fcf0$3761f6d0$ () cs ! sjtu ! edu ! cn
[Download RAW message or body]

Thank you, David. I have noticed that the policy is nothing different from that for \
processing an athrow instruction: "Otherwise, if the Java Virtual Machine \
implementation enforces the rules on structured locking described in  2.11.10 and if \
the first of those rules is violated during invocation of the current method, then \
athrow throws an IllegalMonitorStateException instead of the object previously being \
thrown." 

I agree that it is not an issue. A quick suggestion that may help understand the \
replacement: the specification may say one more sentence (e.g., the runtime processes \
a runtime exception using the strategy for processing an athrow instruction) in \
$2.11.9. 

Regards,
Yuting


-----邮件原件-----
发件人: David Holmes [mailto:david.holmes@oracle.com] 
发送时间: 2017年5月21日 17:19
收件人: Ioi Lam <ioi.lam@oracle.com>; hotspot-runtime-dev@openjdk.java.net; \
chenyt@cs.sjtu.edu.cn 主题: Re: 答复: 答复: 答复: HotSpot and IBM's J9 behave \
quite differently when processing monitorenters and monitorexits

Ioi, Yuting,

After close examination I have closed this bug (8180615) as not an issue.

What is happening is that we have a successful monitorenter followed by an invalid \
monitorenter. The invalid monitorenter throws NPE as expected and the call stack \
unwinds. As we leave the method frame the system can see that we still have a locked \
monitor, which means we have skipped an unlock and so have violated the "structured \
locking" requirements that a VM can choose to enforce. Because of that we throw the \
IllegalMonitorStateException, which replaces the NullPointerException that was in the \
process of being thrown.

If you change the examples to only test the invalid enter/exits, and ensure all \
entered monitors are exited properly then you will see the results you expect with \
regard to the NullPointerException.

Thanks,
David

On 19/05/2017 2:47 AM, Ioi Lam wrote:
> Hi 雨亭,
> 
> Thanks for the bug report. I have created a new bug,
> https://bugs.openjdk.java.net/browse/JDK-8180615 and attached your 
> test case.
> 
> JDK-8180615 - monitorenter on null object produces unexpected 
> IllegalMonitorStateException
> 
> - Ioi
> 
> On 5/18/17 8:15 AM, 陈雨亭 wrote:
> > HotSpot still misses catching the NullPointerException for the next 
> > program (only one pair of entermonitor and monitorexit). It only 
> > performs checks when the function exits and throw an 
> > IllegalMonitorStateException. We should enumerate more cases to test 
> > the JVMs.
> > 
> > public void main(java.lang.String[]) throws java.lang.Exception;
> > descriptor: ([Ljava/lang/String;)V
> > flags: ACC_PUBLIC
> > Code:
> > stack=1, locals=2, args_size=2
> > 0: aload_0
> > 1: monitorenter
> > 2: aconst_null
> > 3: monitorexit
> > 4: return
> > Exceptions:
> > throws java.lang.Exception
> > 
> > -----邮件原件-----
> > 发件人: 陈雨亭 [mailto:chenyt@cs.sjtu.edu.cn]
> > 发送时间: 2017年5月18日 7:56
> > 收件人: 'Mikael Gerdin' <mikael.gerdin@oracle.com>; 'David Holmes' 
> > <david.holmes@oracle.com>
> > 抄送: 'hotspot-runtime-dev@openjdk.java.net' 
> > <hotspot-runtime-dev@openjdk.java.net>
> > 主题: 答复: 答复: 答复: HotSpot and IBM's J9 behave quite differently \
> > when  processing monitorenters and monitorexits
> > 
> > Is it caused by the locking structure? I tested the next two 
> > programs, and saw a difference was raised by the locking structure. 
> > The first program throws an IllegalMonitorStateException and the 
> > second one throws the NullPointerException. We just monitored another 
> > object in the first program.
> > 
> > public void main(java.lang.String[]) throws java.lang.Exception;
> > descriptor: ([Ljava/lang/String;)V
> > flags: ACC_PUBLIC
> > Code:
> > stack=1, locals=2, args_size=2
> > 0: aload_0
> > 1: monitorenter
> > 2: aconst_null
> > 3: monitorenter
> > 4: aconst_null
> > 5: monitorexit
> > 6: aload_0
> > 7: monitorexit
> > 8: return
> > Exceptions:
> > throws java.lang.Exception
> > 
> > 
> > public void main(java.lang.String[]) throws java.lang.Exception;
> > descriptor: ([Ljava/lang/String;)V
> > flags: ACC_PUBLIC
> > Code:
> > stack=1, locals=2, args_size=2
> > 0: aconst_null
> > 1: monitorenter
> > 2: aconst_null
> > 3: monitorexit
> > 4: return
> > Exceptions:
> > throws java.lang.Exception
> > 
> > -----邮件原件-----
> > 发件人: Mikael Gerdin [mailto:mikael.gerdin@oracle.com]
> > 发送时间: 2017年5月18日 7:26
> > 收件人: 陈雨亭 <chenyt@cs.sjtu.edu.cn>; 'David Holmes' 
> > <david.holmes@oracle.com>
> > 抄送: hotspot-runtime-dev@openjdk.java.net
> > 主题: Re: 答复: 答复: HotSpot and IBM's J9 behave quite differently when 
> > processing monitorenters and monitorexits
> > 
> > Hi,
> > 
> > On 2017-05-18 16:22, 陈雨亭 wrote:
> > > HotSpot throws a NullPointerException correctly when an initialized 
> > > object as set as null, as follows.
> > > 
> > > public void main(java.lang.String[]) throws java.lang.Exception;
> > > descriptor: ([Ljava/lang/String;)V
> > > flags: ACC_PUBLIC
> > > Code:
> > > stack=1, locals=2, args_size=2
> > > 0: new           #2                  // class Search
> > > 3: invokespecial #14                 // Method "<init>":()V
> > > 6: aconst_null
> > > 7: monitorenter
> > > 8: aconst_null
> > > 9: monitorexit
> > > 10: return
> > > Exceptions:
> > > throws java.lang.Exception
> > > 
> > Right. I just realized I missed the fact that the code did end up 
> > dereferencing the object but the code which actually makes that 
> > happen is not particularly easy to follow.
> > Sorry for the noise.
> > /Mikael
> > 
> > > -----邮件原件-----
> > > 发件人: Mikael Gerdin [mailto:mikael.gerdin@oracle.com]
> > > 发送时间: 2017年5月18日 4:55
> > > 收件人: David Holmes <david.holmes@oracle.com>; chenyt 
> > > <chenyt@cs.sjtu.edu.cn>
> > > 抄送: hotspot-runtime-dev@openjdk.java.net
> > > 主题: Re: 答复: HotSpot and IBM's J9 behave quite differently when 
> > > processing monitorenters and monitorexits
> > > 
> > > 
> > > 
> > > On 2017-05-18 07:25, David Holmes wrote:
> > > > Hi Yuting,
> > > > 
> > > > On 18/05/2017 2:49 PM, chenyt wrote:
> > > > > Hi, David,
> > > > > 
> > > > > I tested the next program (let one object to be monitored be null) 
> > > > > using
> > > > > J9 and HotSpot. HotSpot does not throw a NullPointerException, as 
> > > > > the specification says. J9 looks fine (throw a 
> > > > > NullPointerException). Am I wrong here?
> > > > I can't readily test this as I don't have Jimple nor quick and easy 
> > > > access to bytecode assemblers. It is strange though as the 
> > > > interpreter code contains this:
> > > > 
> > > > CASE(_monitorenter): {
> > > > oop lockee = STACK_OBJECT(-1);
> > > > // derefing's lockee ought to provoke implicit null check
> > > > CHECK_NULL(lockee);
> > > In the x86 templateTable we do
> > > 
> > > void TemplateTable::monitorenter() {
> > > transition(atos, vtos);
> > > 
> > > // check for NULL object
> > > __ null_check(rax);
> > > 
> > > 
> > > but looking in macroAssembler_x86.cpp null_check only does a cmp 
> > > (setting the eflags) but there's no condtional branch on the Z flag?
> > > 
> > > It feels like I'm crazy but several of these __ null_check() calls 
> > > seem broken to me.
> > > 
> > > /Mikael
> > > 
> > > > If I can find some time I will try to test this myself.
> > > > 
> > > > > It is very interesting that I have found so many unexpected 
> > > > > behaviors here.
> > > > Well you only found two and they are related. :) Manually assembled 
> > > > monitor code is not something very many (any?) people care about or 
> > > > do
> > > > - other than emulating correct language usage.
> > > > 
> > > > Cheers,
> > > > David
> > > > 
> > > > > public void main(java.lang.String[]) throws java.lang.Exception;
> > > > > descriptor: ([Ljava/lang/String;)V
> > > > > flags: ACC_PUBLIC
> > > > > Code:
> > > > > stack=1, locals=2, args_size=2
> > > > > 0: aload_0
> > > > > 1: monitorenter
> > > > > 2: aconst_null
> > > > > 3: monitorenter
> > > > > 4: aconst_null
> > > > > 5: monitorexit
> > > > > 6: aload_0
> > > > > 7: monitorexit
> > > > > 8: return
> > > > > Exceptions:
> > > > > throws java.lang.Exception
> > > > > 
> > > > > Jimple code is given as follows:
> > > > > public void main(java.lang.String[]) throws java.lang.Exception
> > > > > {
> > > > > Search r0;
> > > > > Search r2;
> > > > > java.lang.String[] r1;
> > > > > 
> > > > > r0 := @this: Search;
> > > > > r1 := @parameter0: java.lang.String[];
> > > > > r2 = null;
> > > > > entermonitor r0;
> > > > > entermonitor r2;
> > > > > exitmonitor r2;
> > > > > exitmonitor r0;
> > > > > 
> > > > > return;
> > > > > }
> > > > > 
> > > > > Wishes,
> > > > > Yuting
> > > > > 
> > > > > 
> > > > > On Thu, 18 May 2017 13:23:09 +1000, David Holmes wrote:
> > > > > > One correction ...
> > > > > > 
> > > > > > On 18/05/2017 10:29 AM, David Holmes wrote:
> > > > > > > On 18/05/2017 8:12 AM, 陈雨亭 wrote:
> > > > > > > > Thank you, David. I have seen from the specification that 
> > > > > > > > structured locking enforcement is optional. The second and the 
> > > > > > > > third ones are cases of structured/nested lockings. Will 
> > > > > > > > non-nested locking sequences raise deadlocks? Of course it is a 
> > > > > > > > different topic, while it might be better if it can be kicked 
> > > > > > > > out earlier from the specification/JVM.
> > > > > > > Non-nested locking doesn't necessarily lead to deadlocks - you 
> > > > > > > just need a different locking order for that (even if properly 
> > > > > > > nested).
> > > > > > > There are locking patterns that rely on the ability to lock and 
> > > > > > > unlock in different order ie chained-locking for walking 
> > > > > > > linked-lists A->B->C->D:
> > > > > > > - lock A, lock B, unlock A, lock C, unlock B, lock D, unlock C ...
> > > > > > > 
> > > > > > > The VM spec allows a little flexibility in how the monitor 
> > > > > > > bytecodes can be used compared to the Java programming language.
> > > > > > > That's not something that will change.
> > > > > > > 
> > > > > > > > The first example is still a problem. It seems that HotSpot 
> > > > > > > > allows to monitor a pure object reference without initialized 
> > > > > > > > (Is it true? How can this checking be omitted?). J9 reports a 
> > > > > > > > verifyerror as follows.
> > > > > > > > 
> > > > > > > > Exception in thread "main" java.lang.VerifyError: JVMVRFY012 
> > > > > > > > stack shape inconsistent; class=Search, 
> > > > > > > > method=main([Ljava/lang/String;)V, pc=6 Exception Details:
> > > > > > > > Location:
> > > > > > > > Search.main([Ljava/lang/String;)V @6: JBmonitorenter
> > > > > > > > Reason:
> > > > > > > > Type 'uninitialized' (current frame, stack[1]) is not 
> > > > > > > > assignable to 'java/lang/Object'
> > > > > > > > Current Frame:
> > > > > > > > bci: @6
> > > > > > > > flags: { }
> > > > > > > > locals: { 'Search', '[Ljava/lang/String;' }
> > > > > > > > stack: { 'uninitialized', 'uninitialized' }
> > > > > > > > at T.main(T.java:4)
> > > > > > > Yes I think this may be a bug in hotspot. The type-checking for 
> > > > > > > the monitor bytecodes requires a matching type of reference on 
> > > > > > > the operand stack - but "uninitialized" does not match Object, 
> > > > > > > as J9 reports. But I'm not an expert on this aspect of 
> > > > > > > verification so I may not be interpreting it correctly.
> > > > > > > 
> > > > > > > More below ...
> > > > > > > 
> > > > > > > > -----邮件原件-----
> > > > > > > > 发件人: David Holmes [mailto:david.holmes@oracle.com]
> > > > > > > > 发送时间: 2017年5月17日 14:41
> > > > > > > > 收件人: 陈雨亭 <chenyt@cs.sjtu.edu.cn>; 
> > > > > > > > hotspot-runtime-dev@openjdk.java.net
> > > > > > > > 主题: Re: HotSpot and IBM's J9 behave quite differently when 
> > > > > > > > processing monitorenters and monitorexits
> > > > > > > > 
> > > > > > > > Hi,
> > > > > > > > On 18/05/2017 7:23 AM, 陈雨亭 wrote:
> > > > > > > > > Am I wrong?
> > > > > > > > I will look at each situation in detail when I get a chance but 
> > > > > > > > structured locking enforcement is optional. Also balancing the 
> > > > > > > > number of locks and unlocks in a frame does not mean they can't 
> > > > > > > > be locked and unlocked in a non-nested fashion - just that by 
> > > > > > > > the end the number of unlocks matches the number of locks.
> > > > > > > > 
> > > > > > > > BTW the way you respond to these emails, as if having a 
> > > > > > > > conversation with yourself, makes it difficult to respond as we 
> > > > > > > > can't readily see what is the new email and what is the original.
> > > > > > > > 
> > > > > > > > Cheers,
> > > > > > > > David
> > > > > > > > 
> > > > > > > > > The byte code for main() in case 1 is as follows. The strange 
> > > > > > > > > thing is that NullPointerException is also not thrown at runtime.
> > > > > > > That is strange as it does for the normal obvious case of using
> > > > > > > synchronized(o) when o is null.
> > > > > > Ah - it isn't null it just an object for which the constructor 
> > > > > > has not been run. The runtime can't tell the difference between a 
> > > > > > pointer to a valid initialized object, and a pointer to an 
> > > > > > uninitialized chunk of memory.
> > > > > > 
> > > > > > > > > public void main(java.lang.String[]) throws java.lang.Exception;
> > > > > > > > > descriptor: ([Ljava/lang/String;)V
> > > > > > > > > flags: ACC_PUBLIC
> > > > > > > > > Code:
> > > > > > > > > stack=3, locals=2, args_size=2
> > > > > > > > > 0: new           #2                  // class Search
> > > > > > This allocated an object - hence no null reference. But this is 
> > > > > > what verification should have complained about.
> > > > > > 
> > > > > > David
> > > > > > -----
> > > > > > 
> > > > > > > > > 3: dup
> > > > > > > > > 4: aload_0
> > > > > > > > > 5: monitorenter
> > > > > > > > > 6: monitorenter
> > > > > > > > > 7: monitorexit
> > > > > > > > > 8: aload_0
> > > > > > > > > 9: monitorexit
> > > > > > > > > 10: return
> > > > > > > > > Exceptions:
> > > > > > > > > throws java.lang.Exception
> > > > > > > > > 
> > > > > > > > > 主题: HotSpot and IBM's J9 behave quite differently when 
> > > > > > > > > processing monitorenters and monitorexits
> > > > > > > > > 
> > > > > > > > > I have tested several programs (in Jimple) and found that 
> > > > > > > > > HotSpot and
> > > > > > > > > J9 match monitorenters and monitorexits quite differently.
> > > > > > > > > Verifiers should play more important roles here.
> > > > > > > The job of the verifier is to establish some basic guarantees 
> > > > > > > for the JVM to then operate under. The verifier plays no role in 
> > > > > > > checking how monitorenter/exit are used in combination, only 
> > > > > > > that each individual bytecode meets some basic type constraints.
> > > > > > > 
> > > > > > > > > (1) Test the next program (r2 is not initizlied) on HotSpot 
> > > > > > > > > and J9.
> > > > > > > > > J9 throw out a verifier error, while HotSpot does not. It 
> > > > > > > > > seems that HotSpot's verifier forgets to check whether a 
> > > > > > > > > monitored object is initialized.
> > > > > > > > > 
> > > > > > > > > public class Search extends java.lang.Object { public void
> > > > > > > > > <init>()
> > > > > > > > > {
> > > > > > > > > Search r0;
> > > > > > > > > r0 := @this: Search;
> > > > > > > > > specialinvoke r0.<java.lang.Object: void <init>()>();
> > > > > > > > > return;
> > > > > > > > > }
> > > > > > > > > public void main(java.lang.String[]) throws java.lang.Exception
> > > > > > > > > {
> > > > > > > > > Search r0;
> > > > > > > > > Search r2;
> > > > > > > > > java.lang.String[] r1;
> > > > > > > > > r0 := @this: Search;
> > > > > > > > > r1 := @parameter0: java.lang.String[];
> > > > > > > > > r2 = new Search;
> > > > > > > > > 
> > > > > > > > > entermonitor r2;
> > > > > > > > > entermonitor r0;
> > > > > > > > > exitmonitor r2;
> > > > > > > > > exitmonitor r0;
> > > > > > > > > return;
> > > > > > > > > }
> > > > > > > > > }
> > > > > > > Verification was covered above.
> > > > > > > 
> > > > > > > > > (2) Test the next program on HotSpot and J9, and both do not 
> > > > > > > > > report any errors. However, I guess the order in the program 
> > > > > > > > > (entermonitor r2; => entermonitor r0; =>  exitmonitor r2; => 
> > > > > > > > > exitmonitor r0;) violates the situation of "structured locking"
> > > > > > > > > (Structured locking is the situation when, during a method 
> > > > > > > > > invocation, every exit on a given monitor matches a preceding 
> > > > > > > > > entry on that monitor, see the specification
> > > > > > > > > 
> > > > > > > > > https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-2.html#jvms-2. \
> > > > > > > > >  
> > > > > > > > > 
> > > > > > > > > 11.10)
> > > > > > > > > ?
> > > > > > > No it doesn't violate structured locking as the number of enters 
> > > > > > > and exits match, and there is always an enter before an exit.
> > > > > > > 
> > > > > > > > > Actually, the words (every exit on a given monitor matches a 
> > > > > > > > > preceding entry on that monitor) are not quite clear as for me.
> > > > > > > > > Otherwise the first rule (The number of monitor entries 
> > > > > > > > > performed by T on M during a method invocation must equal the 
> > > > > > > > > number of monitor exits performed by T on M during the method 
> > > > > > > > > invocation whether the method  invocation completes normally 
> > > > > > > > > or
> > > > > > > > > abruptly.) is sufficient.
> > > > > > > The number of enters and exits must not only match/balance, but 
> > > > > > > there must be an enter before a corresponding exit.
> > > > > > > 
> > > > > > > > > public class Search extends java.lang.Object {
> > > > > > > > > 
> > > > > > > > > public void <init>()
> > > > > > > > > {
> > > > > > > > > Search r0;
> > > > > > > > > r0 := @this: Search;
> > > > > > > > > specialinvoke r0.<java.lang.Object: void <init>()>();
> > > > > > > > > return;
> > > > > > > > > }
> > > > > > > > > 
> > > > > > > > > public void main(java.lang.String[]) throws java.lang.Exception
> > > > > > > > > {
> > > > > > > > > Search r0;
> > > > > > > > > Search r2;
> > > > > > > > > java.lang.String[] r1;
> > > > > > > > > r0 := @this: Search;
> > > > > > > > > r1 := @parameter0: java.lang.String[];
> > > > > > > > > r2 = new Search;
> > > > > > > > > specialinvoke r2.<Search: void <init>()>();
> > > > > > > > > entermonitor r2;
> > > > > > > > > entermonitor r0;
> > > > > > > > > exitmonitor r2;
> > > > > > > > > exitmonitor r0;
> > > > > > > > > return;
> > > > > > > > > }
> > > > > > > > > }
> > > > > > > > > 
> > > > > > > > > (3) The next program enters monitor in <init> and exits it in 
> > > > > > > > > main().
> > > > > > > > > HotSpot throws a runtime exception, while J9 does not. Should 
> > > > > > > > > this program be rejected by the verifiers?
> > > > > > > No this does not violate any verification rules. The runtime 
> > > > > > > behaviour depends on whether structured locking is enforced or 
> > > > > > > not.(Even in hotspot there can be differences between 
> > > > > > > interpreted and jitted code).
> > > > > > > 
> > > > > > > Hope that helps clarify things.
> > > > > > > 
> > > > > > > David
> > > > > > > -----
> > > > > > > 
> > > > > > > > > public class Search extends java.lang.Object {
> > > > > > > > > 
> > > > > > > > > public void <init>()
> > > > > > > > > {
> > > > > > > > > Search r0;
> > > > > > > > > r0 := @this: Search;
> > > > > > > > > specialinvoke r0.<java.lang.Object: void <init>()>();
> > > > > > > > > entermonitor r0;
> > > > > > > > > return;
> > > > > > > > > }
> > > > > > > > > 
> > > > > > > > > public void main(java.lang.String[]) throws java.lang.Exception
> > > > > > > > > {
> > > > > > > > > Search r0;
> > > > > > > > > Search r2;
> > > > > > > > > java.lang.S
> 


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic