From 4253db22526da9997f3b0140995cac09c41aeb22 Mon Sep 17 00:00:00 2001 From: Dean Long Date: Fri, 3 Apr 2026 04:08:21 +0000 Subject: [PATCH] 8350208: CTW: GraphKit::add_safepoint_edges asserts "not enough operands for reexecution" Co-authored-by: Quan Anh Mai Reviewed-by: mhaessig, vlivanov --- src/hotspot/share/opto/doCall.cpp | 15 +-- src/hotspot/share/opto/parse.hpp | 6 +- src/hotspot/share/opto/parse1.cpp | 11 +- .../TestDebugDuringExceptionCatching.java | 126 ++++++++++++++++++ .../lang/invoke/lib/InstructionHelper.java | 25 +++- 5 files changed, 166 insertions(+), 17 deletions(-) create mode 100644 test/hotspot/jtreg/compiler/exceptions/TestDebugDuringExceptionCatching.java diff --git a/src/hotspot/share/opto/doCall.cpp b/src/hotspot/share/opto/doCall.cpp index e4418631d17..9a1da726f00 100644 --- a/src/hotspot/share/opto/doCall.cpp +++ b/src/hotspot/share/opto/doCall.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 1998, 2025, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1998, 2026, 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 @@ -37,6 +37,7 @@ #include "opto/callGenerator.hpp" #include "opto/castnode.hpp" #include "opto/cfgnode.hpp" +#include "opto/graphKit.hpp" #include "opto/mulnode.hpp" #include "opto/parse.hpp" #include "opto/rootnode.hpp" @@ -909,8 +910,7 @@ void Parse::catch_call_exceptions(ciExceptionHandlerStream& handlers) { if (handler_bci < 0) { // merge with corresponding rethrow node throw_to_exit(make_exception_state(ex_oop)); } else { // Else jump to corresponding handle - push_ex_oop(ex_oop); // Clear stack and push just the oop. - merge_exception(handler_bci); + push_and_merge_exception(handler_bci, ex_oop); } } @@ -1008,13 +1008,10 @@ void Parse::catch_inline_exceptions(SafePointNode* ex_map) { int handler_bci = handler->handler_bci(); if (remaining == 1) { - push_ex_oop(ex_node); // Push exception oop for handler if (PrintOpto && WizardMode) { tty->print_cr(" Catching every inline exception bci:%d -> handler_bci:%d", bci(), handler_bci); } - // If this is a backwards branch in the bytecodes, add safepoint - maybe_add_safepoint(handler_bci); - merge_exception(handler_bci); // jump to handler + push_and_merge_exception(handler_bci, ex_node); // jump to handler return; // No more handling to be done here! } @@ -1039,15 +1036,13 @@ void Parse::catch_inline_exceptions(SafePointNode* ex_map) { const TypeInstPtr* tinst = TypeOopPtr::make_from_klass_unique(klass)->cast_to_ptr_type(TypePtr::NotNull)->is_instptr(); assert(klass->has_subklass() || tinst->klass_is_exact(), "lost exactness"); Node* ex_oop = _gvn.transform(new CheckCastPPNode(control(), ex_node, tinst)); - push_ex_oop(ex_oop); // Push exception oop for handler if (PrintOpto && WizardMode) { tty->print(" Catching inline exception bci:%d -> handler_bci:%d -- ", bci(), handler_bci); klass->print_name(); tty->cr(); } // If this is a backwards branch in the bytecodes, add safepoint - maybe_add_safepoint(handler_bci); - merge_exception(handler_bci); + push_and_merge_exception(handler_bci, ex_oop); } set_control(not_subtype_ctrl); diff --git a/src/hotspot/share/opto/parse.hpp b/src/hotspot/share/opto/parse.hpp index 397a7796f88..42f44f0f7ea 100644 --- a/src/hotspot/share/opto/parse.hpp +++ b/src/hotspot/share/opto/parse.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2025, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 2026, 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 @@ -474,8 +474,8 @@ class Parse : public GraphKit { void merge( int target_bci); // Same as plain merge, except that it allocates a new path number. void merge_new_path( int target_bci); - // Merge the current mapping into an exception handler. - void merge_exception(int target_bci); + // Push the exception oop and merge the current mapping into an exception handler. + void push_and_merge_exception(int target_bci, Node* ex_oop); // Helper: Merge the current mapping into the given basic block void merge_common(Block* target, int pnum); // Helper functions for merging individual cells. diff --git a/src/hotspot/share/opto/parse1.cpp b/src/hotspot/share/opto/parse1.cpp index 647e8515b30..683633f6355 100644 --- a/src/hotspot/share/opto/parse1.cpp +++ b/src/hotspot/share/opto/parse1.cpp @@ -1651,9 +1651,14 @@ void Parse::merge_new_path(int target_bci) { } //-------------------------merge_exception------------------------------------- -// Merge the current mapping into the basic block starting at bci -// The ex_oop must be pushed on the stack, unlike throw_to_exit. -void Parse::merge_exception(int target_bci) { +// Push the given ex_oop onto the stack, then merge the current mapping into +// the basic block starting at target_bci. +void Parse::push_and_merge_exception(int target_bci, Node* ex_oop) { + // Add the safepoint before trimming the stack and pushing the exception oop. + // We could add the safepoint after, but then the bci would also need to be + // advanced to target_bci first, so the stack state matches. + maybe_add_safepoint(target_bci); + push_ex_oop(ex_oop); // Push exception oop for handler #ifdef ASSERT if (target_bci <= bci()) { C->set_exception_backedge(); diff --git a/test/hotspot/jtreg/compiler/exceptions/TestDebugDuringExceptionCatching.java b/test/hotspot/jtreg/compiler/exceptions/TestDebugDuringExceptionCatching.java new file mode 100644 index 00000000000..999c73fb73a --- /dev/null +++ b/test/hotspot/jtreg/compiler/exceptions/TestDebugDuringExceptionCatching.java @@ -0,0 +1,126 @@ +/* + * Copyright (c) 2026, 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. + */ +package compiler.exceptions; + +import compiler.lib.ir_framework.Run; +import compiler.lib.ir_framework.Test; +import compiler.lib.ir_framework.TestFramework; + +import java.lang.classfile.Label; +import java.lang.constant.ClassDesc; +import java.lang.invoke.MethodHandle; +import java.lang.invoke.MethodHandles; +import java.lang.invoke.MethodType; + +import jdk.test.lib.Asserts; +import test.java.lang.invoke.lib.InstructionHelper; + +/** + * @test + * @bug 8350208 + * @summary Safepoints added during the processing of exception handlers should never reexecute + * @library /test/lib /test/jdk/java/lang/invoke/common / + * @build test.java.lang.invoke.lib.InstructionHelper + * + * @run main/othervm ${test.main.class} + */ +public class TestDebugDuringExceptionCatching { + + public static class V { + int v; + } + + static final int ITERATIONS = 100; + static final RuntimeException EXCEPTION = new RuntimeException(); + + /** + * Construct something that looks like this: + * 
{@code
+     * int snippet(V v) {
+     *     int i = 0;
+     *     LoopHead: {
+     *         if (i >= 100) {
+     *             goto LoopEnd;
+     *         }
+     *         i++;
+     *         try {
+     *             v.v = 1;
+     *         } catch (Throwable) {
+     *             // Not really, the LoopHead is the exception Handler
+     *             goto LoopHead;
+     *         }
+     *     }
+     *     LoopEnd:
+     *     return i;
+     * }
+     * }
+ */ + static final MethodHandle SNIPPET_HANDLE; + static final ClassDesc CLASS_DESC = TestDebugDuringExceptionCatching.class.describeConstable().get(); + static { + SNIPPET_HANDLE = InstructionHelper.buildMethodHandle(MethodHandles.lookup(), + "snippet", + MethodType.methodType(int.class, V.class), + CODE -> { + Label loopHead = CODE.newLabel(); + Label loopEnd = CODE.newLabel(); + Label tryStart = CODE.newLabel(); + Label tryEnd = CODE.newLabel(); + CODE. + iconst_0(). + istore(1). + // The loop head should have a RuntimeException as the sole element on the stack + getstatic(CLASS_DESC, "EXCEPTION", RuntimeException.class.describeConstable().get()). + labelBinding(loopHead). + pop(). + iload(1). + ldc(ITERATIONS). + if_icmpge(loopEnd). + iinc(1, 1). + aload(0). + iconst_1(). + labelBinding(tryStart). + putfield(V.class.describeConstable().get(), "v", int.class.describeConstable().get()). + labelBinding(tryEnd). + // The stack is empty here + labelBinding(loopEnd). + iload(1). + ireturn(); + CODE.exceptionCatchAll(tryStart, tryEnd, loopHead); + }); + } + + @Test + private static int testBackwardHandler(V v) throws Throwable { + return (int) SNIPPET_HANDLE.invokeExact(v); + } + + @Run(test = "testBackwardHandler") + public void run() throws Throwable { + Asserts.assertEQ(ITERATIONS, testBackwardHandler(null)); + } + + public static void main(String[] args) { + TestFramework.run(); + } +} diff --git a/test/jdk/java/lang/invoke/common/test/java/lang/invoke/lib/InstructionHelper.java b/test/jdk/java/lang/invoke/common/test/java/lang/invoke/lib/InstructionHelper.java index 123cccd53e7..c7185f691b2 100644 --- a/test/jdk/java/lang/invoke/common/test/java/lang/invoke/lib/InstructionHelper.java +++ b/test/jdk/java/lang/invoke/common/test/java/lang/invoke/lib/InstructionHelper.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2010, 2026, 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 @@ -25,6 +25,7 @@ package test.java.lang.invoke.lib; import java.lang.classfile.ClassBuilder; import java.lang.classfile.ClassFile; +import java.lang.classfile.CodeBuilder; import java.lang.classfile.TypeKind; import java.lang.constant.*; @@ -32,6 +33,7 @@ import java.lang.invoke.MethodHandle; import java.lang.invoke.MethodHandles; import java.lang.invoke.MethodType; import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Consumer; import static java.lang.invoke.MethodType.fromMethodDescriptorString; @@ -135,4 +137,25 @@ public class InstructionHelper { String classDescStr = sb.insert(sb.length() - 1, suffix).toString(); return ClassDesc.ofDescriptor(classDescStr); } + + public static MethodHandle buildMethodHandle(MethodHandles.Lookup l, String methodName, MethodType methodType, Consumer builder) { + ClassDesc genClassDesc = classDesc(l.lookupClass(), "$Code_" + COUNT.getAndIncrement()); + return buildMethodHandle(l, genClassDesc, methodName, methodType, builder); + } + + private static MethodHandle buildMethodHandle(MethodHandles.Lookup l, ClassDesc classDesc, String methodName, MethodType methodType, Consumer builder) { + try { + byte[] bytes = ClassFile.of().build(classDesc, classBuilder -> { + classBuilder.withMethod(methodName, + MethodTypeDesc.ofDescriptor(methodType.toMethodDescriptorString()), + ClassFile.ACC_PUBLIC + ClassFile.ACC_STATIC, + methodBuilder -> methodBuilder.withCode(builder)); + }); + Class clazz = l.defineClass(bytes); + return l.findStatic(clazz, methodName, methodType); + } catch (Throwable t) { + t.printStackTrace(); + throw new RuntimeException("Failed to buildMethodHandle: " + methodName + " type " + methodType); + } + } }