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

Hi Dean, the following test fails with the reversed order (my load_interpreter_state patch above) on PPC64, but passes with the original order. Surprisingly, x86_64 passes with either version (with asm assertions in unlock_object applied from above). Maybe I should contribute this test even though it doesn't trigger the problem in the original VM?

diff --git a/test/hotspot/jtreg/compiler/locks/TestUnlockOSR.java b/test/hotspot/jtreg/compiler/locks/TestUnlockOSR.java
new file mode 100644
index 00000000000..be58f21c977
--- /dev/null
+++ b/test/hotspot/jtreg/compiler/locks/TestUnlockOSR.java
@@ -0,0 +1,49 @@
+/*
+ * Copyright (c) 2023 SAP SE. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ *
+ */
+
+/*
+ * @test
+ * @summary During OSR, locks get transferred from interpreter frame.
+ *          Check that unlocking 2 such locks works in the OSR compiled nmethod.
+ *          Some platforms verify that the unlocking happens in the corrent order.
+ *
+ * @run main/othervm -Xbatch TestUnlockOSR
+ */
+
+public class TestUnlockOSR {
+    static void test_method(Object a, Object b, int limit) {
+        synchronized(a) {
+            synchronized(b) {
+                for (int i = 0; i < limit; i++) {}
+            }
+        }
+    }
+
+    public static void main(String[] args) {
+        Object a = new TestUnlockOSR(),
+               b = new TestUnlockOSR();
+        for (int i = 0; i < 10000; i++) { test_method(a, b, 0); } // compile
+        test_method(a, b, 100000); // deopt, trigger OSR
+    }
+}

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

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