diff --git a/src/hotspot/share/c1/c1_GraphBuilder.cpp b/src/hotspot/share/c1/c1_GraphBuilder.cpp index 3f6993e81f1..ce5e9a2c10f 100644 --- a/src/hotspot/share/c1/c1_GraphBuilder.cpp +++ b/src/hotspot/share/c1/c1_GraphBuilder.cpp @@ -1110,16 +1110,20 @@ void GraphBuilder::load_indexed(BasicType type) { set_pending_load_indexed(dli); return; // Nothing else to do for now } else { - NewInstance* buffer = new NewInstance(elem_klass, state_before, false, true); - buffer->set_null_free(true); - _memory->new_instance(buffer); - result = append_split(buffer); load_indexed = new LoadIndexed(array, index, length, type, state_before); - load_indexed->set_buffer(buffer); - // The LoadIndexed node will initialize this instance by copying from - // the flat field. Ensure these stores are visible before any - // subsequent store that publishes this reference. - need_membar = true; + // Deoptimize on non-null because buffering requires the value class to be initialized + bool assert_null = !array_klass->is_elem_null_free() && !elem_klass->is_initialized(); + if (!assert_null) { + NewInstance* buffer = new NewInstance(elem_klass, state_before, false, true); + buffer->set_null_free(true); + _memory->new_instance(buffer); + result = append_split(buffer); + load_indexed->set_buffer(buffer); + // The LoadIndexed node will initialize this instance by copying from + // the flat field. Ensure these stores are visible before any + // subsequent store that publishes this reference. + need_membar = true; + } } } else { load_indexed = new LoadIndexed(array, index, length, type, state_before); diff --git a/src/hotspot/share/c1/c1_LIRGenerator.cpp b/src/hotspot/share/c1/c1_LIRGenerator.cpp index 29e8f32d39b..52bb4e224b4 100644 --- a/src/hotspot/share/c1/c1_LIRGenerator.cpp +++ b/src/hotspot/share/c1/c1_LIRGenerator.cpp @@ -2167,21 +2167,41 @@ void LIRGenerator::do_LoadField(LoadField* x) { ciInlineKlass* vk = field->type()->as_inline_klass(); #ifdef ASSERT assert(field->is_atomic(), "No atomic access required"); + assert(!is_volatile, "Flat fields cannot be volatile"); assert(x->state_before() != nullptr, "Needs state before"); #endif - // Allocate buffer (we can't easily do this conditionally on the null check below - // because branches added in the LIR are opaque to the register allocator). - NewInstance* buffer = new NewInstance(vk, x->state_before(), false, true); - do_NewInstance(buffer); - LIRItem dest(buffer, this); + NewInstance* buffer = nullptr; + bool assert_null = !field->is_null_free() && !vk->is_initialized(); + if (!assert_null) { + // Allocate the buffer before loading the payload because allocation may safepoint + // and a payload may contain oops represented as raw bits and thus invisible to the GC. + // We can't easily allocate conditionally on the null check below because branches + // added in the LIR are opaque to the register allocator. + buffer = new NewInstance(vk, x->state_before(), false, true); + do_NewInstance(buffer); + } - // Copy the payload to the buffer BasicType bt = vk->atomic_size_to_basic_type(field->is_null_free()); LIR_Opr payload = new_register((bt == T_LONG) ? bt : T_INT); access_load_at(decorators, bt, object, LIR_OprFact::intConst(field->offset_in_bytes()), payload, // Make sure to emit an implicit null check info ? new CodeEmitInfo(info) : nullptr, info); + + if (assert_null) { + // Deoptimize on non-null because buffering requires the value class to be initialized + CodeEmitInfo* null_assert_info = state_for(x, x->state_before()); + __ logical_and(payload, null_marker_mask(bt, field), payload); + __ cmp(lir_cond_notEqual, payload, (bt == T_LONG) ? LIR_OprFact::longConst(0) : LIR_OprFact::intConst(0)); + __ branch(lir_cond_notEqual, new DeoptimizeStub(null_assert_info, Deoptimization::Reason_null_assert, + Deoptimization::Action_make_not_entrant)); + __ move(LIR_OprFact::oopConst(nullptr), rlock_result(x)); + return; + } + + // Copy the payload to the buffer + assert(buffer != nullptr, "buffer required"); + LIRItem dest(buffer, this); access_store_at(decorators, bt, dest, LIR_OprFact::intConst(vk->payload_offset()), payload); if (field->is_null_free()) { @@ -2364,6 +2384,30 @@ void LIRGenerator::do_LoadIndexed(LoadIndexed* x) { } } + ciFlatArrayKlass* flat_array_klass = x->array()->is_loaded_flat_array() ? + x->array()->declared_type()->as_flat_array_klass() : nullptr; + bool assert_null = flat_array_klass != nullptr && !flat_array_klass->is_elem_null_free() && + !flat_array_klass->element_klass()->as_inline_klass()->is_initialized(); + if (assert_null) { + // Deoptimize on non-null because buffering requires the value class to be initialized + assert(x->buffer() == nullptr && x->delayed() == nullptr, "null assertion should not buffer"); + assert(flat_array_klass->is_elem_atomic(), "nullable flat arrays must use an atomic layout"); + ciInlineKlass* elem_klass = flat_array_klass->element_klass()->as_inline_klass(); + CodeEmitInfo* null_assert_info = state_for(x, x->state_before()); + BasicType bt = elem_klass->atomic_size_to_basic_type(false); + LIR_Opr elm_op = get_and_load_element_address(array, index); + ComputedAddressValue* elm_resolved_addr = new ComputedAddressValue(as_ValueType(bt), elm_op); + LIRItem elm_item(elm_resolved_addr, this); + LIR_Opr payload = new_register((bt == T_LONG) ? bt : T_INT); + access_load_at(IN_HEAP, bt, elm_item, LIR_OprFact::intConst(0), payload, nullptr, nullptr); + __ logical_and(payload, null_marker_mask(bt, elem_klass->null_marker_offset_in_payload()), payload); + __ cmp(lir_cond_notEqual, payload, (bt == T_LONG) ? LIR_OprFact::longConst(0) : LIR_OprFact::intConst(0)); + __ branch(lir_cond_notEqual, new DeoptimizeStub(null_assert_info, Deoptimization::Reason_null_assert, + Deoptimization::Action_make_not_entrant)); + __ move(LIR_OprFact::oopConst(nullptr), rlock_result(x)); + return; + } + Value element = nullptr; if (x->buffer() != nullptr) { assert(x->array()->is_loaded_flat_array(), "must be"); diff --git a/test/hotspot/jtreg/compiler/valhalla/inlinetypes/TestUninitializedFlatAccess.java b/test/hotspot/jtreg/compiler/valhalla/inlinetypes/TestUninitializedFlatAccess.java new file mode 100644 index 00000000000..04aa43ed21f --- /dev/null +++ b/test/hotspot/jtreg/compiler/valhalla/inlinetypes/TestUninitializedFlatAccess.java @@ -0,0 +1,104 @@ +/* + * 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.valhalla.inlinetypes; + +import jdk.test.lib.Asserts; + +/** + * @test + * @bug 8389089 + * @summary Accessing an uninitialized flat field or array should not initialize its value class. + * @library /test/lib + * @enablePreview + * @run main ${test.main.class} + * @run main/othervm -Xcomp -XX:TieredStopAtLevel=1 + * -XX:CompileCommand=compileonly,${test.main.class}::test* + * ${test.main.class} + */ + +public class TestUninitializedFlatAccess { + static int fieldClassInitCount; + static int arrayClassInitCount; + + static value class FieldValue { + final int value = 0; + + static { + fieldClassInitCount++; + } + } + + static value class ArrayValue { + final int value = 0; + + static { + arrayClassInitCount++; + } + } + + FieldValue value; + + Object testFieldLoad() { + return value; + } + + void testFieldStore(FieldValue value) { + this.value = value; + } + + static Object testArrayLoad() { + ArrayValue[] array = new ArrayValue[1]; + return array[0]; + } + + static ArrayValue[] testArrayStore(ArrayValue value) { + ArrayValue[] array = new ArrayValue[1]; + array[0] = value; + return array; + } + + public static void main(String[] args) { + TestUninitializedFlatAccess t = new TestUninitializedFlatAccess(); + ArrayValue[] tmp = new ArrayValue[0]; + Asserts.assertEQ(fieldClassInitCount, 0, "FieldValue should not be initialized"); + Asserts.assertEQ(arrayClassInitCount, 0, "ArrayValue should not be initialized"); + + Object fieldValue = t.testFieldLoad(); + Asserts.assertNull(fieldValue, "Unexpected field value"); + Asserts.assertEQ(fieldClassInitCount, 0, "FieldValue should not be initialized"); + + t.testFieldStore(null); + Asserts.assertNull(fieldValue, "Unexpected field value"); + Asserts.assertEQ(fieldClassInitCount, 0, "FieldValue should not be initialized"); + + Object arrayValue = testArrayLoad(); + Asserts.assertNull(arrayValue, "Unexpected array value"); + Asserts.assertEQ(arrayClassInitCount, 0, "ArrayValue should not be initialized"); + + ArrayValue[] array = testArrayStore(null); + Asserts.assertNull(array[0], "Unexpected array value"); + Asserts.assertEQ(arrayClassInitCount, 0, "ArrayValue should not be initialized"); + } +} +