Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/hotspot/share/classfile/javaClasses.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,7 @@ class java_lang_String : AllStatic {

// Tester
static inline bool is_instance(oop obj);
static inline bool is_instance_without_asserts(oop obj);

// Debugging
static void print(oop java_string, outputStream* st, int max_length = MaxStringPrintSize);
Expand Down
7 changes: 7 additions & 0 deletions src/hotspot/share/classfile/javaClasses.inline.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,13 @@ bool java_lang_String::is_instance(oop obj) {
return obj != nullptr && obj->klass() == vmClasses::String_klass();
}

// We need to be able to check if this is a string even when we are in an
// erroneous state where the klass is corrupted/can't be read safely.
// This happens when printing an oop during GC error reporting.
bool java_lang_String::is_instance_without_asserts(oop obj) {
return obj != nullptr && obj->klass_without_asserts() == vmClasses::String_klass();
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you make a small upstream patch for this in mainline (either in addition or instead of this part of the change?)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I'll do it separately, I want to get this fixed in Valhalla ASAP.

// Accessors

oop java_lang_ref_Reference::weak_referent_no_keepalive(oop ref) {
Expand Down
10 changes: 8 additions & 2 deletions src/hotspot/share/gc/shared/modRefBarrierSet.inline.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,8 @@ clone_in_heap(oop src, oop dst, size_t size) {
template <DecoratorSet decorators, typename BarrierSetT>
inline void ModRefBarrierSet::AccessBarrier<decorators, BarrierSetT>::
value_copy_in_heap(void* src, void* dst, InlineKlass* md, LayoutKind lk) {
if (HasDecorator<decorators, IS_DEST_UNINITIALIZED>::value || (!md->contains_oops())) {
if (!md->contains_oops()) {
// If we do not have oops in the flat array, we can just do a raw copy.
Raw::value_copy(src, dst, md, lk);
} else {
BarrierSetT* bs = barrier_set_cast<BarrierSetT>(BarrierSet::barrier_set());
Expand All @@ -170,9 +171,13 @@ value_copy_in_heap(void* src, void* dst, InlineKlass* md, LayoutKind lk) {
// Pre-barriers...
OopMapBlock* map = md->start_of_nonstatic_oop_maps();
OopMapBlock* const end = map + md->nonstatic_oop_map_count();
bool is_uninitialized = HasDecorator<decorators, IS_DEST_UNINITIALIZED>::value;
while (map != end) {
address doop_address = dst_oop_addr_offset + map->offset();
bs->write_ref_array_pre((OopType*) doop_address, map->count(), false);
// The pre-barrier only impacts G1, which will emit a barrier if the destination is
// initialized. Note that we should not emit a barrier if the destination is uninitialized,
// as doing so will fill the SATB queue with garbage data.
bs->write_ref_array_pre((OopType*) doop_address, map->count(), is_uninitialized);
map++;
}

Expand All @@ -182,6 +187,7 @@ value_copy_in_heap(void* src, void* dst, InlineKlass* md, LayoutKind lk) {
map = md->start_of_nonstatic_oop_maps();
while (map != end) {
address doop_address = dst_oop_addr_offset + map->offset();
// The post-barrier needs to be called for initialized and uninitialized destinations.
bs->write_ref_array((HeapWord*) doop_address, map->count());
map++;
}
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/oops/oop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ char* oopDesc::print_value_string() {

void oopDesc::print_value_on(outputStream* st) const {
oop obj = const_cast<oopDesc*>(this);
if (java_lang_String::is_instance(obj)) {
if (java_lang_String::is_instance_without_asserts(obj)) {
java_lang_String::print(obj, st);
print_address_on(st);
} else {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
/*
* 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=serial
* @summary Ensures that value arrays can get arraycopied properly with Serial.
* This test will likely crash if that is not the case.
* @bug 8370479
* @enablePreview
* @requires vm.flagless
* @library /test/lib /
* @modules java.base/jdk.internal.value
java.management
* @build jdk.test.whitebox.WhiteBox
* @run driver jdk.test.lib.helpers.ClassFileInstaller jdk.test.whitebox.WhiteBox
* @run junit/othervm/timeout=480 -Xint -XX:+UseSerialGC -XX:+UseCompressedOops -Xlog:gc*=info
-XX:+UseCompressedClassPointers
-Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI
runtime.valhalla.inlinetypes.FlatArrayCopyingTest
*/

/*
* @test id=parallel
* @summary Ensures that value arrays can get arraycopied properly with Parallel.
* This test will likely crash if that is not the case.
* @bug 8370479
* @enablePreview
* @requires vm.flagless
* @library /test/lib /
* @modules java.base/jdk.internal.value
java.management
* @build jdk.test.whitebox.WhiteBox
* @run driver jdk.test.lib.helpers.ClassFileInstaller jdk.test.whitebox.WhiteBox
* @run junit/othervm/timeout=480 -Xint -XX:+UseParallelGC -XX:+UseCompressedOops -Xlog:gc*=info
-XX:ParallelGCThreads=1 -XX:-UseDynamicNumberOfGCThreads
-Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI
runtime.valhalla.inlinetypes.FlatArrayCopyingTest
*/

/*
* @test id=g1
* @summary Ensures that value arrays can get arraycopied properly with G1.
* This test will likely crash if that is not the case.
* @bug 8370479
* @enablePreview
* @requires vm.flagless
* @library /test/lib /
* @modules java.base/jdk.internal.value
java.management
* @build jdk.test.whitebox.WhiteBox
* @run driver jdk.test.lib.helpers.ClassFileInstaller jdk.test.whitebox.WhiteBox
* @run junit/othervm/timeout=480 -Xint -XX:+UseG1GC -XX:+UseCompressedOops -Xlog:gc*=info
-XX:ParallelGCThreads=1 -XX:ConcGCThreads=1 -XX:-UseDynamicNumberOfGCThreads
-XX:-G1UseConcRefinement -XX:+UseCompressedClassPointers
-Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI
runtime.valhalla.inlinetypes.FlatArrayCopyingTest
*/

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 jdk.test.whitebox.WhiteBox;

import static org.junit.jupiter.api.Assertions.*;

public final class FlatArrayCopyingTest {
private static final WhiteBox WB = WhiteBox.getWhiteBox();

public static value record Element(Identity underlying) {}
public static class Identity {}

@Test
public void testCopyingOften() {
Object[] array = ValueClass.newNullableAtomicArray(Element.class, 16);
for (int i = 0; i < 1_000_000; i++) {
if (i == array.length) {
array = Arrays.copyOf(array, array.length << 1);
}
// Do a very "random" full GC.
if (i == 5123123) {
WB.fullGC();
}
array[i] = new Element(new Identity());
}
// Smallest power of 2 that fits 1 million elements.
assertEquals(1_048_576, array.length);
}

}