From e45e106fc65aa3e7886b1e6fe422f3b6660807fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20H=C3=BCbner?= Date: Wed, 19 Nov 2025 15:17:34 +0100 Subject: [PATCH 1/6] Use ptrdiff_t in value_at_addr. --- src/hotspot/share/oops/flatArrayOop.inline.hpp | 6 ++++-- src/hotspot/share/oops/inlineKlass.cpp | 2 +- src/hotspot/share/oops/inlineKlass.hpp | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/hotspot/share/oops/flatArrayOop.inline.hpp b/src/hotspot/share/oops/flatArrayOop.inline.hpp index 67b95dabab0..c0eff06d79c 100644 --- a/src/hotspot/share/oops/flatArrayOop.inline.hpp +++ b/src/hotspot/share/oops/flatArrayOop.inline.hpp @@ -38,8 +38,10 @@ inline void* flatArrayOopDesc::base() const { return arrayOopDesc::base(T_FLAT_E inline void* flatArrayOopDesc::value_at_addr(int index, jint lh) const { assert(is_within_bounds(index), "index out of bounds"); - address addr = (address) base(); - addr += (index << Klass::layout_helper_log2_element_size(lh)); + address array_base = (address) base(); + ptrdiff_t offset = (ptrdiff_t) index << Klass::layout_helper_log2_element_size(lh); + address addr = array_base + offset; + assert(addr >= array_base, "must be"); return (void*) addr; } diff --git a/src/hotspot/share/oops/inlineKlass.cpp b/src/hotspot/share/oops/inlineKlass.cpp index 4905498bc87..8ffe3171abf 100644 --- a/src/hotspot/share/oops/inlineKlass.cpp +++ b/src/hotspot/share/oops/inlineKlass.cpp @@ -226,7 +226,7 @@ void InlineKlass::copy_payload_to_addr(void* src, void* dst, LayoutKind lk, bool } } -oop InlineKlass::read_payload_from_addr(const oop src, int offset, LayoutKind lk, TRAPS) { +oop InlineKlass::read_payload_from_addr(const oop src, size_t offset, LayoutKind lk, TRAPS) { assert(src != nullptr, "Must be"); assert(is_layout_supported(lk), "Unsupported layout"); switch(lk) { diff --git a/src/hotspot/share/oops/inlineKlass.hpp b/src/hotspot/share/oops/inlineKlass.hpp index bdb2849acb8..47753890a22 100644 --- a/src/hotspot/share/oops/inlineKlass.hpp +++ b/src/hotspot/share/oops/inlineKlass.hpp @@ -233,7 +233,7 @@ class InlineKlass: public InstanceKlass { // is compatible with all the other layouts. void write_value_to_addr(oop src, void* dst, LayoutKind lk, bool dest_is_initialized, TRAPS); - oop read_payload_from_addr(const oop src, int offset, LayoutKind lk, TRAPS); + oop read_payload_from_addr(const oop src, size_t offset, LayoutKind lk, TRAPS); void copy_payload_to_addr(void* src, void* dst, LayoutKind lk, bool dest_is_initialized); // oop iterate raw inline type data pointer (where oop_addr may not be an oop, but backing/array-element) From fafd5ce85fdfd1a9d40dacf70cf8cc5e2c485da5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20H=C3=BCbner?= Date: Thu, 20 Nov 2025 11:33:11 +0100 Subject: [PATCH 2/6] Fix another overflow. --- src/hotspot/share/oops/flatArrayOop.inline.hpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/hotspot/share/oops/flatArrayOop.inline.hpp b/src/hotspot/share/oops/flatArrayOop.inline.hpp index c0eff06d79c..603e9a89144 100644 --- a/src/hotspot/share/oops/flatArrayOop.inline.hpp +++ b/src/hotspot/share/oops/flatArrayOop.inline.hpp @@ -58,7 +58,9 @@ inline oop flatArrayOopDesc::obj_at(int index, TRAPS) const { assert(is_within_bounds(index), "index %d out of bounds %d", index, length()); FlatArrayKlass* faklass = FlatArrayKlass::cast(klass()); InlineKlass* vk = InlineKlass::cast(faklass->element_klass()); - int offset = ((char*)value_at_addr(index, faklass->layout_helper())) - ((char*)(oopDesc*)this); + char* this_oop = (char*) (oopDesc*) this; + char* val = (char*) value_at_addr(index, faklass->layout_helper()); + ptrdiff_t offset = val - this_oop; oop res = vk->read_payload_from_addr((oopDesc*)this, offset, faklass->layout_kind(), CHECK_NULL); return res; } From ff7be305400d1b2215b4c668a2d2f6c0688a0c9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20H=C3=BCbner?= Date: Thu, 20 Nov 2025 11:36:50 +0100 Subject: [PATCH 3/6] Include test case. --- test/hotspot/gtest/oops/test_flatArrayOop.cpp | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 test/hotspot/gtest/oops/test_flatArrayOop.cpp diff --git a/test/hotspot/gtest/oops/test_flatArrayOop.cpp b/test/hotspot/gtest/oops/test_flatArrayOop.cpp new file mode 100644 index 00000000000..69bce0de696 --- /dev/null +++ b/test/hotspot/gtest/oops/test_flatArrayOop.cpp @@ -0,0 +1,65 @@ +/* + * Copyright (c) 2025, 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. + */ + +#include "oops/flatArrayOop.hpp" +#include "unittest.hpp" +#include "utilities/globalDefinitions.hpp" + +// INTENTIONALLY SMALL BACKING, SHOULD ONLY CONTAIN METADATA + A FEW ELEMENTS. +static unsigned char memory[1024]; + +// Do not perform operations on the array's memory without ensuring that the +// backing is large enough and you will not go out of bounds. +static flatArrayOopDesc* fake_flat_array(int length) { + flatArrayOopDesc* farr = (flatArrayOopDesc*) cast_to_oop(memory); + // We can't ensure the backing for the length, but we can still do pointer + // arithmetic and e.g. ensure that the resulting pointers didn't overflow. + farr->set_length(length); + return farr; +} + +// What FlatArrayKlass::array_layout_helper does, but w/o InlineKlass +static int make_lh(int payload_size_bytes, bool null_free) { + BasicType etype = T_FLAT_ELEMENT; + int esize = log2i_exact(round_up_power_of_2(payload_size_bytes)); + int hsize = arrayOopDesc::base_offset_in_bytes(etype); + return Klass::array_layout_helper(Klass::_lh_array_tag_flat_value, null_free, hsize, etype, esize); +} + +static void ensure_no_overflow(flatArrayOopDesc* farr, int lh) { + void* vaa_small = farr->value_at_addr(123, lh); + EXPECT_TRUE(vaa_small >= farr); + void* vaa_large = farr->value_at_addr(321999888, lh); + EXPECT_TRUE(vaa_large >= farr); +} + +TEST_VM(flatArrayOopDesc, value_at_addr_intbox_nullable) { + flatArrayOopDesc* farr = fake_flat_array(500000000); + ensure_no_overflow(farr, make_lh(8, false)); +} + + +TEST_VM(flatArrayOopDesc, value_at_addr_intbox_null_free) { + flatArrayOopDesc* farr = fake_flat_array(500000000); + ensure_no_overflow(farr, make_lh(4, true)); +} From a7dbd59f5d91c88d2500ad8841f1f53b13462c97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20H=C3=BCbner?= Date: Thu, 20 Nov 2025 13:12:57 +0100 Subject: [PATCH 4/6] Fix up tests. --- test/hotspot/gtest/oops/test_flatArrayOop.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/hotspot/gtest/oops/test_flatArrayOop.cpp b/test/hotspot/gtest/oops/test_flatArrayOop.cpp index 69bce0de696..c896f996e00 100644 --- a/test/hotspot/gtest/oops/test_flatArrayOop.cpp +++ b/test/hotspot/gtest/oops/test_flatArrayOop.cpp @@ -30,8 +30,8 @@ static unsigned char memory[1024]; // Do not perform operations on the array's memory without ensuring that the // backing is large enough and you will not go out of bounds. -static flatArrayOopDesc* fake_flat_array(int length) { - flatArrayOopDesc* farr = (flatArrayOopDesc*) cast_to_oop(memory); +static flatArrayOop fake_flat_array(int length) { + flatArrayOop farr = flatArrayOop(cast_to_oop(memory)); // We can't ensure the backing for the length, but we can still do pointer // arithmetic and e.g. ensure that the resulting pointers didn't overflow. farr->set_length(length); @@ -46,7 +46,7 @@ static int make_lh(int payload_size_bytes, bool null_free) { return Klass::array_layout_helper(Klass::_lh_array_tag_flat_value, null_free, hsize, etype, esize); } -static void ensure_no_overflow(flatArrayOopDesc* farr, int lh) { +static void ensure_no_overflow(flatArrayOop farr, int lh) { void* vaa_small = farr->value_at_addr(123, lh); EXPECT_TRUE(vaa_small >= farr); void* vaa_large = farr->value_at_addr(321999888, lh); @@ -54,12 +54,12 @@ static void ensure_no_overflow(flatArrayOopDesc* farr, int lh) { } TEST_VM(flatArrayOopDesc, value_at_addr_intbox_nullable) { - flatArrayOopDesc* farr = fake_flat_array(500000000); + flatArrayOop farr = fake_flat_array(500000000); ensure_no_overflow(farr, make_lh(8, false)); } TEST_VM(flatArrayOopDesc, value_at_addr_intbox_null_free) { - flatArrayOopDesc* farr = fake_flat_array(500000000); + flatArrayOop farr = fake_flat_array(500000000); ensure_no_overflow(farr, make_lh(4, true)); } From 49320b9019e9241b45bcc73c8589a8f0f5977e04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20H=C3=BCbner?= Date: Fri, 21 Nov 2025 16:30:26 +0100 Subject: [PATCH 5/6] C1 changes --- src/hotspot/share/c1/c1_Instruction.hpp | 5 +++-- src/hotspot/share/c1/c1_InstructionPrinter.cpp | 2 +- src/hotspot/share/c1/c1_LIRGenerator.cpp | 8 ++++---- src/hotspot/share/c1/c1_LIRGenerator.hpp | 2 +- 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/hotspot/share/c1/c1_Instruction.hpp b/src/hotspot/share/c1/c1_Instruction.hpp index 40f27060c75..410b6b7f202 100644 --- a/src/hotspot/share/c1/c1_Instruction.hpp +++ b/src/hotspot/share/c1/c1_Instruction.hpp @@ -1002,7 +1002,7 @@ class DelayedLoadIndexed : public CompilationResourceObj { LoadIndexed* _load_instr; ValueStack* _state_before; ciField* _field; - int _offset; + size_t _offset; public: DelayedLoadIndexed(LoadIndexed* load, ValueStack* state_before) : _load_instr(load) @@ -1011,6 +1011,7 @@ class DelayedLoadIndexed : public CompilationResourceObj { , _offset(0) { } void update(ciField* field, int offset) { + assert(offset >= 0, "must be"); _field = field; _offset += offset; } @@ -1018,7 +1019,7 @@ class DelayedLoadIndexed : public CompilationResourceObj { LoadIndexed* load_instr() const { return _load_instr; } ValueStack* state_before() const { return _state_before; } ciField* field() const { return _field; } - int offset() const { return _offset; } + size_t offset() const { return _offset; } }; LEAF(StoreIndexed, AccessIndexed) diff --git a/src/hotspot/share/c1/c1_InstructionPrinter.cpp b/src/hotspot/share/c1/c1_InstructionPrinter.cpp index b9d5ce6e452..3349850a5ba 100644 --- a/src/hotspot/share/c1/c1_InstructionPrinter.cpp +++ b/src/hotspot/share/c1/c1_InstructionPrinter.cpp @@ -382,7 +382,7 @@ void InstructionPrinter::do_ArrayLength(ArrayLength* x) { void InstructionPrinter::do_LoadIndexed(LoadIndexed* x) { print_indexed(x); if (x->delayed() != nullptr) { - output()->print(" +%d", x->delayed()->offset()); + output()->print(" +%zu", x->delayed()->offset()); output()->print(" (%c)", type2char(x->delayed()->field()->type()->basic_type())); } else { output()->print(" (%c)", type2char(x->elt_type())); diff --git a/src/hotspot/share/c1/c1_LIRGenerator.cpp b/src/hotspot/share/c1/c1_LIRGenerator.cpp index f2a6be509e5..9c1eadaaa57 100644 --- a/src/hotspot/share/c1/c1_LIRGenerator.cpp +++ b/src/hotspot/share/c1/c1_LIRGenerator.cpp @@ -1757,7 +1757,7 @@ void LIRGenerator::access_sub_element(LIRItem& array, LIRItem& index, LIR_Opr& r } void LIRGenerator::access_flat_array(bool is_load, LIRItem& array, LIRItem& index, LIRItem& obj_item, - ciField* field, int sub_offset) { + ciField* field, size_t sub_offset) { assert(sub_offset == 0 || field != nullptr, "Sanity check"); // Find the starting address of the source (inside the array) @@ -1773,7 +1773,7 @@ void LIRGenerator::access_flat_array(bool is_load, LIRItem& array, LIRItem& inde ciField* inner_field = elem_klass->nonstatic_field_at(i); assert(!inner_field->is_flat(), "flat fields must have been expanded"); int obj_offset = inner_field->offset_in_bytes(); - int elm_offset = obj_offset - elem_klass->payload_offset() + sub_offset; // object header is not stored in array. + size_t elm_offset = obj_offset - elem_klass->payload_offset() + sub_offset; // object header is not stored in array. BasicType field_type = inner_field->type()->basic_type(); // Types which are smaller than int are still passed in an int register. @@ -1796,7 +1796,7 @@ void LIRGenerator::access_flat_array(bool is_load, LIRItem& array, LIRItem& inde DecoratorSet decorators = IN_HEAP; if (is_load) { access_load_at(decorators, field_type, - elm_item, LIR_OprFact::intConst(elm_offset), temp, + elm_item, LIR_OprFact::longConst(elm_offset), temp, nullptr, nullptr); access_store_at(decorators, field_type, obj_item, LIR_OprFact::intConst(obj_offset), temp, @@ -1806,7 +1806,7 @@ void LIRGenerator::access_flat_array(bool is_load, LIRItem& array, LIRItem& inde obj_item, LIR_OprFact::intConst(obj_offset), temp, nullptr, nullptr); access_store_at(decorators, field_type, - elm_item, LIR_OprFact::intConst(elm_offset), temp, + elm_item, LIR_OprFact::longConst(elm_offset), temp, nullptr, nullptr); } } diff --git a/src/hotspot/share/c1/c1_LIRGenerator.hpp b/src/hotspot/share/c1/c1_LIRGenerator.hpp index 01e6d016ffb..3362c3e7e20 100644 --- a/src/hotspot/share/c1/c1_LIRGenerator.hpp +++ b/src/hotspot/share/c1/c1_LIRGenerator.hpp @@ -275,7 +275,7 @@ class LIRGenerator: public InstructionVisitor, public BlockClosure { void do_vectorizedMismatch(Intrinsic* x); void do_blackhole(Intrinsic* x); - void access_flat_array(bool is_load, LIRItem& array, LIRItem& index, LIRItem& obj_item, ciField* field = nullptr, int offset = 0); + void access_flat_array(bool is_load, LIRItem& array, LIRItem& index, LIRItem& obj_item, ciField* field = nullptr, size_t offset = 0); void access_sub_element(LIRItem& array, LIRItem& index, LIR_Opr& result, ciField* field, int sub_offset); LIR_Opr get_and_load_element_address(LIRItem& array, LIRItem& index); bool needs_flat_array_store_check(StoreIndexed* x); From 8b0289f5359367879eab1fa5ef5e291a1d40eee6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20H=C3=BCbner?= Date: Fri, 21 Nov 2025 17:57:56 +0100 Subject: [PATCH 6/6] Add regression test. --- .../FlatArrayLargeIndicesTest.java | 97 +++++++++++++++++++ 1 file changed, 97 insertions(+) create mode 100644 test/hotspot/jtreg/runtime/valhalla/inlinetypes/FlatArrayLargeIndicesTest.java diff --git a/test/hotspot/jtreg/runtime/valhalla/inlinetypes/FlatArrayLargeIndicesTest.java b/test/hotspot/jtreg/runtime/valhalla/inlinetypes/FlatArrayLargeIndicesTest.java new file mode 100644 index 00000000000..7e3eb502184 --- /dev/null +++ b/test/hotspot/jtreg/runtime/valhalla/inlinetypes/FlatArrayLargeIndicesTest.java @@ -0,0 +1,97 @@ +/* + * Copyright (c) 2025, 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=interpreted + * @summary Ensures that large flat array indexing doesn't cause overflows. + * @bug 8371604 + * @enablePreview + * @requires vm.flagless & os.maxMemory >= 19G + * @library /test/lib / + * @modules java.base/jdk.internal.value + * @run junit/othervm/timeout=480 -Xmx18G -Xint + runtime.valhalla.inlinetypes.FlatArrayLargeIndicesTest + */ + +/* + * @test id=c1 + * @summary Ensures that large flat array indexing doesn't cause overflows. + * @bug 8371604 + * @enablePreview + * @requires vm.flagless & os.maxMemory >= 19G + * @library /test/lib / + * @modules java.base/jdk.internal.value + * @run junit/othervm/timeout=480 -Xmx18G -XX:TieredStopAtLevel=3 -Xcomp + runtime.valhalla.inlinetypes.FlatArrayLargeIndicesTest + */ + +/* + * @test id=c2 + * @summary Ensures that large flat array indexing doesn't cause overflows. + * @bug 8371604 + * @enablePreview + * @requires vm.flagless & os.maxMemory >= 19G + * @library /test/lib / + * @modules java.base/jdk.internal.value + * @run junit/othervm/timeout=480 -Xmx18G -XX:-TieredCompilation -Xcomp + runtime.valhalla.inlinetypes.FlatArrayLargeIndicesTest + */ + +package runtime.valhalla.inlinetypes; + +import java.util.Arrays; +import jdk.internal.value.ValueClass; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.*; + +public final class FlatArrayLargeIndicesTest { + private static final int BIG = 290_000_000; // 64*BIG > int32 addressable limit + private static final int INDEX_1 = 272146430; // real: observed in one crash + private static final int INDEX_2 = 289062500; // real: observed in another crash + + public static value record Box(int underlying) {} + + @Test + public void testStore() { + Box[] arr = new Box[BIG]; + assertFlat(arr); + arr[INDEX_1] = new Box(19); + System.out.println(arr); + } + + @Test + public void testLoad() { + Box[] arr = new Box[BIG]; + assertFlat(arr); + Box box = arr[289062500]; + assertNull(box, "the box should be null"); + } + + private void assertFlat(Box[] arr) { + assertTrue(ValueClass.isFlatArray(arr), "expected the array to be flattened"); + } + +}