8275638: GraphKit::combine_exception_states fails with "matching stack sizes" assert

Reviewed-by: dlong, kvn

Author: rwestrel

src/hotspot/share/opto/callGenerator.cpp

@@ -416,6 +416,11 @@ class LateInlineMHCallGenerator : public LateInlineCallGenerator {
 };
 
 bool LateInlineMHCallGenerator::do_late_inline_check(Compile* C, JVMState* jvms) {
+  // When inlining a virtual call, the null check at the call and the call itself can throw. These 2 paths have different
+  // expression stacks which causes late inlining to break. The MH invoker is not expected to be called from a method wih
+  // exception handlers. When there is no exception handler, GraphKit::builtin_throw() pops the stack which solves the issue
+  // of late inlining with exceptions.
+  assert(!jvms->method()->has_exception_handlers(), "no exception handler expected");
   // Even if inlining is not allowed, a virtual call can be strength-reduced to a direct call.
   bool allow_inline = C->inlining_incrementally();
   bool input_not_const = true;

src/hotspot/share/opto/graphKit.cpp

@@ -613,6 +613,13 @@ void GraphKit::builtin_throw(Deoptimization::DeoptReason reason, Node* arg) {
       const TypeOopPtr* val_type = TypeOopPtr::make_from_klass(env()->String_klass());
       Node *store = access_store_at(ex_node, adr, adr_typ, null(), val_type, T_OBJECT, IN_HEAP);
 
+      if (!method()->has_exception_handlers()) {
+        // We don't need to preserve the stack if there's no handler as the entire frame is going to be popped anyway.
+        // This prevents issues with exception handling and late inlining.
+        set_sp(0);
+        clean_stack(0);
+      }
+
       add_exception_state(make_exception_state(ex_node));
       return;
     }

test/hotspot/jtreg/compiler/exceptions/TestLateMHInlineExceptions.java

@@ -0,0 +1,105 @@
+/*
+ * Copyright (c) 2021, Red Hat, Inc. 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
+ * @bug 8275638
+ * @summary GraphKit::combine_exception_states fails with "matching stack sizes" assert
+ *
+ * @run main/othervm -XX:-BackgroundCompilation -XX:-UseOnStackReplacement -XX:CompileCommand=dontinline,TestLateMHInlineExceptions::m
+ *                   -XX:+IgnoreUnrecognizedVMOptions -XX:+AlwaysIncrementalInline TestLateMHInlineExceptions
+ * @run main/othervm -XX:-BackgroundCompilation -XX:-UseOnStackReplacements -XX:+IgnoreUnrecognizedVMOptions -XX:+AlwaysIncrementalInline
+ *                   TestLateMHInlineExceptions
+ *
+ */
+
+import java.lang.invoke.MethodHandle;
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.MethodType;
+
+public class TestLateMHInlineExceptions {
+    public static void main(String[] args) throws Throwable {
+        TestLateMHInlineExceptions test = new TestLateMHInlineExceptions();
+        for (int i = 0; i < 20_000; i++) {
+            test1(test);
+            try {
+                test1(null);
+            } catch (NullPointerException npe) {
+            }
+            test2(test);
+            test2(null);
+            test3(test);
+            try {
+                test3(null);
+            } catch (NullPointerException npe) {
+            }
+            test4(test);
+            test4(null);
+        }
+    }
+
+    void m() {
+    }
+
+    static final MethodHandle mh;
+
+    static {
+        MethodHandles.Lookup lookup = MethodHandles.lookup();
+        try {
+            mh = lookup.findVirtual(TestLateMHInlineExceptions.class, "m", MethodType.methodType(void.class));
+        } catch (NoSuchMethodException e) {
+            e.printStackTrace();
+            throw new RuntimeException("Method handle lookup failed");
+        } catch (IllegalAccessException e) {
+            e.printStackTrace();
+            throw new RuntimeException("Method handle lookup failed");
+        }
+    }
+
+    private static void test1(TestLateMHInlineExceptions test) throws Throwable {
+        mh.invokeExact(test);
+    }
+
+    private static void test2(TestLateMHInlineExceptions test) throws Throwable {
+        try {
+            mh.invokeExact(test);
+        } catch (NullPointerException npe) {
+        }
+    }
+
+    private static void inlined(TestLateMHInlineExceptions test) throws Throwable {
+        mh.invokeExact(test);
+    }
+
+
+    private static void test3(TestLateMHInlineExceptions test) throws Throwable {
+        inlined(test);
+    }
+
+    private static void test4(TestLateMHInlineExceptions test) throws Throwable {
+        try {
+            inlined(test);
+        } catch (NullPointerException npe) {
+        }
+    }
+}