8277602: Deopt code does not extend the stack enough if the caller is…

… an optimize entry blob

Reviewed-by: dlong, thartmann

src/hotspot/cpu/x86/universalUpcallHandler_x86_64.cpp

@@ -616,12 +616,9 @@ address ProgrammableUpcallHandler::generate_optimized_upcall_stub(jobject receiv
   int reg_save_area_size = compute_reg_save_area_size(abi);
   int arg_save_area_size = compute_arg_save_area_size(conv);
   int res_save_area_size = compute_res_save_area_size(conv);
-  // To spill receiver during deopt
-  int deopt_spill_size = 1 * BytesPerWord;
 
   int shuffle_area_offset    = 0;
-  int deopt_spill_offset     = shuffle_area_offset    + out_arg_area;
-  int res_save_area_offset   = deopt_spill_offset     + deopt_spill_size;
+  int res_save_area_offset   = shuffle_area_offset    + out_arg_area;
   int arg_save_area_offset   = res_save_area_offset   + res_save_area_size;
   int reg_save_area_offset   = arg_save_area_offset   + arg_save_area_size;
   int frame_data_offset      = reg_save_area_offset   + reg_save_area_size;
@@ -648,9 +645,6 @@ address ProgrammableUpcallHandler::generate_optimized_upcall_stub(jobject receiv
   //      | res_save_area       |
   //      |---------------------| = res_save_are_offset
   //      |                     |
-  //      | deopt_spill         |
-  //      |---------------------| = deopt_spill_offset
-  //      |                     |
   // SP-> | out_arg_area        |   needs to be at end for shadow space
   //
   //

src/hotspot/share/runtime/deoptimization.cpp

@@ -585,7 +585,7 @@ Deoptimization::UnrollBlock* Deoptimization::fetch_unroll_info_helper(JavaThread
   // QQQ I'd rather see this pushed down into last_frame_adjust
   // and have it take the sender (aka caller).
 
-  if (deopt_sender.is_compiled_frame() || caller_was_method_handle) {
+  if (deopt_sender.is_compiled_caller() || caller_was_method_handle) {
     caller_adjustment = last_frame_adjust(0, callee_locals);
   } else if (callee_locals > callee_parameters) {
     // The caller frame may need extending to accommodate

src/hotspot/share/runtime/frame.hpp

@@ -146,6 +146,11 @@ class frame {
 
   bool is_interpreted_frame_valid(JavaThread* thread) const;       // performs sanity checks on interpreted frames.
 
+  // is this frame doing a call using the compiled calling convention?
+  bool is_compiled_caller() const {
+    return is_compiled_frame() || is_optimized_entry_frame();
+  }
+
   // tells whether this frame is marked for deoptimization
   bool should_be_deoptimized() const;
 

src/hotspot/share/runtime/vframeArray.cpp

@@ -337,6 +337,7 @@ void vframeArrayElement::unpack_on_stack(int caller_actual_parameters,
   for(i = 0; i < expressions()->size(); i++) {
     StackValue *value = expressions()->at(i);
     intptr_t*   addr  = iframe()->interpreter_frame_expression_stack_at(i);
+    assert(!is_bottom_frame || !(caller->is_compiled_caller() && addr >= caller->unextended_sp()), "overwriting caller frame!");
     switch(value->type()) {
       case T_INT:
         *addr = value->get_int();
@@ -375,6 +376,7 @@ void vframeArrayElement::unpack_on_stack(int caller_actual_parameters,
   for(i = 0; i < locals()->size(); i++) {
     StackValue *value = locals()->at(i);
     intptr_t* addr  = iframe()->interpreter_frame_local_at(i);
+    assert(!is_bottom_frame || !(caller->is_compiled_caller() && addr >= caller->unextended_sp()), "overwriting caller frame!");
     switch(value->type()) {
       case T_INT:
         *addr = value->get_int();

test/jdk/java/foreign/upcalldeopt/TestUpcallDeopt.java

@@ -0,0 +1,112 @@
+/*
+ * 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 id=default_gc
+ * @bug 8277602
+ * @requires ((os.arch == "amd64" | os.arch == "x86_64") & sun.arch.data.model == "64") | os.arch == "aarch64"
+ * @library /test/lib
+ * @library ../
+ * @build sun.hotspot.WhiteBox
+ * @run driver jdk.test.lib.helpers.ClassFileInstaller sun.hotspot.WhiteBox
+ *
+ * @run main/othervm
+ *   -Xbootclasspath/a:.
+ *   -XX:+UnlockDiagnosticVMOptions
+ *   -XX:+WhiteBoxAPI
+ *   --enable-native-access=ALL-UNNAMED
+ *   -Xbatch
+ *   TestUpcallDeopt
+ */
+
+import jdk.incubator.foreign.Addressable;
+import jdk.incubator.foreign.CLinker;
+import jdk.incubator.foreign.FunctionDescriptor;
+import jdk.incubator.foreign.NativeSymbol;
+import jdk.incubator.foreign.SymbolLookup;
+import jdk.incubator.foreign.MemoryAddress;
+
+import java.lang.invoke.MethodHandle;
+import java.lang.invoke.MethodType;
+import java.lang.ref.Reference;
+
+import jdk.incubator.foreign.ResourceScope;
+import sun.hotspot.WhiteBox;
+
+import static java.lang.invoke.MethodHandles.lookup;
+
+public class TestUpcallDeopt extends NativeTestHelper {
+    static final WhiteBox WB = WhiteBox.getWhiteBox();
+
+    static final CLinker linker = CLinker.systemCLinker();
+
+    static final MethodHandle MH_foo;
+    static final MethodHandle MH_m;
+
+    static {
+        try {
+            System.loadLibrary("UpcallDeopt");
+            SymbolLookup lookup = SymbolLookup.loaderLookup();
+            MH_foo = linker.downcallHandle(
+                    lookup.lookup("foo").orElseThrow(),
+                    FunctionDescriptor.ofVoid(C_POINTER, C_INT, C_INT, C_INT, C_INT));
+            MH_m = lookup().findStatic(TestUpcallDeopt.class, "m",
+                    MethodType.methodType(void.class, int.class, int.class, int.class, int.class));
+        } catch (ReflectiveOperationException e) {
+            throw new RuntimeException(e);
+        }
+    }
+
+    static boolean armed;
+
+    // we need to deoptimize through an uncommon trap in the callee of the optimized upcall stub
+    // that is created when calling upcallStub below
+    public static void main(String[] args) throws Throwable {
+        try (ResourceScope scope = ResourceScope.newConfinedScope()) {
+            NativeSymbol stub = linker.upcallStub(MH_m, FunctionDescriptor.ofVoid(C_INT, C_INT, C_INT, C_INT), scope);
+            armed = false;
+            for (int i = 0; i < 20_000; i++) {
+                payload(stub); // warmup
+            }
+
+            armed = true;
+            payload(stub); // test
+        }
+    }
+
+    static void payload(NativeSymbol cb) throws Throwable {
+        MH_foo.invokeExact((Addressable) cb, 0, 1, 2, 3);
+        Reference.reachabilityFence(cb); // keep oop alive across call
+    }
+
+    // Takes a bunch of arguments, even though unused, to test
+    // if the caller's frame is extended enough to spill these arguments.
+    static void m(int a0, int a1, int a2, int a3) {
+        if (armed) {
+            // Trigger uncommon trap from this frame
+            WB.verifyFrames(/*log=*/true, /*updateRegisterMap=*/true);
+            WB.verifyFrames(/*log=*/true, /*updateRegisterMap=*/false); // triggers different code paths
+        }
+    }
+
+}

test/jdk/java/foreign/upcalldeopt/libUpcallDeopt.c

@@ -0,0 +1,32 @@
+/*
+ * 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.
+ */
+
+#ifdef _WIN64
+#define EXPORT __declspec(dllexport)
+#else
+#define EXPORT
+#endif
+
+EXPORT void foo(void (*cb)(int, int, int, int), int a0, int a1, int a2, int a3) {
+    cb(a0, a1, a2, a3);
+}