From e452e6c8b06e7ff3d47d0b3cd6fa8d73eb85b655 Mon Sep 17 00:00:00 2001 From: Vladimir Ivanov Date: Tue, 14 Jul 2026 16:13:18 +0000 Subject: [PATCH] 8374783: C2 compilation asserts with "slice of address and input slice don't match" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Vladimir Ivanov Co-authored-by: Roberto Castañeda Lozano Reviewed-by: chagedorn, qamai --- src/hotspot/share/opto/callGenerator.cpp | 36 +-- src/hotspot/share/opto/classes.hpp | 1 + src/hotspot/share/opto/compile.cpp | 8 + src/hotspot/share/opto/graphKit.hpp | 2 +- src/hotspot/share/opto/opaquenode.cpp | 8 + src/hotspot/share/opto/opaquenode.hpp | 13 + .../TestLateInliningWithSliceNarrowing.java | 225 ++++++++++++++++++ 7 files changed, 277 insertions(+), 16 deletions(-) create mode 100644 test/hotspot/jtreg/compiler/inlining/TestLateInliningWithSliceNarrowing.java diff --git a/src/hotspot/share/opto/callGenerator.cpp b/src/hotspot/share/opto/callGenerator.cpp index d0b48982b0f..10df25abcb8 100644 --- a/src/hotspot/share/opto/callGenerator.cpp +++ b/src/hotspot/share/opto/callGenerator.cpp @@ -717,26 +717,32 @@ void CallGenerator::do_late_inline_helper() { C->inline_printer()->record(method(), jvms, InliningResult::SUCCESS, "late inline succeeded"); } - // Capture any exceptional control flow - GraphKit kit(new_jvms); - - // Find the result object - Node* result = C->top(); - int result_size = method()->return_type()->size(); - if (result_size != 0 && !kit.stopped()) { - result = (result_size == 1) ? kit.pop() : kit.pop_pair(); - } - - if (call->is_CallStaticJava() && call->as_CallStaticJava()->is_boxing_method()) { - result = kit.must_be_not_null(result, false); - } - if (inline_cg()->is_inline()) { C->set_has_loops(C->has_loops() || inline_cg()->method()->has_loops()); C->env()->notice_inlined_method(inline_cg()->method()); } C->set_inlining_progress(true); - C->set_do_cleanup(kit.stopped()); // path is dead; needs cleanup + + // Find the result object and capture any exceptional control flow. + GraphKit kit(new_jvms); + Node* result = C->top(); + + assert(!C->do_cleanup(), "already set"); + if (kit.stopped()) { + C->set_do_cleanup(true); // path is dead; needs cleanup + } else { + result = kit.pop_node(method()->return_type()->basic_type()); + if (result != C->top() && !result_not_used) { + if (call->is_CallStaticJava() && + call->as_CallStaticJava()->is_boxing_method()) { + result = kit.must_be_not_null(result, false); + } + // Limit result type propagation until next IGVN cleanup. + const Type* result_type = kit.gvn().type(callprojs.resproj); + result = kit.gvn().transform(new OpaqueParseNode(C, result, result_type)); + } + } + kit.replace_call(call, result, true, do_asserts); } } diff --git a/src/hotspot/share/opto/classes.hpp b/src/hotspot/share/opto/classes.hpp index 4d06e20875a..53a72f979db 100644 --- a/src/hotspot/share/opto/classes.hpp +++ b/src/hotspot/share/opto/classes.hpp @@ -288,6 +288,7 @@ macro(OpaqueZeroTripGuard) macro(OpaqueConstantBool) macro(OpaqueInitializedAssertionPredicate) macro(OpaqueTemplateAssertionPredicate) +macro(OpaqueParse) macro(PowD) macro(ProfileBoolean) macro(OrI) diff --git a/src/hotspot/share/opto/compile.cpp b/src/hotspot/share/opto/compile.cpp index e5f91875516..0c7083ed723 100644 --- a/src/hotspot/share/opto/compile.cpp +++ b/src/hotspot/share/opto/compile.cpp @@ -2051,10 +2051,13 @@ void Compile::inline_string_calls(bool parse_time) { _late_inlines_pos = _late_inlines.length(); } + assert(!do_cleanup(), "already set"); + while (_string_late_inlines.length() > 0) { CallGenerator* cg = _string_late_inlines.pop(); cg->do_late_inline(); if (failing()) return; + set_do_cleanup(false); // ignore and reset } _string_late_inlines.trunc_to(0); } @@ -2070,10 +2073,13 @@ void Compile::inline_boxing_calls(PhaseIterGVN& igvn) { _late_inlines_pos = _late_inlines.length(); + assert(!do_cleanup(), "already set"); + while (_boxing_late_inlines.length() > 0) { CallGenerator* cg = _boxing_late_inlines.pop(); cg->do_late_inline(); if (failing()) return; + set_do_cleanup(false); // ignore and reset } _boxing_late_inlines.trunc_to(0); @@ -2647,12 +2653,14 @@ void Compile::check_no_dead_use() const { #endif void Compile::inline_vector_reboxing_calls() { + assert(!do_cleanup(), "already set"); if (C->_vector_reboxing_late_inlines.length() > 0) { _late_inlines_pos = C->_late_inlines.length(); while (_vector_reboxing_late_inlines.length() > 0) { CallGenerator* cg = _vector_reboxing_late_inlines.pop(); cg->do_late_inline(); if (failing()) return; + assert(!do_cleanup(), "should not be set"); print_method(PHASE_INLINE_VECTOR_REBOX, 3, cg->call_node()); } _vector_reboxing_late_inlines.trunc_to(0); diff --git a/src/hotspot/share/opto/graphKit.hpp b/src/hotspot/share/opto/graphKit.hpp index d371dfb2e32..ef160ac6f1a 100644 --- a/src/hotspot/share/opto/graphKit.hpp +++ b/src/hotspot/share/opto/graphKit.hpp @@ -500,7 +500,7 @@ class GraphKit : public Phase { int n_size = type2size[n_type]; if (n_size == 1) return pop(); else if (n_size == 2) return pop_pair(); - else return nullptr; + else return C->top(); } Node* control() const { return map_not_null()->control(); } diff --git a/src/hotspot/share/opto/opaquenode.cpp b/src/hotspot/share/opto/opaquenode.cpp index 428379e84ae..a3b50d269fb 100644 --- a/src/hotspot/share/opto/opaquenode.cpp +++ b/src/hotspot/share/opto/opaquenode.cpp @@ -183,6 +183,14 @@ void OpaqueInitializedAssertionPredicateNode::dump_spec(outputStream* st) const } #endif // NOT PRODUCT +// Do NOT remove the opaque node until subsequent IGVN pass. +Node* OpaqueParseNode::Identity(PhaseGVN* phase) { + if (phase->is_IterGVN()) { + return in(1); + } + return this; +} + uint ProfileBooleanNode::hash() const { return NO_HASH; } bool ProfileBooleanNode::cmp( const Node &n ) const { return (&n == this); diff --git a/src/hotspot/share/opto/opaquenode.hpp b/src/hotspot/share/opto/opaquenode.hpp index bb3da2aa65f..7ec7e23144a 100644 --- a/src/hotspot/share/opto/opaquenode.hpp +++ b/src/hotspot/share/opto/opaquenode.hpp @@ -258,6 +258,19 @@ class OpaqueInitializedAssertionPredicateNode : public Node { NOT_PRODUCT(void dump_spec(outputStream* st) const); }; +// The node is used during late inlining to limit type propagation between cleanup phases. +// It avoids type paradoxes caused by divergence between recorded type and IR shapes +// during successive late inlining attempts. +class OpaqueParseNode : public TypeNode { + public: + OpaqueParseNode(Compile* C, Node* n, const Type* t) : TypeNode(t, 2) { + init_req(1, n); + C->record_for_igvn(this); + } + virtual int Opcode() const; + virtual Node* Identity(PhaseGVN* phase); +}; + //------------------------------ProfileBooleanNode------------------------------- // A node represents value profile for a boolean during parsing. // Once parsing is over, the node goes away (during IGVN). diff --git a/test/hotspot/jtreg/compiler/inlining/TestLateInliningWithSliceNarrowing.java b/test/hotspot/jtreg/compiler/inlining/TestLateInliningWithSliceNarrowing.java new file mode 100644 index 00000000000..fbae0454d07 --- /dev/null +++ b/test/hotspot/jtreg/compiler/inlining/TestLateInliningWithSliceNarrowing.java @@ -0,0 +1,225 @@ +/* + * 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.inlining; + +import java.lang.reflect.Field; +import jdk.internal.misc.Unsafe; +import jdk.test.lib.Asserts; + +/** + * @test + * @bug 8374783 + * @summary Test that address type refinements after an incremental inlining + * step are propagated by IGVN before the next step. Failing to + * propagate such refinements could lead to slice mismatches between + * field-derived and IGVN-recorded address types when parsing bytecode + * in subsequent inlining steps. + * @library /test/lib + * @modules java.base/jdk.internal.misc + * @run main ${test.main.class} + * @run main/othervm -Xbatch + -XX:CompileCommand=compileonly,${test.main.class}::test* + -XX:CompileCommand=dontinline,${test.main.class}::notInlined* + -XX:CompileCommand=delayinline,${test.main.class}::late* + ${test.main.class} + */ + +class A { + int f; +} + +public class TestLateInliningWithSliceNarrowing { + + private static Unsafe UNSAFE = Unsafe.getUnsafe(); + private static final long F_OFFSET; + private static final long INT_ARRAY_OFFSET; + + static { + try { + Field fField = A.class.getDeclaredField("f"); + F_OFFSET = UNSAFE.objectFieldOffset(fField); + } catch (Exception e) { + throw new RuntimeException(e); + } + INT_ARRAY_OFFSET = UNSAFE.arrayBaseOffset(int[].class); + } + + static A notInlinedId(A a) { + return a; + } + + static long lateOffset() { + return F_OFFSET; + } + + static long lateOffsetMinusFour() { + return F_OFFSET - 4; + } + + static long lateOffsetDividedByTwo() { + return F_OFFSET / 2; + } + + static long lateArrayOffset() { + return INT_ARRAY_OFFSET; + } + + static void lateStore(A a) { + a.f = 42; + } + + static void lateArrayStore(int[] a) { + a[0] = 42; + } + + static int lateLoad(A a) { + return a.f; + } + + static Object lateBase(A a) { + return a; + } + + // Test that when lateStore() is inlined, the IGVN-recorded type of the + // accessed memory address (captured by an AddP) has been updated to reflect + // the compiler-known offset discovered by inlining lateOffset(). Failure to + // do so leads to a slice mismatch when parsing the inlined store. + static int testLoadFromLateDiscoveredOffsetThenStoreAtConstOffset(A a) { + long o = lateOffset(); + int val = UNSAFE.getInt(a, o); + lateStore(a); + return val; + } + + // Test that when lateLoad() is inlined, the IGVN-recorded type of the + // accessed memory address (captured by an AddP) has been updated to reflect + // the compiler-known offset discovered by inlining lateOffset(). Failure to + // do so leads to a slice mismatch when parsing the inlined load. + static int testLoadFromLateDiscoveredOffsetThenLoadFromConstOffset(A a) { + long o = lateOffset(); + int val = UNSAFE.getInt(a, o); + lateLoad(a); + return val; + } + + // Test a variation of the above where lateOffsetMinusFour() is not used + // directly by an AddP node. This test does not require updating the + // IGVN-recorded type of the accessed memory address for correctness, + // because lateStore() does not reuse the corresponding AddP node. + static int testLoadFromLateDiscoveredOffsetPlusFourThenStoreAtConstOffset(A a) { + long o = lateOffsetMinusFour(); + int val = UNSAFE.getInt(a, o + 4); + lateStore(a); + return val; + } + + // Test a variation of the above using a different arithmetic operation, + // with the same expectations. + static int testLoadFromLateDiscoveredOffsetTimesTwoThenStoreAtConstOffset(A a) { + long o = lateOffsetDividedByTwo(); + int val = UNSAFE.getInt(a, o * 2); + lateStore(a); + return val; + } + + // Test a variation of the first test where failing to update the + // IGVN-recorded type of the accessed memory address would result in a slice + // mismatch that will lead to an incorrect memory graph (the memory input of + // the last load would bypass the memory output of the store). + static int testLoadFromLateDiscoveredOffsetThenStoreAtConstOffsetThenReloadFromConstOffset(A a) { + A a2 = notInlinedId(a); + long o = lateOffset(); + int val = UNSAFE.getInt(a, o); + lateStore(a); + return a2.f + val; + } + + // Test a variation of the first test where the offset is compiler-known + // from the beginning, but the unsafe base address is only discovered by + // inlining lateBase(). This variation does not require a cleanup between + // the late inlining of lateBase() and lateLoad() for correctness: a slice + // mismatch cannot occur because the memory access within lateLoad() does + // not reuse the same address node (AddP) as the unsafe load. The unsafe + // load address node is not reusable by the lateLoad() access because it is + // obscured by casts by the time lateLoad() is late inlined. Making the + // address node reusable by both loads would require a cleanup round, which + // would prevent the mismatch from happening in the first place. + static int testLoadFromLateDiscoveredBaseThenLoadFromKnownBase(A a) { + Object obj = lateBase(a); + int val = UNSAFE.getInt(obj, F_OFFSET); + lateLoad(a); + return val; + } + + // Test a variation of the first test using an array instead of a class + // instance. No slice mismatch occurs because the address types for both + // memory accesses lead to the same slice, regardless of whether the offset + // is compiler-known. + static int testArrayLoadFromLateDiscoveredOffsetThenStoreAtConstOffset(int[] a) { + long o = lateArrayOffset(); + int val = UNSAFE.getInt(a, o); + lateArrayStore(a); + return val; + } + + public static void main(String[] args) { + for (int i = 0; i < 10_000; i++) { + { + A a = new A(); + int result = testLoadFromLateDiscoveredOffsetThenStoreAtConstOffset(a); + Asserts.assertEquals(0, result); + } + { + A a = new A(); + int result = testLoadFromLateDiscoveredOffsetThenLoadFromConstOffset(a); + Asserts.assertEquals(0, result); + } + { + A a = new A(); + int result = testLoadFromLateDiscoveredOffsetPlusFourThenStoreAtConstOffset(a); + Asserts.assertEquals(0, result); + } + { + A a = new A(); + int result = testLoadFromLateDiscoveredOffsetTimesTwoThenStoreAtConstOffset(a); + Asserts.assertEquals(0, result); + } + { + A a = new A(); + int result = testLoadFromLateDiscoveredOffsetThenStoreAtConstOffsetThenReloadFromConstOffset(a); + Asserts.assertEquals(42, result); + } + { + A a = new A(); + int result = testLoadFromLateDiscoveredBaseThenLoadFromKnownBase(a); + Asserts.assertEquals(0, result); + } + { + int[] a = new int[1]; + int result = testArrayLoadFromLateDiscoveredOffsetThenStoreAtConstOffset(a); + Asserts.assertEquals(0, result); + } + } + } +}