diff --git a/src/hotspot/share/prims/unsafe.cpp b/src/hotspot/share/prims/unsafe.cpp index a6300e81468..80dfaf90a28 100644 --- a/src/hotspot/share/prims/unsafe.cpp +++ b/src/hotspot/share/prims/unsafe.cpp @@ -480,7 +480,9 @@ UNSAFE_LEAF (void, Unsafe_WriteBackPostSync0(JNIEnv *env, jobject unsafe)) { ////// Random queries -static jlong find_field_offset(jclass clazz, jstring name, TRAPS) { +// Finds the object field offset of a field with the matching name, or an error code +// Error code -1 is not found, -2 is static field +static jlong find_known_instance_field_offset(jclass clazz, jstring name, TRAPS) { assert(clazz != nullptr, "clazz must not be null"); assert(name != nullptr, "name must not be null"); @@ -489,16 +491,20 @@ static jlong find_field_offset(jclass clazz, jstring name, TRAPS) { InstanceKlass* k = InstanceKlass::cast(java_lang_Class::as_Klass(JNIHandles::resolve_non_null(clazz))); - jint offset = -1; + jint offset = -1; // Not found for (JavaFieldStream fs(k); !fs.done(); fs.next()) { Symbol *name = fs.name(); if (name->equals(utf_name)) { - offset = fs.offset(); + if (!fs.access_flags().is_static()) { + offset = fs.offset(); + } else { + offset = -2; // A static field + } break; } } if (offset < 0) { - THROW_0(vmSymbols::java_lang_InternalError()); + return offset; // Error code } return field_offset_from_byte_offset(offset); } @@ -527,8 +533,8 @@ UNSAFE_ENTRY(jlong, Unsafe_ObjectFieldOffset0(JNIEnv *env, jobject unsafe, jobje return find_field_offset(field, 0, THREAD); } UNSAFE_END -UNSAFE_ENTRY(jlong, Unsafe_ObjectFieldOffset1(JNIEnv *env, jobject unsafe, jclass c, jstring name)) { - return find_field_offset(c, name, THREAD); +UNSAFE_ENTRY(jlong, Unsafe_KnownObjectFieldOffset0(JNIEnv *env, jobject unsafe, jclass c, jstring name)) { + return find_known_instance_field_offset(c, name, THREAD); } UNSAFE_END UNSAFE_ENTRY(jlong, Unsafe_StaticFieldOffset0(JNIEnv *env, jobject unsafe, jobject field)) { @@ -882,7 +888,7 @@ static JNINativeMethod jdk_internal_misc_Unsafe_methods[] = { {CC "freeMemory0", CC "(" ADR ")V", FN_PTR(Unsafe_FreeMemory0)}, {CC "objectFieldOffset0", CC "(" FLD ")J", FN_PTR(Unsafe_ObjectFieldOffset0)}, - {CC "objectFieldOffset1", CC "(" CLS LANG "String;)J", FN_PTR(Unsafe_ObjectFieldOffset1)}, + {CC "knownObjectFieldOffset0", CC "(" CLS LANG "String;)J", FN_PTR(Unsafe_KnownObjectFieldOffset0)}, {CC "staticFieldOffset0", CC "(" FLD ")J", FN_PTR(Unsafe_StaticFieldOffset0)}, {CC "staticFieldBase0", CC "(" FLD ")" OBJ, FN_PTR(Unsafe_StaticFieldBase0)}, {CC "ensureClassInitialized0", CC "(" CLS ")V", FN_PTR(Unsafe_EnsureClassInitialized0)}, diff --git a/src/java.base/share/classes/java/util/concurrent/atomic/AtomicIntegerFieldUpdater.java b/src/java.base/share/classes/java/util/concurrent/atomic/AtomicIntegerFieldUpdater.java index f947eb4f7db..2250009e8f5 100644 --- a/src/java.base/share/classes/java/util/concurrent/atomic/AtomicIntegerFieldUpdater.java +++ b/src/java.base/share/classes/java/util/concurrent/atomic/AtomicIntegerFieldUpdater.java @@ -403,6 +403,9 @@ public abstract class AtomicIntegerFieldUpdater { if (!Modifier.isVolatile(modifiers)) throw new IllegalArgumentException("Must be volatile type"); + if (Modifier.isStatic(modifiers)) + throw new IllegalArgumentException("Must not be a static field"); + // Access to protected field members is restricted to receivers only // of the accessing class, or one of its subclasses, and the // accessing class must in turn be a subclass (or package sibling) diff --git a/src/java.base/share/classes/java/util/concurrent/atomic/AtomicLongFieldUpdater.java b/src/java.base/share/classes/java/util/concurrent/atomic/AtomicLongFieldUpdater.java index b31a8edf53a..5f0a666cb04 100644 --- a/src/java.base/share/classes/java/util/concurrent/atomic/AtomicLongFieldUpdater.java +++ b/src/java.base/share/classes/java/util/concurrent/atomic/AtomicLongFieldUpdater.java @@ -398,6 +398,9 @@ public abstract class AtomicLongFieldUpdater { if (!Modifier.isVolatile(modifiers)) throw new IllegalArgumentException("Must be volatile type"); + if (Modifier.isStatic(modifiers)) + throw new IllegalArgumentException("Must not be a static field"); + // Access to protected field members is restricted to receivers only // of the accessing class, or one of its subclasses, and the // accessing class must in turn be a subclass (or package sibling) diff --git a/src/java.base/share/classes/java/util/concurrent/atomic/AtomicReferenceFieldUpdater.java b/src/java.base/share/classes/java/util/concurrent/atomic/AtomicReferenceFieldUpdater.java index e3ca4830d5a..4a758f77a47 100644 --- a/src/java.base/share/classes/java/util/concurrent/atomic/AtomicReferenceFieldUpdater.java +++ b/src/java.base/share/classes/java/util/concurrent/atomic/AtomicReferenceFieldUpdater.java @@ -363,6 +363,9 @@ public abstract class AtomicReferenceFieldUpdater { if (!Modifier.isVolatile(modifiers)) throw new IllegalArgumentException("Must be volatile type"); + if (Modifier.isStatic(modifiers)) + throw new IllegalArgumentException("Must not be a static field"); + // Access to protected field members is restricted to receivers only // of the accessing class, or one of its subclasses, and the // accessing class must in turn be a subclass (or package sibling) diff --git a/src/java.base/share/classes/jdk/internal/misc/Unsafe.java b/src/java.base/share/classes/jdk/internal/misc/Unsafe.java index ff854a8ecb3..016566ae659 100644 --- a/src/java.base/share/classes/jdk/internal/misc/Unsafe.java +++ b/src/java.base/share/classes/jdk/internal/misc/Unsafe.java @@ -1069,6 +1069,9 @@ public final class Unsafe { * the field locations in a form usable by {@link #getInt(Object,long)}. * Therefore, code which will be ported to such JVMs on 64-bit platforms * must preserve all bits of static field offsets. + * + * @throws NullPointerException if the field is {@code null} + * @throws IllegalArgumentException if the field is static * @see #getInt(Object, long) */ public long objectFieldOffset(Field f) { @@ -1080,13 +1083,17 @@ public final class Unsafe { } /** - * Reports the location of the field with a given name in the storage - * allocation of its class. + * (For compile-time known instance fields in JDK code only) Reports the + * location of the field with a given name in the storage allocation of its + * class. + *

+ * This API is used to avoid creating reflective Objects in Java code at + * startup. This should not be used to find fields in non-trusted code. + * Use the {@link #objectFieldOffset(Field) Field}-accepting version for + * arbitrary fields instead. * * @throws NullPointerException if any parameter is {@code null}. - * @throws InternalError if there is no field named {@code name} declared - * in class {@code c}, i.e., if {@code c.getDeclaredField(name)} - * would throw {@code java.lang.NoSuchFieldException}. + * @throws InternalError if the presumably known field couldn't be found * * @see #objectFieldOffset(Field) */ @@ -1095,7 +1102,16 @@ public final class Unsafe { throw new NullPointerException(); } - return objectFieldOffset1(c, name); + long result = knownObjectFieldOffset0(c, name); + if (result < 0) { + String type = switch ((int) result) { + case -2 -> "a static field"; + case -1 -> "not found"; + default -> "unknown"; + }; + throw new InternalError("Field %s.%s %s".formatted(c.getTypeName(), name, type)); + } + return result; } /** @@ -1113,6 +1129,9 @@ public final class Unsafe { * a few bits to encode an offset within a non-array object, * However, for consistency with other methods in this class, * this method reports its result as a long value. + * + * @throws NullPointerException if the field is {@code null} + * @throws IllegalArgumentException if the field is not static * @see #getInt(Object, long) */ public long staticFieldOffset(Field f) { @@ -1132,6 +1151,9 @@ public final class Unsafe { * which is a "cookie", not guaranteed to be a real Object, and it should * not be used in any way except as argument to the get and put routines in * this class. + * + * @throws NullPointerException if the field is {@code null} + * @throws IllegalArgumentException if the field is not static */ public Object staticFieldBase(Field f) { if (f == null) { @@ -3848,10 +3870,10 @@ public final class Unsafe { @IntrinsicCandidate private native void copyMemory0(Object srcBase, long srcOffset, Object destBase, long destOffset, long bytes); private native void copySwapMemory0(Object srcBase, long srcOffset, Object destBase, long destOffset, long bytes, long elemSize); - private native long objectFieldOffset0(Field f); - private native long objectFieldOffset1(Class c, String name); - private native long staticFieldOffset0(Field f); - private native Object staticFieldBase0(Field f); + private native long objectFieldOffset0(Field f); // throws IAE + private native long knownObjectFieldOffset0(Class c, String name); // error code: -1 not found, -2 static + private native long staticFieldOffset0(Field f); // throws IAE + private native Object staticFieldBase0(Field f); // throws IAE private native boolean shouldBeInitialized0(Class c); private native void ensureClassInitialized0(Class c); private native int arrayBaseOffset0(Class arrayClass); // public version returns long to promote correct arithmetic diff --git a/test/jdk/java/util/concurrent/tck/AtomicIntegerFieldUpdaterTest.java b/test/jdk/java/util/concurrent/tck/AtomicIntegerFieldUpdaterTest.java index edbf0f3ae86..91900e37b6a 100644 --- a/test/jdk/java/util/concurrent/tck/AtomicIntegerFieldUpdaterTest.java +++ b/test/jdk/java/util/concurrent/tck/AtomicIntegerFieldUpdaterTest.java @@ -44,6 +44,7 @@ public class AtomicIntegerFieldUpdaterTest extends JSR166TestCase { private volatile int privateField; int w; float z; + static volatile int q; public static void main(String[] args) { main(suite(), args); } @@ -88,6 +89,16 @@ public class AtomicIntegerFieldUpdaterTest extends JSR166TestCase { } catch (IllegalArgumentException success) {} } + /** + * construction with static field throws IllegalArgumentException + */ + public void testConstructor4() { + try { + updaterFor("q"); + shouldThrow(); + } catch (IllegalArgumentException success) {} + } + /** * construction using private field from subclass throws RuntimeException */ diff --git a/test/jdk/java/util/concurrent/tck/AtomicLongFieldUpdaterTest.java b/test/jdk/java/util/concurrent/tck/AtomicLongFieldUpdaterTest.java index 17689061426..5edcf871f42 100644 --- a/test/jdk/java/util/concurrent/tck/AtomicLongFieldUpdaterTest.java +++ b/test/jdk/java/util/concurrent/tck/AtomicLongFieldUpdaterTest.java @@ -44,6 +44,7 @@ public class AtomicLongFieldUpdaterTest extends JSR166TestCase { private volatile long privateField; long w; float z; + static volatile long q; public static void main(String[] args) { main(suite(), args); } @@ -88,6 +89,16 @@ public class AtomicLongFieldUpdaterTest extends JSR166TestCase { } catch (IllegalArgumentException success) {} } + /** + * construction with static field throws IllegalArgumentException + */ + public void testConstructor4() { + try { + updaterFor("q"); + shouldThrow(); + } catch (IllegalArgumentException success) {} + } + /** * construction using private field from subclass throws RuntimeException */ diff --git a/test/jdk/java/util/concurrent/tck/AtomicReferenceFieldUpdaterTest.java b/test/jdk/java/util/concurrent/tck/AtomicReferenceFieldUpdaterTest.java index 6df68c4c634..313f40f7918 100644 --- a/test/jdk/java/util/concurrent/tck/AtomicReferenceFieldUpdaterTest.java +++ b/test/jdk/java/util/concurrent/tck/AtomicReferenceFieldUpdaterTest.java @@ -45,6 +45,7 @@ public class AtomicReferenceFieldUpdaterTest extends JSR166TestCase { Object z; Item w; volatile int i; + static volatile Item q; public static void main(String[] args) { main(suite(), args); @@ -100,6 +101,17 @@ public class AtomicReferenceFieldUpdaterTest extends JSR166TestCase { } catch (ClassCastException success) {} } + /** + * construction with static field throws IllegalArgumentException + */ + public void testConstructor5() { + try { + updaterFor("q"); + shouldThrow(); + } catch (IllegalArgumentException success) {} + } + + /** * construction using private field from subclass throws RuntimeException */ diff --git a/test/jdk/jdk/internal/misc/Unsafe/AddressComputationContractTest.java b/test/jdk/jdk/internal/misc/Unsafe/AddressComputationContractTest.java new file mode 100644 index 00000000000..ebdfa843272 --- /dev/null +++ b/test/jdk/jdk/internal/misc/Unsafe/AddressComputationContractTest.java @@ -0,0 +1,104 @@ +/* + * 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. + */ + +import java.lang.reflect.Field; + +import org.junit.jupiter.api.Test; + +import static jdk.internal.misc.Unsafe.getUnsafe; +import static org.junit.jupiter.api.Assertions.*; + +/* + * @test + * @bug 8361300 + * @summary Verify Unsafe memory address computation method contracts, + * exposed via sun.misc.Unsafe + * @modules java.base/jdk.internal.misc + * @run junit AddressComputationContractTest + */ +public class AddressComputationContractTest { + + int instanceField; + static int staticField; + + private static final Field INSTANCE_FIELD; + private static final Field STATIC_FIELD; + + static { + try { + INSTANCE_FIELD = AddressComputationContractTest.class.getDeclaredField("instanceField"); + STATIC_FIELD = AddressComputationContractTest.class.getDeclaredField("staticField"); + } catch (ReflectiveOperationException ex) { + throw new ExceptionInInitializerError(ex); + } + } + + @Test + void objectFieldOffset() { + assertDoesNotThrow(() -> getUnsafe().objectFieldOffset(INSTANCE_FIELD)); + assertThrows(NullPointerException.class, () -> getUnsafe().objectFieldOffset(null)); + assertThrows(IllegalArgumentException.class, () -> getUnsafe().objectFieldOffset(STATIC_FIELD)); + } + + @Test + void knownObjectFieldOffset() { + assertDoesNotThrow(() -> getUnsafe().objectFieldOffset(AddressComputationContractTest.class, "instanceField")); + assertThrows(NullPointerException.class, () -> getUnsafe().objectFieldOffset(null, "instanceField")); + assertThrows(NullPointerException.class, () -> getUnsafe().objectFieldOffset(AddressComputationContractTest.class, null)); + // Two conventional failure cases, not necessarily complete + var dneMsg = assertThrows(InternalError.class, () -> getUnsafe().objectFieldOffset(AddressComputationContractTest.class, "doesNotExist")).getMessage(); + assertTrue(dneMsg.contains("AddressComputationContractTest.doesNotExist") && dneMsg.contains("not found"), dneMsg); + var staticMsg = assertThrows(InternalError.class, () -> getUnsafe().objectFieldOffset(AddressComputationContractTest.class, "staticField")).getMessage(); + assertTrue(staticMsg.contains("AddressComputationContractTest.staticField") && staticMsg.contains("static field"), staticMsg); + } + + @Test + void staticFieldOffset() { + assertDoesNotThrow(() -> getUnsafe().staticFieldOffset(STATIC_FIELD)); + assertThrows(NullPointerException.class, () -> getUnsafe().staticFieldOffset(null)); + assertThrows(IllegalArgumentException.class, () -> getUnsafe().staticFieldOffset(INSTANCE_FIELD)); + } + + @Test + void staticFieldBase() { + assertDoesNotThrow(() -> getUnsafe().staticFieldBase(STATIC_FIELD)); + assertThrows(NullPointerException.class, () -> getUnsafe().staticFieldBase(null)); + assertThrows(IllegalArgumentException.class, () -> getUnsafe().staticFieldBase(INSTANCE_FIELD)); + } + + @Test + void arrayBaseOffset() { + assertDoesNotThrow(() -> getUnsafe().arrayBaseOffset(int[].class)); + assertThrows(NullPointerException.class, () -> getUnsafe().arrayBaseOffset(null)); + // Caused by VM trying to throw java.lang.InvalidClassException (there's one in java.io instead) + assertThrows(NoClassDefFoundError.class, () -> getUnsafe().arrayBaseOffset(AddressComputationContractTest.class)); + } + + @Test + void arrayIndexScale() { + assertDoesNotThrow(() -> getUnsafe().arrayIndexScale(int[].class)); + assertThrows(NullPointerException.class, () -> getUnsafe().arrayIndexScale(null)); + // Caused by VM trying to throw java.lang.InvalidClassException (there's one in java.io instead) + assertThrows(NoClassDefFoundError.class, () -> getUnsafe().arrayIndexScale(AddressComputationContractTest.class)); + } +}