Skip to content

Commit

Permalink
JEP 384: Record final fields not modifiable through reflection
Browse files Browse the repository at this point in the history
Related to: #9929
Fix: #9529

- implement restrictions on record final fields through reflection
- test that final fields in records cant be modified through reflection
- test that jdk.internal.misc.Unsafe methods objectFieldOffset/staticFieldOffset/staticFieldBase support records
- test that there is no record support for VarHandle.set
Signed-off-by: Theresa Mammarella <Theresa.T.Mammarella@ibm.com>
  • Loading branch information
theresa-m committed Jul 28, 2020
1 parent ea62a4f commit 9530444
Show file tree
Hide file tree
Showing 6 changed files with 202 additions and 4 deletions.
9 changes: 7 additions & 2 deletions runtime/jcl/common/reflecthelp.c
Original file line number Diff line number Diff line change
Expand Up @@ -830,8 +830,13 @@ createField(struct J9VMThread *vmThread, jfieldID fieldID)
#if defined(USE_SUN_REFLECT)
J9VMJAVALANGREFLECTFIELD_SET_MODIFIERS(vmThread, fieldObject, j9FieldID->field->modifiers & CFR_FIELD_ACCESS_MASK);
#if JAVA_SPEC_VERSION >= 15
if (J9_ARE_ALL_BITS_SET(j9FieldID->field->modifiers, J9AccFinal | J9AccStatic)) {
J9VMJAVALANGREFLECTFIELD_SET_TRUSTEDFINAL(vmThread, fieldObject, JNI_TRUE);
/* trust that static final fields and final record class fields will not be modified. */
if (J9_ARE_ALL_BITS_SET(j9FieldID->field->modifiers, J9AccFinal)) {
if (J9_ARE_ALL_BITS_SET(j9FieldID->field->modifiers, J9AccStatic)
|| J9ROMCLASS_IS_RECORD(j9FieldID->declaringClass->romClass))
{
J9VMJAVALANGREFLECTFIELD_SET_TRUSTEDFINAL(vmThread, fieldObject, JNI_TRUE);
}
}
#endif /* JAVA_SPEC_VERSION >= 15 */
#endif
Expand Down
3 changes: 2 additions & 1 deletion test/functional/Java15andUp/build.xml
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@

<javac srcdir="${src}" destdir="${build}" debug="true" fork="true" executable="${compiler.javac}" includeAntRuntime="false" encoding="ISO-8859-1">
<src path="${src}"/>
<compilerarg line='--enable-preview --release ${JDK_VERSION}' />
<compilerarg line='--add-exports java.base/jdk.internal.misc=ALL-UNNAMED' />
<compilerarg line='--enable-preview --source ${JDK_VERSION}' />
<classpath>
<pathelement location="${LIB_DIR}/testng.jar"/>
<pathelement location="${LIB_DIR}/jcommander.jar"/>
Expand Down
24 changes: 24 additions & 0 deletions test/functional/Java15andUp/playlist.xml
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,28 @@
<subset>15</subset>
</subsets>
</test>
<test>
<testCaseName>Jep384Tests</testCaseName>
<variations>
<variation>--enable-preview</variation>
</variations>
<command>$(JAVA_COMMAND) $(JVM_OPTIONS) \
--add-exports=java.base/jdk.internal.misc=ALL-UNNAMED \
-cp $(Q)$(LIB_DIR)$(RESOURCES_DIR)$(P)$(TESTNG)$(P)$(TEST_RESROOT)$(D)GeneralTest.jar$(Q) \
org.testng.TestNG -d $(REPORTDIR) $(Q)$(TEST_RESROOT)$(D)testng.xml$(Q) -testnames Jep384Tests \
-groups $(TEST_GROUP) \
-excludegroups $(DEFAULT_EXCLUDE); \
$(TEST_STATUS)
</command>
<levels>
<level>sanity</level>
</levels>
<groups>
<group>functional</group>
</groups>
<subsets>
<!-- run for Java 15 only since this is a preview feature. -->
<subset>15</subset>
</subsets>
</test>
</playlist>
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
*******************************************************************************/

import org.testng.annotations.Test;
import org.testng.log4testng.Logger;

import org.testng.AssertJUnit;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,164 @@
package org.openj9.test.records;

/*******************************************************************************
* Copyright (c) 2020, 2020 IBM Corp. and others
*
* This program and the accompanying materials are made available under
* the terms of the Eclipse Public License 2.0 which accompanies this
* distribution and is available at https://www.eclipse.org/legal/epl-2.0/
* or the Apache License, Version 2.0 which accompanies this distribution and
* is available at https://www.apache.org/licenses/LICENSE-2.0.
*
* This Source Code may also be made available under the following
* Secondary Licenses when the conditions for such availability set
* forth in the Eclipse Public License, v. 2.0 are satisfied: GNU
* General Public License, version 2 with the GNU Classpath
* Exception [1] and GNU General Public License, version 2 with the
* OpenJDK Assembly Exception [2].
*
* [1] https://www.gnu.org/software/classpath/license.html
* [2] http://openjdk.java.net/legal/assembly-exception.html
*
* SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 OR LicenseRef-GPL-2.0 WITH Assembly-exception
*******************************************************************************/

import org.testng.annotations.Test;
import org.testng.AssertJUnit;

import java.lang.invoke.MethodHandles;
import java.lang.invoke.VarHandle;
import java.lang.reflect.Field;
import jdk.internal.misc.Unsafe;

@Test(groups = { "level.sanity" })
public class RecordFinalFieldTests {

static Unsafe unsafe = Unsafe.getUnsafe();

record TestRecord(Integer recordComponent) {
static String modifiableField = "old";
static final String finalField = "old";
}

/* All generated record component fields are final and are not modifiable through reflection. */
@Test(expectedExceptions = java.lang.IllegalAccessException.class)
public void test_javaLangReflectFieldSet_finalInstanceRecordField() throws Throwable {
TestRecord recordClassObject = new TestRecord(0);
Field finalRecordField = recordClassObject.getClass().getDeclaredField("recordComponent");
finalRecordField.setAccessible(true);
finalRecordField.set(recordClassObject, 5);
}

/* Final static fields in record classes are not modifiable through reflection. */
@Test(expectedExceptions = java.lang.IllegalAccessException.class)
public void test_javaLangReflectFieldSet_finalStaticRecordField() throws Throwable {
TestRecord recordClassObject = new TestRecord(0);
Field finalRecordField = recordClassObject.getClass().getDeclaredField("finalField");
finalRecordField.setAccessible(true);
finalRecordField.set(recordClassObject, "new");
}

/* Make sure non-final fields in record classes can still be modified. */
@Test
public void test_javaLangReflectFieldSet_staticRecordField() throws Throwable {
TestRecord recordClassObject = new TestRecord(0);
Field recordField = recordClassObject.getClass().getDeclaredField("modifiableField");
recordField.setAccessible(true);
recordField.set(recordClassObject, "new");
}

/* Verify behavior of setting final instance field in a record constructor. */
record TestRecordWithConstructorSettingFinalInstance(Integer recordComponent) {
TestRecordWithConstructorSettingFinalInstance {
recordComponent = 0;
}
}

@Test
public void test_recordClassConstructor_setFinalInstanceField() {
TestRecordWithConstructorSettingFinalInstance record = new TestRecordWithConstructorSettingFinalInstance(0);
}

/* Verify behavior of setting final static field in a record constructor. */
record TestRecordWithConstructorSettingFinalStatic() {
static final String finalField = "old";

TestRecordWithConstructorSettingFinalStatic {
try {
Field finalRecordField = this.getClass().getDeclaredField("finalField");
finalRecordField.setAccessible(true);
finalRecordField.set(this, "new");
AssertJUnit.fail("No exception was thrown.");
} catch(IllegalAccessException e) {
/* expected exception - test passed*/
} catch(Throwable e) {
AssertJUnit.fail(e.getMessage());
}
}
}

@Test
public void test_recordClassConstructor_setFinalStaticField() {
TestRecordWithConstructorSettingFinalStatic record = new TestRecordWithConstructorSettingFinalStatic();
}

/* Verify behavior of setting static field in a record constructor. */
record TestRecordWithConstructorSettingModifiableStatic() {
static String modifiableField = "old";

TestRecordWithConstructorSettingModifiableStatic {
try {
Field finalRecordField = this.getClass().getDeclaredField("modifiableField");
finalRecordField.setAccessible(true);
finalRecordField.set(this, "new");
} catch(Throwable e) {
AssertJUnit.fail(e.getMessage());
}
}
}

@Test
public void test_recordClassConstructor_setStaticField() {
TestRecordWithConstructorSettingModifiableStatic record = new TestRecordWithConstructorSettingModifiableStatic();
}

/* Check that Unsafe.objectFieldOffset supports records. */
@Test
public void test_jdkInternalMiscUnsafe_objectFieldOffset() throws Throwable {
Field finalRecordField = TestRecord.class.getDeclaredField("recordComponent");
unsafe.objectFieldOffset(finalRecordField);
}

/* Check that Unsafe.staticFieldBase supports records. */
@Test
public void test_jdkInternalMiscUnsafe_staticFieldOffset() throws Throwable {
Field finalRecordField = TestRecord.class.getDeclaredField("finalField");
unsafe.staticFieldOffset(finalRecordField);
}

/* Check that Unsafe.staticFieldBase supports records. */
@Test
public void test_jdkInternalMiscUnsafe_staticFieldBase() throws Throwable {
Field finalRecordField = TestRecord.class.getDeclaredField("finalField");
unsafe.staticFieldBase(finalRecordField);
}

/* VarHandle.set is not supported for records. */
// TODO this should be UnsupportedOperationException - openj9 is throwing WrongMethodTypeException from method handles code which is being replaced.
@Test(expectedExceptions = java.lang.Exception.class)
public void test_javaLangInvokeMethodHandles_unreflectVarHandle_instance() throws Throwable {
Field finalRecordField = TestRecord.class.getDeclaredField("recordComponent");
finalRecordField.setAccessible(true);
VarHandle finalRecordFieldHandle = MethodHandles.lookup().unreflectVarHandle(finalRecordField);
finalRecordFieldHandle.set(new Integer(5));
}

/* VarHandle.set is not supported for records. */
@Test(expectedExceptions = java.lang.UnsupportedOperationException.class)
public void test_javaLangInvokeMethodHandles_unreflectVarHandle_static() throws Throwable {
Field finalRecordField = TestRecord.class.getDeclaredField("finalField");
finalRecordField.setAccessible(true);
VarHandle finalRecordFieldHandle = MethodHandles.lookup().unreflectVarHandle(finalRecordField);
finalRecordFieldHandle.set("new");
}
}
5 changes: 5 additions & 0 deletions test/functional/Java15andUp/testng.xml
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,9 @@
<class name="org.openj9.test.sealedclasses.SealedClassesTests"/>
</classes>
</test>
<test name="Jep384Tests">
<classes>
<class name="org.openj9.test.records.RecordFinalFieldTests"/>
</classes>
</test>
</suite>

0 comments on commit 9530444

Please sign in to comment.