RFR 8217223 [lworld][c1] monitorenter on value object should throw IllegalMonitorStateException

[David Holmes]

Hi Ioi,

On 16/01/2019 4:32 pm, Ioi Lam wrote:
> https://bugs.openjdk.java.net/browse/JDK-8217223
> http://cr.openjdk.java.net/~iklam/valhalla/8217223-c1-monitorenter-throw-imse.v01/ 
> 
> 
> Here's a fix for C1 to properly throw IllegalMonitorStateException
> when synchronizing on value objects.
> 
> Ideally, we should just call into Runtime1::monitorenter(), which
> handles the IMSE throwing. However, due to C1's limitation, when
> an exception happens inside Runtime1::monitorenter(), there's no
> way for the current method to catch this exception, so for code
> like this:
> 
>      try {
>          synchronized(vt) {}
>      } catch (Throwable t) {
>          foo();
>      }
> 
> foo() is never called and the exception is thrown to the method's
> caller.

That seems strange behaviour. If an exception is thrown anywhere then 
the compiler should take care of finding the first applicable catch 
block for it! If it doesn't then isn't that the bug that should be fixed?

David
-----

> 
> As a work around, I added an explicit check in MonitorEnterStub to
> call a SimpleExceptionStub to throw the IMSE.
> 
> Here's what the generated code looks like. The instructions marked
> by "+" are added by this patch.
> 
> 
> +UseBiasedLocking case:
> ======================
> 
>   ;; MonitorEnterStub slow case
>    0x00007fe788e285f0: mov    (%rsi),%rdi
> + 0x00007fe788e285f3: test   $0x405,%edi
>         ;; 0x405 = markOopDesc::always_locked_pattern
> + 0x00007fe788e285f9: jne    0x00007fe788e28612
>    0x00007fe788e285ff: mov    %rsi,0x8(%rsp)
>    0x00007fe788e28604: mov    %rdx,(%rsp)
>    0x00007fe788e28608: callq  0x00007fe7889d8840
>         ;;{runtime_call monitorenter_nofpu}
> 
>    0x00007fe788e2860d: jmpq   0x00007fe788e28381
>   ;; SimpleExceptionStub slow case
> + 0x00007fe788e28612: callq  0x00007fe7889d7e40
>         ;;{runtime_call throw_illegal_monitor_state_exception}
> 
> 
> -UseBiasedLocking case:
> ======================
> 
>    0x00007fe1e8e2828c: lea    0x20(%rsp),%rdx
>    0x00007fe1e8e28291: mov    %rsi,0x8(%rdx)
>    0x00007fe1e8e28295: mov    (%rsi),%rax
> + 0x00007fe1e8e28298: test   $0x405,%eax
> + 0x00007fe1e8e2829d: jne    0x00007fe1e8e28528 ;; MonitorEnterStub
>    0x00007fe1e8e282a3: or     $0x1,%rax
>    0x00007fe1e8e282a7: mov    %rax,(%rdx)
>    0x00007fe1e8e282aa: lock cmpxchg %rdx,(%rsi)
> 
> (the MonitorEnterStub looks identical to the +UseBiasedLocking case).
> 
> Thanks
> - Ioi
> 
>