RFR: 8253540: InterpreterRuntime::monitorexit should be a JRT_LEAF function

[Daniel D.Daugherty]

On Wed, 23 Sep 2020 19:40:02 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> Right, looks like the UseHeavyMonitors case in unlock_object misses the "Free entry" part.
>> 
>> I believe this is how it's supposed to work:
>> - TemplateTable::monitorexit only uses unlock_object if the obj is not null and it is found in the monitor section on
>>   stack.
>> - unlock_object should clear the obj field on stack ("Free entry" on x86)
>> - when calling monitorexit for the same unlocked obj, TemplateTable::monitorexit shouldn't find it on stack any more and
>>   throw the exception
>> 
>> Ah, we have "elem->set_obj(NULL);" in InterpreterRuntime::monitorexit. So this should also cover the UseHeavyMonitors
>> case.
>
> The callers of unlock_object all check that the object is non-null and not already unlocked, otherwise they throw
> NSME.  I haven't checked the non-x86 platforms recently to verify that, but that's how it should work. This should
> really be refactored so that it's easier to tell.  But this shouldn't throw NSME inside of the InterpreterRuntime
> function.

I went through all the callers of InterpreterMacroAssembler::unlock_object()
and it does look like all caller paths are properly protected by an ownership
check and a throwing of IllegalMonitorStateException.

InterpreterMacroAssembler::unlock_object() isn't the only caller of
InterpreterRuntime::monitorexit(). It looks like it is also used here:

src/hotspot/cpu/zero/zeroInterpreter_zero.cpp
src/hotspot/share/interpreter/zero/bytecodeInterpreter.cpp

At least one of the code paths in bytecodeInterpreter.cpp has
a (sort of) proper pre-check for IllegalMonitorStateException
before the call to InterpreterRuntime::monitorexit(). But at least
two of the code paths call InterpreterRuntime::monitorexit() and
check for throwing IllegalMonitorStateException afterwards
which totally boggles my mind.

-------------

PR: https://git.openjdk.java.net/jdk/pull/320