RFR: 8316746: Top of lock-stack does not match the unlocked object [v3]

[Martin Doerr]

On Thu, 28 Sep 2023 10:38:52 GMT, Martin Doerr <mdoerr at openjdk.org> wrote:

>> I think we need to support "Top of lock-stack does not match the unlocked object" and take the slow path. Reason: see JBS issue.
>> Currently only PPC64, x86_64 and aarch64 code. I'd like to get feedback before implementing it for other platforms (needed for all platforms).
>
> Martin Doerr has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision:
> 
>  - Pass may_be_unordered information to lightweight_unlock.
>  - Merge remote-tracking branch 'origin' into 8316746_lock_stack
>  - Add x86_64 and aarch64 implementation.
>  - 8316746: Top of lock-stack does not match the unlocked object

Thanks for looking at it! I'm currently using this PR as workaround until a better fix is found (without integrating it). I have come to the same conclusion that JIT compilers reject methods which may possibly contain unbalanced monitors.

What I can observe is that the test passes with this:

diff --git a/src/hotspot/share/opto/parse1.cpp b/src/hotspot/share/opto/parse1.cpp
index 36a87cd8b0d..385dc54f947 100644
--- a/src/hotspot/share/opto/parse1.cpp
+++ b/src/hotspot/share/opto/parse1.cpp
@@ -222,7 +222,7 @@ void Parse::load_interpreter_state(Node* osr_buf) {
   assert(jvms()->monitor_depth() == 0, "should be no active locks at beginning of osr");
   int mcnt = osr_block->flow()->monitor_count();
   Node *monitors_addr = basic_plus_adr(osr_buf, osr_buf, (max_locals+mcnt*2-1)*wordSize);
-  for (index = 0; index < mcnt; index++) {
+  for (index = mcnt; (--index) >= 0;) {
     // Make a BoxLockNode for the monitor.
     Node *box = _gvn.transform(new BoxLockNode(next_monitor()));
 

I think the lowest address contains the latest monitor (because the stack grows downwards) which should get pushed latest. Is this currently wrong?

Note that x86_64 doesn't have the lock checks, so we don't know if it's affected, too. The test may work even if it's wrong because we unlock both objects.
Other locking modes don't really care about the order, so the test simply passes even though it's wrong (at least on PPC64).

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

PR Comment: https://git.openjdk.org/jdk/pull/15903#issuecomment-1740943594