Skip to content

Commit 0bfd27d

Browse files
hangshao0hangshao0
authored
Merge pull request #22899 from AditiS11/Issue-22697
Add JEP 401 verification for strict instance fields
2 parents eb02540 + f11e13e commit 0bfd27d

8 files changed

Lines changed: 203 additions & 26 deletions

File tree

‎runtime/bcutil/cfreader.c‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1687,6 +1687,9 @@ checkFields(J9PortLibrary* portLib, J9CfrClassFile * classfile, U_8 * segment, U
16871687
U_32 value, maskedValue, errorCode, offset = 0;
16881688
U_32 i;
16891689
U_8 sigChar, sigTag, constantTag;
1690+
#if defined(J9VM_OPT_VALHALLA_VALUE_TYPES)
1691+
BOOLEAN valueTypeClass = J9_IS_CLASSFILE_VALUETYPE(classfile);
1692+
#endif /* defined(J9VM_OPT_VALHALLA_VALUE_TYPES) */
16901693

16911694
for (i = 0; i < classfile->fieldsCount; i++) {
16921695
field = &(classfile->fields[i]);
@@ -1718,6 +1721,17 @@ checkFields(J9PortLibrary* portLib, J9CfrClassFile * classfile, U_8 * segment, U
17181721
goto _errorFound;
17191722
}
17201723

1724+
#if defined(J9VM_OPT_VALHALLA_VALUE_TYPES)
1725+
if (valueTypeClass) {
1726+
if (J9_ARE_NO_BITS_SET(field->accessFlags, CFR_ACC_STATIC)) {
1727+
if (!J9_ARE_ALL_BITS_SET(field->accessFlags, CFR_ACC_FINAL | CFR_ACC_STRICT_INIT)) {
1728+
errorCode = J9NLS_CFR_ERR_NON_STATIC_FIELD_MUST_BE_FINAL_STRICT_INIT__ID;
1729+
goto _errorFound;
1730+
}
1731+
}
1732+
}
1733+
#endif /* defined(J9VM_OPT_VALHALLA_VALUE_TYPES) */
1734+
17211735
offset = 2;
17221736
value = field->nameIndex;
17231737
if ((!value) || (value >= classfile->constantPoolCount)) {

‎runtime/bcverify/rtverify.c‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1083,6 +1083,14 @@ verifyBytecodes (J9BytecodeVerificationData * verifyData)
10831083
errorStackIndex = stackTop - liveStack->stackElements;
10841084
goto _inconsistentStack;
10851085
}
1086+
} else {
1087+
#if defined(J9VM_OPT_VALHALLA_VALUE_TYPES)
1088+
if ((JBifacmpeq == bc) || (JBifacmpne == bc) || (JBifnonnull == bc) || (JBifnull == bc)) {
1089+
errorType = J9NLS_BCV_ERR_UNINITIALIZED_VALUE_OBJECT__ID;
1090+
storeVerifyErrorData(verifyData, (I_16)bc, (U_32)errorStackIndex, (UDATA)-1, (UDATA)-1, start);
1091+
goto _verifyError;
1092+
}
1093+
#endif /* defined(J9VM_OPT_VALHALLA_VALUE_TYPES) */
10861094
}
10871095
} else {
10881096
POP_TOS_INTEGER;

‎runtime/nls/cfre/cfrerr.nls‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1833,3 +1833,10 @@ J9NLS_CFR_ERR_LOADABLEDESCRIPTORS_ENTRY_NOT_UTF8_TYPE.explanation=Please consult
18331833
J9NLS_CFR_ERR_LOADABLEDESCRIPTORS_ENTRY_NOT_UTF8_TYPE.system_action=The JVM will throw a verification or classloading related exception such as java.lang.ClassFormatError.
18341834
J9NLS_CFR_ERR_LOADABLEDESCRIPTORS_ENTRY_NOT_UTF8_TYPE.user_response=Contact the provider of the classfile for a corrected version.
18351835
# END NON-TRANSLATABLE
1836+
1837+
J9NLS_CFR_ERR_NON_STATIC_FIELD_MUST_BE_FINAL_STRICT_INIT=Non-static field declared in a value class does not have its ACC_FINAL and ACC_STRICT_INIT flags set.
1838+
# START NON-TRANSLATABLE
1839+
J9NLS_CFR_ERR_NON_STATIC_FIELD_MUST_BE_FINAL_STRICT_INIT.explanation=In a valid value class, a field without its ACC_STATIC flag set must have ACC_FINAL and ACC_STRICT_INIT flags set.
1840+
J9NLS_CFR_ERR_NON_STATIC_FIELD_MUST_BE_FINAL_STRICT_INIT.system_action=The JVM will throw a verification or classloading exception such as java.lang.ClassFormatError.
1841+
J9NLS_CFR_ERR_NON_STATIC_FIELD_MUST_BE_FINAL_STRICT_INIT.user_response=Contact the provider of the classfile for a corrected version.
1842+
# END NON-TRANSLATABLE

‎runtime/nls/vrfy/j9bcv.nls‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -438,3 +438,11 @@ J9NLS_BCV_ERR_BC_NEW_ARRAY.system_action=The JVM will throw a verification or cl
438438
J9NLS_BCV_ERR_BC_NEW_ARRAY.user_response=Contact the provider of the classfile for a corrected version.
439439

440440
# END NON-TRANSLATABLE
441+
442+
J9NLS_BCV_ERR_UNINITIALIZED_VALUE_OBJECT=Uninitialized value object used with acmpeq, acmpne, ifnonnull, or ifnull.
443+
# START NON-TRANSLATABLE
444+
J9NLS_BCV_ERR_UNINITIALIZED_VALUE_OBJECT.explanation=Verification failed because a newly created but uninitialized value object was used with if_acmpeq, if_acmpne, ifnonnull or ifnull.
445+
J9NLS_BCV_ERR_UNINITIALIZED_VALUE_OBJECT.system_action=The JVM will throw a verification or classloading related exception such as java.lang.VerifyError.
446+
J9NLS_BCV_ERR_UNINITIALIZED_VALUE_OBJECT.user_response=Contact the provider of the classfile for a corrected version.
447+
448+
# END NON-TRANSLATABLE

‎test/functional/Valhalla/src/org/openj9/test/lworld/StrictFieldGenerator.java‎

Lines changed: 68 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,6 @@
2828
public class StrictFieldGenerator extends ClassLoader {
2929
private static StrictFieldGenerator generator = new StrictFieldGenerator();
3030

31-
// ACC_STRICT_INIT is not yet supported by ASM
32-
private static final int ACC_STRICT_INIT = ACC_STRICT;
33-
3431
public static Class<?> generateTestPutStrictFinalFieldLateLarval() {
3532
String className = "TestPutStrictFinalFieldLateLarval";
3633
return generateTestPutStrictFinalField(className, true, false);
@@ -50,7 +47,7 @@ private static Class<?> generateTestPutStrictFinalField(String className, boolea
5047
ACC_PUBLIC + ValhallaUtils.ACC_IDENTITY,
5148
className, null, "java/lang/Object", null);
5249

53-
cw.visitField(ACC_STRICT_INIT | ACC_FINAL, fieldName, fieldDesc, null, null);
50+
cw.visitField(ValhallaUtils.ACC_STRICT_INIT | ACC_FINAL, fieldName, fieldDesc, null, null);
5451

5552
MethodVisitor mv = cw.visitMethod(ACC_PUBLIC, "<init>", "()V", null, null);
5653
mv.visitCode();
@@ -114,9 +111,9 @@ private static Class<?> generateTestPutGetStrictStaticField(String className, bo
114111
ACC_PUBLIC + ValhallaUtils.ACC_IDENTITY,
115112
className, null, "java/lang/Object", null);
116113

117-
cw.visitField(ACC_STATIC | ACC_STRICT_INIT | ACC_FINAL, fieldName, fieldDesc, null, null);
114+
cw.visitField(ACC_STATIC | ValhallaUtils.ACC_STRICT_INIT | ACC_FINAL, fieldName, fieldDesc, null, null);
118115
if (secondField) {
119-
cw.visitField(ACC_STATIC | ACC_STRICT_INIT | ACC_FINAL, fieldName2, fieldDesc, null, null);
116+
cw.visitField(ACC_STATIC | ValhallaUtils.ACC_STRICT_INIT | ACC_FINAL, fieldName2, fieldDesc, null, null);
120117
}
121118

122119
MethodVisitor mv = cw.visitMethod(ACC_STATIC, "<clinit>", "()V", null, null);
@@ -176,10 +173,10 @@ private static Class<?> generateTestInstanceStrictField(String className, boolea
176173
ACC_PUBLIC + ValhallaUtils.ACC_IDENTITY,
177174
className, null, "java/lang/Object", null);
178175

179-
cw.visitField(ACC_STRICT_INIT, fieldName, fieldDesc, null, null);
176+
cw.visitField(ValhallaUtils.ACC_STRICT_INIT, fieldName, fieldDesc, null, null);
180177
if (multipleFields) {
181-
cw.visitField(ACC_STRICT_INIT, fieldName2, fieldDesc, null, null);
182-
cw.visitField(ACC_STRICT_INIT, fieldName3, fieldDesc, null, null);
178+
cw.visitField(ValhallaUtils.ACC_STRICT_INIT, fieldName2, fieldDesc, null, null);
179+
cw.visitField(ValhallaUtils.ACC_STRICT_INIT, fieldName3, fieldDesc, null, null);
183180
}
184181

185182
MethodVisitor mv = cw.visitMethod(ACC_PUBLIC, "<init>", "()V", null, null);
@@ -217,7 +214,7 @@ static Class<?> generateTestInstanceStrictNonFinalFieldSetAfterThisInit() {
217214
private static Class<?> generateTestInstanceStrictFieldThisInit(String className, boolean finalField) {
218215
String fieldName = "i";
219216
String fieldDesc = "I";
220-
int fieldFlags = ACC_STRICT_INIT | (finalField ? ACC_FINAL : 0);
217+
int fieldFlags = ValhallaUtils.ACC_STRICT_INIT | (finalField ? ACC_FINAL : 0);
221218

222219
ClassWriter cw = new ClassWriter(0);
223220
cw.visit(ValhallaUtils.VALUE_TYPE_CLASS_FILE_VERSION,
@@ -254,4 +251,65 @@ private static Class<?> generateTestInstanceStrictFieldThisInit(String className
254251
byte[] bytes = cw.toByteArray();
255252
return generator.defineClass(className, cw.toByteArray(), 0, bytes.length);
256253
}
254+
255+
public static Class<?> generateTestIfNonNullUninitializedThis() {
256+
String className = "TestIfNonNullUninitializedThis";
257+
return generateUninitializedValueField(className, false, true, false, false);
258+
}
259+
260+
public static Class<?> generateTestIfNullUninitializedThis() {
261+
String className = "TestIfNullUninitializedThis";
262+
return generateUninitializedValueField(className, true, false, false, false);
263+
}
264+
265+
public static Class<?> generateTestIfAcmpeqUninitializedValue() {
266+
String className = "TestIfAcmpeqUninitializedValue";
267+
return generateUninitializedValueField(className, false, false, true, false);
268+
}
269+
270+
public static Class<?> generateTestIfAcmpneUninitializedValue() {
271+
String className = "TestIfAcmpneUninitializedValue";
272+
return generateUninitializedValueField(className, false, false, false, true);
273+
}
274+
275+
private static Class<?> generateUninitializedValueField(String className, boolean ifNull, boolean ifNonNull, boolean ifAcmpeq, boolean ifAcmpne) {
276+
ClassWriter cw = new ClassWriter(0);
277+
cw.visit(ValhallaUtils.VALUE_TYPE_CLASS_FILE_VERSION,
278+
ACC_PUBLIC + ValhallaUtils.ACC_IDENTITY,
279+
className, null, "java/lang/Object", null);
280+
MethodVisitor mv = cw.visitMethod(ACC_PUBLIC, "<init>", "()V", null, null);
281+
mv.visitCode();
282+
Label label = new Label();
283+
284+
if (ifNull || ifNonNull) {
285+
mv.visitVarInsn(ALOAD, 0);
286+
if (ifNull) {
287+
mv.visitJumpInsn(IFNULL, label);
288+
} else {
289+
mv.visitJumpInsn(IFNONNULL, label);
290+
}
291+
mv.visitInsn(NOP);
292+
}
293+
294+
if (ifAcmpeq || ifAcmpne) {
295+
mv.visitTypeInsn(NEW, "java/lang/Object");
296+
mv.visitInsn(DUP);
297+
mv.visitVarInsn(ALOAD, 0);
298+
if (ifAcmpeq) {
299+
mv.visitJumpInsn(IF_ACMPEQ, label);
300+
} else {
301+
mv.visitJumpInsn(IF_ACMPNE, label);
302+
}
303+
mv.visitInsn(NOP);
304+
}
305+
mv.visitLabel(label);
306+
mv.visitVarInsn(ALOAD, 0);
307+
mv.visitMethodInsn(INVOKESPECIAL, "java/lang/Object", "<init>", "()V", false);
308+
mv.visitInsn(RETURN);
309+
mv.visitMaxs(3, 1);
310+
mv.visitEnd();
311+
cw.visitEnd();
312+
byte[] bytes = cw.toByteArray();
313+
return generator.defineClass(className, bytes, 0, bytes.length);
314+
}
257315
}

‎test/functional/Valhalla/src/org/openj9/test/lworld/StrictFieldTests.java‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,4 +139,44 @@ static public void testInstanceStrictNonFinalFieldSetAfterThisInit() throws Thro
139139
Class<?> c = StrictFieldGenerator.generateTestInstanceStrictNonFinalFieldSetAfterThisInit();
140140
c.newInstance();
141141
}
142+
143+
/* If an uninitialized 'this' reference is used with IFNONNULL,
144+
* verification must fail with a VerifyError.
145+
*/
146+
@Test(expectedExceptions = VerifyError.class,
147+
expectedExceptionsMessageRegExp = ".*<init>.*JBifnonnull.*")
148+
static public void testIfNonNullUninitializedThis() throws Throwable {
149+
Class<?> c = StrictFieldGenerator.generateTestIfNonNullUninitializedThis();
150+
c.newInstance();
151+
}
152+
153+
/* If an uninitialized 'this' reference is used with IFNULL,
154+
* verification must fail with a VerifyError.
155+
*/
156+
@Test(expectedExceptions = VerifyError.class,
157+
expectedExceptionsMessageRegExp = ".*<init>.*JBifnull.*")
158+
static public void testIfNullUninitializedThis() throws Throwable {
159+
Class<?> c = StrictFieldGenerator.generateTestIfNullUninitializedThis();
160+
c.newInstance();
161+
}
162+
163+
/* If a newly created value object of uninitialized type is used with IF_ACMPEQ
164+
* verification must fail with a VerifyError.
165+
*/
166+
@Test(expectedExceptions = VerifyError.class,
167+
expectedExceptionsMessageRegExp = ".*<init>.*JBifacmpeq.*")
168+
static public void testIfAcmpeqUninitializedValue() throws Throwable {
169+
Class<?> c = StrictFieldGenerator.generateTestIfAcmpeqUninitializedValue();
170+
c.newInstance();
171+
}
172+
173+
/* If a newly created value object of uninitialized type is used with IF_ACMPNE
174+
* verification must fail with a VerifyError.
175+
*/
176+
@Test(expectedExceptions = VerifyError.class,
177+
expectedExceptionsMessageRegExp = ".*<init>.*JBifacmpne.*")
178+
static void testIfAcmpneUninitializedValue() throws Throwable {
179+
Class<?> c = StrictFieldGenerator.generateTestIfAcmpneUninitializedValue();
180+
c.newInstance();
181+
}
142182
}

‎test/functional/Valhalla/src/org/openj9/test/lworld/ValhallaUtils.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323

2424
import org.objectweb.asm.*;
2525

26+
import static org.objectweb.asm.Opcodes.*;
27+
2628
public class ValhallaUtils {
2729
/**
2830
* Currently value type is built on JDK26, so use java file major version (44 + 26) for now.
@@ -34,6 +36,7 @@ public class ValhallaUtils {
3436

3537
/* workaround till the new ASM is released */
3638
static final int ACC_IDENTITY = 0x20;
39+
static final int ACC_STRICT_INIT = ACC_STRICT;
3740

3841
/* ImplicitCreation flags */
3942
static final int ACC_DEFAULT = 0x1;

‎test/functional/Valhalla/src_qtypes/org/openj9/test/lworld/ValueTypeGenerator.java‎

Lines changed: 55 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525

2626
import static org.objectweb.asm.Opcodes.*;
2727

28+
import static org.openj9.test.lworld.ValhallaUtils.ACC_STRICT_INIT;
29+
2830
import java.io.FileNotFoundException;
2931
import java.io.FileOutputStream;
3032
import java.io.IOException;
@@ -228,7 +230,9 @@ private static byte[] generateClass(ClassConfiguration config) {
228230
int makeMaxLocal = 0;
229231
String makeValueSig = "";
230232
String makeValueGenericSig = "";
231-
for (String s : fields) {
233+
boolean[] isStrictInitField = new boolean[fields.length];
234+
for (int i = 0; i < fields.length; i++) {
235+
String s = fields[i];
232236
String[] nameAndSigValue = s.split(":");
233237
final int fieldModifiers;
234238
if (isRef) {
@@ -237,16 +241,17 @@ private static byte[] generateClass(ClassConfiguration config) {
237241
if ((nameAndSigValue.length > 2) && nameAndSigValue[2].equals("static")) {
238242
fieldModifiers = ACC_PUBLIC + ACC_STATIC;
239243
} else if ((nameAndSigValue.length > 3) && nameAndSigValue[3].equals("volatile")) {
240-
fieldModifiers = ACC_PUBLIC + ACC_FINAL + ACC_VOLATILE;
244+
fieldModifiers = ACC_PUBLIC + ACC_FINAL + ACC_VOLATILE + ACC_STRICT_INIT;
241245
} else {
242-
fieldModifiers = ACC_PUBLIC + ACC_FINAL;
246+
fieldModifiers = ACC_PUBLIC + ACC_FINAL + ACC_STRICT_INIT;
243247
}
244248
}
245249
fv = cw.visitField(fieldModifiers, nameAndSigValue[0], nameAndSigValue[1], null, null);
246250
if ((nameAndSigValue.length > 2) && nameAndSigValue[2].equals("NR")) {
247251
fv.visitAttribute(new ValhallaUtils.NullRestrictedAttribute());
248252
}
249253
fv.visitEnd();
254+
isStrictInitField[i] = (fieldModifiers & ACC_STRICT_INIT) != 0;
250255
if ((nameAndSigValue.length <= 2) || !nameAndSigValue[2].equals("static")) {
251256
makeValueSig += nameAndSigValue[1];
252257
makeValueGenericSig += "Ljava/lang/Object;";
@@ -260,7 +265,7 @@ private static byte[] generateClass(ClassConfiguration config) {
260265
generateFieldMethods(cw, nameAndSigValue, className, isVerifiable, isRef);
261266
}
262267

263-
addInit(cw);
268+
addInit(cw, className, fields, isStrictInitField);
264269
if ("" != makeValueSig) {
265270
addInitWithArgs(cw, className, makeValueSig, fields, makeMaxLocal);
266271
}
@@ -339,13 +344,54 @@ private static void generateFieldMethods(ClassWriter cw, String[] nameAndSigValu
339344
}
340345
}
341346

342-
private static void addInit(ClassWriter cw) {
347+
private static void addInit(ClassWriter cw, String className, String[] fields, boolean[] isStrictInitField) {
343348
MethodVisitor mv = cw.visitMethod(ACC_PUBLIC, "<init>", "()V", null, null);
344349
mv.visitCode();
350+
boolean needsWideStack = false;
351+
for (int i = 0; i < fields.length; i++) {
352+
if (!isStrictInitField[i]) continue;
353+
String[] nameAndSigValue = fields[i].split(":");
354+
String sig = nameAndSigValue[1];
355+
mv.visitVarInsn(ALOAD, 0);
356+
switch (sig.charAt(0)) {
357+
case 'J':
358+
mv.visitInsn(LCONST_0);
359+
needsWideStack = true;
360+
break;
361+
case 'D':
362+
mv.visitInsn(DCONST_0);
363+
needsWideStack = true;
364+
break;
365+
case 'F':
366+
mv.visitInsn(FCONST_0);
367+
break;
368+
case 'I':
369+
case 'S':
370+
case 'B':
371+
case 'C':
372+
case 'Z':
373+
mv.visitInsn(ICONST_0);
374+
break;
375+
default:
376+
boolean isNullRestricted = (nameAndSigValue.length > 2) && "NR".equals(nameAndSigValue[2]);
377+
if (isNullRestricted) {
378+
String fieldTypeName = sig.substring(1, sig.length() - 1);
379+
mv.visitTypeInsn(NEW, fieldTypeName);
380+
mv.visitInsn(DUP);
381+
mv.visitMethodInsn(INVOKESPECIAL, fieldTypeName, "<init>", "()V", false);
382+
needsWideStack = true;
383+
} else {
384+
mv.visitInsn(ACONST_NULL);
385+
}
386+
break;
387+
}
388+
mv.visitFieldInsn(PUTFIELD, className, nameAndSigValue[0], sig);
389+
}
345390
mv.visitVarInsn(ALOAD, 0);
346391
mv.visitMethodInsn(INVOKESPECIAL, "java/lang/Object", "<init>", "()V", false);
347392
mv.visitInsn(RETURN);
348-
mv.visitMaxs(1, 1);
393+
int maxStack = needsWideStack ? 3 : 2;
394+
mv.visitMaxs(maxStack, 1);
349395
mv.visitEnd();
350396
}
351397

@@ -621,10 +667,7 @@ private static void makeGeneric(ClassWriter cw, String className, String methodN
621667
break;
622668
default:
623669
String signature = nameAndSigValue[1];
624-
625-
if ('L' == signature.charAt(0)) {
626-
signature = signature.substring(1, signature.length() - 1);
627-
}
670+
signature = signature.substring(1, signature.length() - 1);
628671
mv.visitTypeInsn(CHECKCAST, signature);
629672
break;
630673
}
@@ -793,9 +836,7 @@ private static void generateStaticSetterGeneric(ClassWriter cw, String[] nameAnd
793836
doubleDetected = true;
794837
break;
795838
default:
796-
if ((nameAndSigValue[1].length() >= 1) && (nameAndSigValue[1].charAt(0) == 'L')) {
797-
mv.visitTypeInsn(CHECKCAST, nameAndSigValue[1].substring(1, nameAndSigValue[1].length() - 1));
798-
}
839+
mv.visitTypeInsn(CHECKCAST, nameAndSigValue[1].substring(1, nameAndSigValue[1].length() - 1));
799840
break;
800841
}
801842
mv.visitMethodInsn(INVOKESTATIC, className, "setStatic" + nameAndSigValue[0], "(" + nameAndSigValue[1] + ")V", false);
@@ -847,9 +888,7 @@ private static void generateSetterGeneric(ClassWriter cw, String[] nameAndSigVal
847888
doubleDetected = true;
848889
break;
849890
default:
850-
if ((nameAndSigValue[1].length() >= 1) && (nameAndSigValue[1].charAt(0) == 'L')) {
851-
mv.visitTypeInsn(CHECKCAST, nameAndSigValue[1].substring(1, nameAndSigValue[1].length() - 1));
852-
}
891+
mv.visitTypeInsn(CHECKCAST, nameAndSigValue[1].substring(1, nameAndSigValue[1].length() - 1));
853892
break;
854893
}
855894
mv.visitMethodInsn(INVOKEVIRTUAL, className, "set" + nameAndSigValue[0], "(" + nameAndSigValue[1] + ")V", false);

0 commit comments

Comments
 (0)