8268717: Upstream: 8268673: Stack walk across optimized entry frame on fresh native thread fails

JornVernee · JornVernee · commit f25e7197fef7 · 2021-06-21T12:06:51.000Z
Reviewed-by: mcimadamore, erikj
diff --git a/make/test/JtregNativeJdk.gmk b/make/test/JtregNativeJdk.gmk
@@ -53,6 +53,8 @@ BUILD_JDK_JTREG_EXECUTABLES_CFLAGS_exeJliLaunchTest := \
     -I$(TOPDIR)/src/java.base/$(OPENJDK_TARGET_OS_TYPE)/native/libjli \
     -I$(TOPDIR)/src/java.base/$(OPENJDK_TARGET_OS)/native/libjli
 
+BUILD_JDK_JTREG_LIBRARIES_LDFLAGS_libAsyncStackWalk := $(LIBCXX)
+
 # Platform specific setup
 ifeq ($(call isTargetOs, windows), true)
   BUILD_JDK_JTREG_EXCLUDE += libDirectIO.c libInheritedChannel.c exelauncher.c
@@ -63,6 +65,7 @@ ifeq ($(call isTargetOs, windows), true)
   BUILD_JDK_JTREG_EXECUTABLES_LIBS_exeJliLaunchTest := $(WIN_LIB_JLI)
   BUILD_JDK_JTREG_EXECUTABLES_LIBS_exeCallerAccessTest := jvm.lib
   BUILD_JDK_JTREG_EXECUTABLES_LIBS_exerevokeall := advapi32.lib
+  BUILD_JDK_JTREG_LIBRARIES_CFLAGS_libAsyncStackWalk := /EHsc
 else
   BUILD_JDK_JTREG_LIBRARIES_LIBS_libstringPlatformChars := -ljava
   BUILD_JDK_JTREG_LIBRARIES_LIBS_libDirectIO := -ljava
diff --git a/src/hotspot/cpu/aarch64/frame_aarch64.cpp b/src/hotspot/cpu/aarch64/frame_aarch64.cpp
@@ -367,6 +367,11 @@ JavaFrameAnchor* OptimizedEntryBlob::jfa_for_frame(const frame& frame) const {
   return nullptr;
 }
 
+bool frame::optimized_entry_frame_is_first() const {
+  ShouldNotCallThis();
+  return false;
+}
+
 frame frame::sender_for_optimized_entry_frame(RegisterMap* map) const {
   ShouldNotCallThis();
   return {};
diff --git a/src/hotspot/cpu/arm/frame_arm.cpp b/src/hotspot/cpu/arm/frame_arm.cpp
@@ -313,6 +313,11 @@ frame frame::sender_for_entry_frame(RegisterMap* map) const {
   return fr;
 }
 
+bool frame::optimized_entry_frame_is_first() const {
+  ShouldNotCallThis();
+  return false;
+}
+
 //------------------------------------------------------------------------------
 // frame::verify_deopt_original_pc
 //
diff --git a/src/hotspot/cpu/ppc/frame_ppc.cpp b/src/hotspot/cpu/ppc/frame_ppc.cpp
@@ -197,6 +197,11 @@ frame frame::sender_for_entry_frame(RegisterMap *map) const {
   return fr;
 }
 
+bool frame::optimized_entry_frame_is_first() const {
+  ShouldNotCallThis();
+  return false;
+}
+
 frame frame::sender_for_interpreter_frame(RegisterMap *map) const {
   // Pass callers initial_caller_sp as unextended_sp.
   return frame(sender_sp(), sender_pc(), (intptr_t*)get_ijava_state()->sender_sp);
diff --git a/src/hotspot/cpu/s390/frame_s390.cpp b/src/hotspot/cpu/s390/frame_s390.cpp
@@ -208,6 +208,11 @@ frame frame::sender_for_entry_frame(RegisterMap *map) const {
   return fr;
 }
 
+bool frame::optimized_entry_frame_is_first() const {
+  ShouldNotCallThis();
+  return false;
+}
+
 frame frame::sender_for_interpreter_frame(RegisterMap *map) const {
   // Pass callers sender_sp as unextended_sp.
   return frame(sender_sp(), sender_pc(), (intptr_t*)(ijava_state()->sender_sp));
diff --git a/src/hotspot/cpu/x86/frame_x86.cpp b/src/hotspot/cpu/x86/frame_x86.cpp
@@ -358,12 +358,20 @@ JavaFrameAnchor* OptimizedEntryBlob::jfa_for_frame(const frame& frame) const {
   return reinterpret_cast<JavaFrameAnchor*>(reinterpret_cast<char*>(frame.unextended_sp()) + in_bytes(jfa_sp_offset()));
 }
 
+bool frame::optimized_entry_frame_is_first() const {
+  assert(is_optimized_entry_frame(), "must be optimzed entry frame");
+  OptimizedEntryBlob* blob = _cb->as_optimized_entry_blob();
+  JavaFrameAnchor* jfa = blob->jfa_for_frame(*this);
+  return jfa->last_Java_sp() == NULL;
+}
+
 frame frame::sender_for_optimized_entry_frame(RegisterMap* map) const {
   assert(map != NULL, "map must be set");
   OptimizedEntryBlob* blob = _cb->as_optimized_entry_blob();
   // Java frame called from C; skip all C frames and return top C
   // frame of that chunk as the sender
   JavaFrameAnchor* jfa = blob->jfa_for_frame(*this);
+  assert(!optimized_entry_frame_is_first(), "must have a frame anchor to go back to");
   assert(jfa->last_Java_sp() > sp(), "must be above this frame on stack");
   // Since we are walking the stack now this nested anchor is obviously walkable
   // even if it wasn't when it was stacked.
diff --git a/src/hotspot/cpu/zero/frame_zero.cpp b/src/hotspot/cpu/zero/frame_zero.cpp
@@ -61,6 +61,11 @@ frame frame::sender_for_entry_frame(RegisterMap *map) const {
   return frame(zeroframe()->next(), sender_sp());
 }
 
+bool frame::optimized_entry_frame_is_first() const {
+  ShouldNotCallThis();
+  return false;
+}
+
 frame frame::sender_for_nonentry_frame(RegisterMap *map) const {
   assert(zeroframe()->is_interpreter_frame() ||
          zeroframe()->is_fake_stub_frame(), "wrong type of frame");
diff --git a/src/hotspot/share/prims/whitebox.cpp b/src/hotspot/share/prims/whitebox.cpp
@@ -2306,6 +2306,7 @@ WB_ENTRY(void, WB_VerifyFrames(JNIEnv* env, jobject wb, jboolean log, jboolean u
     tty_token = ttyLocker::hold_tty();
     tty->print_cr("[WhiteBox::VerifyFrames] Walking Frames");
   }
+  ResourceMark rm; // for verify
   for (StackFrameStream fst(JavaThread::current(), update_map, true); !fst.is_done(); fst.next()) {
     frame* current_frame = fst.current();
     if (log) {
diff --git a/src/hotspot/share/runtime/frame.hpp b/src/hotspot/share/runtime/frame.hpp
@@ -342,6 +342,7 @@ class frame {
 
   // tells whether there is another chunk of Delta stack above
   bool entry_frame_is_first() const;
+  bool optimized_entry_frame_is_first() const;
 
   // Safepoints
 
diff --git a/src/hotspot/share/runtime/frame.inline.hpp b/src/hotspot/share/runtime/frame.inline.hpp
@@ -50,7 +50,9 @@ inline bool frame::is_stub_frame() const {
 }
 
 inline bool frame::is_first_frame() const {
-  return is_entry_frame() && entry_frame_is_first();
+  return (is_entry_frame() && entry_frame_is_first())
+      // Optimized entry frames are only present on certain platforms
+      || (is_optimized_entry_frame() && optimized_entry_frame_is_first());
 }
 
 inline bool frame::is_optimized_entry_frame() const {
diff --git a/test/jdk/java/foreign/stackwalk/TestAsyncStackWalk.java b/test/jdk/java/foreign/stackwalk/TestAsyncStackWalk.java
@@ -0,0 +1,113 @@
+/*
+ *  Copyright (c) 2021, Oracle and/or its affiliates. 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
+ * @requires ((os.arch == "amd64" | os.arch == "x86_64") & sun.arch.data.model == "64") | os.arch == "aarch64"
+ * @library /test/lib
+ * @build sun.hotspot.WhiteBox
+ * @run driver jdk.test.lib.helpers.ClassFileInstaller sun.hotspot.WhiteBox
+ *
+ * @run main/othervm
+ *   -Xbootclasspath/a:.
+ *   -XX:+UnlockDiagnosticVMOptions
+ *   -XX:+WhiteBoxAPI
+ *   -Djdk.internal.foreign.ProgrammableInvoker.USE_INTRINSICS=true
+ *   --enable-native-access=ALL-UNNAMED
+ *   -Xbatch
+ *   TestAsyncStackWalk
+ *
+ * @run main/othervm
+ *   -Xbootclasspath/a:.
+ *   -XX:+UnlockDiagnosticVMOptions
+ *   -XX:+WhiteBoxAPI
+ *   -Djdk.internal.foreign.ProgrammableInvoker.USE_INTRINSICS=false
+ *   --enable-native-access=ALL-UNNAMED
+ *   -Xbatch
+ *   TestAsyncStackWalk
+ */
+
+import jdk.incubator.foreign.CLinker;
+import jdk.incubator.foreign.FunctionDescriptor;
+import jdk.incubator.foreign.SymbolLookup;
+import jdk.incubator.foreign.MemoryAddress;
+
+import java.lang.invoke.MethodHandle;
+import java.lang.invoke.MethodType;
+
+import jdk.incubator.foreign.ResourceScope;
+import sun.hotspot.WhiteBox;
+
+import static java.lang.invoke.MethodHandles.lookup;
+import static jdk.incubator.foreign.CLinker.C_POINTER;
+import static jdk.test.lib.Asserts.assertTrue;
+
+public class TestAsyncStackWalk {
+    static final WhiteBox WB = WhiteBox.getWhiteBox();
+
+    static final CLinker linker = CLinker.getInstance();
+
+    static final MethodHandle MH_asyncStackWalk;
+    static final MethodHandle MH_m;
+
+    static {
+        try {
+            System.loadLibrary("AsyncStackWalk");
+            SymbolLookup lookup = SymbolLookup.loaderLookup();
+            MH_asyncStackWalk = linker.downcallHandle(
+                    lookup.lookup("asyncStackWalk").get(),
+                    MethodType.methodType(void.class, MemoryAddress.class),
+                    FunctionDescriptor.ofVoid(C_POINTER));
+            MH_m = lookup().findStatic(TestAsyncStackWalk.class, "m", MethodType.methodType(void.class));
+        } catch (ReflectiveOperationException e) {
+            throw new RuntimeException(e);
+        }
+    }
+
+    static int invocations;
+    static boolean didStackWalk;
+
+    public static void main(String[] args) throws Throwable {
+        try (ResourceScope scope = ResourceScope.newConfinedScope()) {
+            MemoryAddress stub = linker.upcallStub(MH_m, FunctionDescriptor.ofVoid(), scope);
+            MemoryAddress stubAddress = stub.address();
+            invocations = 0;
+            didStackWalk = false;
+            payload(stubAddress);
+            assertTrue(didStackWalk);
+        }
+    }
+
+    static void payload(MemoryAddress cb) throws Throwable {
+        MH_asyncStackWalk.invokeExact(cb);
+    }
+
+    static void m() {
+        if (invocations++ >= 20000) { // warmup
+            didStackWalk = true;
+            WB.verifyFrames(/*log=*/true, /*updateRegisterMap=*/true);
+            WB.verifyFrames(/*log=*/true, /*updateRegisterMap=*/false); // triggers different code paths
+        }
+    }
+
+}
diff --git a/test/jdk/java/foreign/stackwalk/libAsyncStackWalk.cpp b/test/jdk/java/foreign/stackwalk/libAsyncStackWalk.cpp
@@ -0,0 +1,44 @@
+/*
+ *  Copyright (c) 2021, Oracle and/or its affiliates. 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.
+ *
+ */
+
+#include <thread>
+
+#ifdef _WIN64
+#define EXPORT __declspec(dllexport)
+#else
+#define EXPORT
+#endif
+
+static void start(void (*cb)(void)) {
+    for (int i = 0; i < 25000; i++) {
+        cb();
+    }
+}
+
+extern "C" {
+EXPORT void asyncStackWalk(void (*cb)(void)) {
+    std::thread thrd(start, cb);
+    thrd.join();
+}
+}

Original file line number	Diff line number	Diff line change
`@@ -2306,6 +2306,7 @@ WB_ENTRY(void, WB_VerifyFrames(JNIEnv* env, jobject wb, jboolean log, jboolean u`
`2306`	`2306`	`tty_token = ttyLocker::hold_tty();`
`2307`	`2307`	`tty->print_cr("[WhiteBox::VerifyFrames] Walking Frames");`
`2308`	`2308`	`}`
	`2309`	`+ ResourceMark rm; // for verify`
`2309`	`2310`	`for (StackFrameStream fst(JavaThread::current(), update_map, true); !fst.is_done(); fst.next()) {`
`2310`	`2311`	`frame* current_frame = fst.current();`
`2311`	`2312`	`if (log) {`
Original file line number	Diff line number	Diff line change
`@@ -50,7 +50,9 @@ inline bool frame::is_stub_frame() const {`
`50`	`50`	`}`
`51`	`51`
`52`	`52`	`inline bool frame::is_first_frame() const {`
`53`		`- return is_entry_frame() && entry_frame_is_first();`
	`53`	`+ return (is_entry_frame() && entry_frame_is_first())`
	`54`	`+ // Optimized entry frames are only present on certain platforms`
	`55`	`+ \|\| (is_optimized_entry_frame() && optimized_entry_frame_is_first());`
`54`	`56`	`}`
`55`	`57`
`56`	`58`	`inline bool frame::is_optimized_entry_frame() const {`