RFR: 8317356: Test ClassFile API if it deals with nulls correctly across the whole API
Nizar Benalla
nbenalla at openjdk.org
Wed Sep 4 10:36:51 UTC 2024
The test is inspired from [FFM API's TestNulls](https://github.com/openjdk/jdk/blob/master/test/jdk/java/foreign/TestNulls.java), I customized their Null checking framework it to work with ClassFile API.
The framework for for testing an API in bulk, so that all methods are reflectively called with some values replaced with nulls, so that all combinations are tried.
This test reveals some inconsistencies in the ClassFile API, whether it's a method with nullable arguments`[1]`, nulls are passed to`[2]` or its implementation handles nulls `[3]` `[4]`.
`[1]:` [ModuleRequireInfo#of](https://github.com/openjdk/jdk/blob/25e03b52094f46f89f2fe8f20e7e5622928add5f/src/java.base/share/classes/java/lang/classfile/attribute/ModuleRequireInfo.java#L89)
`[2]:` [Signature$ClassTypeSig#of](https://github.com/openjdk/jdk/blob/7ad61605f1669f51a97f4f263a7afaa9ab7706be/src/java.base/share/classes/java/lang/classfile/Signature.java#L150)
`[3]:` [CatchBuilderImpl#catching](https://github.com/openjdk/jdk/blob/7ad61605f1669f51a97f4f263a7afaa9ab7706be/src/java.base/share/classes/jdk/internal/classfile/impl/CatchBuilderImpl.java#L55)
`[4]:` [CodeBuilder#loadConstant](https://github.com/openjdk/jdk/blob/7ad61605f1669f51a97f4f263a7afaa9ab7706be/src/java.base/share/classes/java/lang/classfile/CodeBuilder.java#L615)
`test/jdk/jdk/classfile/TestNullHostile.java#EXCLUDE_LIST` has the list of methods that are still not null hostile, they are ignored by the test, as making them null hostile either breaks some tests (listed below) or breaks the build with a `NullPointerException` or `BootstapMethodError`. I will paste the relevant methods from that list here.
Tests broken by these methods are :
jdk/classfile/VerifierSelfTest.java
jdk/classfile/TestRecordComponent.java
jdk/classfile/TestNullHostile.java
jdk/classfile/OpcodesValidationTest.java
jdk/classfile/ClassPrinterTest.java
java/lang/invoke/MethodHandlesGeneralTest.java
java/lang/invoke/MethodHandleProxies/WrapperHiddenClassTest.java
java/lang/invoke/MethodHandleProxies/WithSecurityManagerTest.java
java/lang/invoke/MethodHandleProxies/BasicTest.java
Full list of methods
//the implementation of this method in CatchBuilderImpl handles nulls, is this fine?
"java.lang.classfile.CodeBuilder$CatchBuilder/catching(java.lang.constant.ClassDesc,java.util.function.Consumer)/0/0",
// making this method null-hostile causes a BootstapMethodError during the the build
// java.lang.BootstrapMethodError: bootstrap method initialization exception
// Caused by: java.lang.invoke.LambdaConversionException: Exception instantiating lambda object
"java.lang.classfile.CodeBuilder/loadConstant(java.lang.constant.ConstantDesc)/0/0",
"java.lang.classfile.CodeBuilder$BlockCodeBuilder/loadConstant(java.lang.constant.ConstantDesc)/0/0",
//removing these from exclude list breaks CorpusTest and AdvancedTransformationsTest
"java.lang.classfile.ClassHierarchyResolver$ClassHierarchyInfo/ofClass(java.lang.constant.ClassDesc)/0/0",
"java.lang.classfile.attribute.ModuleAttribute$ModuleAttributeBuilder/requires(java.lang.constant.ModuleDesc,int,java.lang.String)/2/0",
"java.lang.classfile.attribute.ModuleAttribute/of(java.lang.classfile.constantpool.ModuleEntry,int,java.lang.classfile.constantpool.Utf8Entry,java.util.Collection,java.util.Collection,java.util.Collection,java.util.Collection,java.util.Collection)/2/0",
"java.lang.classfile.attribute.ModuleRequireInfo/of(java.lang.classfile.constantpool.ModuleEntry,int,java.lang.classfile.constantpool.Utf8Entry)/2/0",
"java.lang.classfile.AttributeMapper/readAttribute(java.lang.classfile.AttributedElement,java.lang.classfile.ClassReader,int)/0/0",
// this one from exclude list breaksa few more tests
"java.lang.classfile.AttributeMapper/readAttribute(java.lang.classfile.AttributedElement,java.lang.classfile.ClassReader,int)/1/0",
// this one breaks jdk/java/lang/invoke/MethodHandleProxies/WithSecurityManagerTest.java
"java.lang.classfile.ClassHierarchyResolver/ofClassLoading(java.lang.ClassLoader)/0/0",
//I need to revisit these
"java.lang.classfile.ClassFileTransform/accept(java.lang.classfile.ClassFileBuilder,java.lang.classfile.ClassFileElement)/0/0",
"java.lang.classfile.ClassFileTransform/accept(java.lang.classfile.ClassFileBuilder,java.lang.classfile.ClassFileElement)/1/0",
"java.lang.classfile.ClassTransform/accept(java.lang.classfile.ClassFileBuilder,java.lang.classfile.ClassFileElement)/0/0",
//can we add stronger checks here?
"java.lang.classfile.components.CodeStackTracker/accept(java.lang.classfile.ClassFileBuilder,java.lang.classfile.ClassFileElement)/0/0",
"java.lang.classfile.components.CodeStackTracker/accept(java.lang.classfile.ClassFileBuilder,java.lang.classfile.ClassFileElement)/1/0",
"java.lang.classfile.CodeTransform/accept(java.lang.classfile.ClassFileBuilder,java.lang.classfile.ClassFileElement)/0/0",
"java.lang.classfile.CodeTransform/accept(java.lang.classfile.ClassFileBuilder,java.lang.classfile.ClassFileElement)/1/0",
"java.lang.classfile.FieldTransform/accept(java.lang.classfile.ClassFileBuilder,java.lang.classfile.ClassFileElement)/0/0",
"java.lang.classfile.FieldTransform/accept(java.lang.classfile.ClassFileBuilder,java.lang.classfile.ClassFileElement)/1/0",
"java.lang.classfile.components.CodeLocalsShifter/accept(java.lang.classfile.ClassFileBuilder,java.lang.classfile.ClassFileElement)/0/0",
"java.lang.classfile.components.CodeLocalsShifter/accept(java.lang.classfile.ClassFileBuilder,java.lang.classfile.ClassFileElement)/1/0",
"java.lang.classfile.components.CodeRelabeler/accept(java.lang.classfile.ClassFileBuilder,java.lang.classfile.ClassFileElement)/1/0",
"java.lang.classfile.components.CodeRelabeler/accept(java.lang.classfile.ClassFileBuilder,java.lang.classfile.ClassFileElement)/0/0",
"java.lang.classfile.MethodTransform/accept(java.lang.classfile.ClassFileBuilder,java.lang.classfile.ClassFileElement)/0/0",
"java.lang.classfile.MethodTransform/accept(java.lang.classfile.ClassFileBuilder,java.lang.classfile.ClassFileElement)/1/0",
// implementation calls these method with null
"java.lang.classfile.Signature$ClassTypeSig/of(java.lang.classfile.Signature$ClassTypeSig,java.lang.constant.ClassDesc,java.lang.classfile.Signature$TypeArg[])/0/0",
"java.lang.classfile.Signature$ClassTypeSig/of(java.lang.classfile.Signature$ClassTypeSig,java.lang.String,java.lang.classfile.Signature$TypeArg[])/0/0",
"java.lang.classfile.Signature$TypeParam/of(java.lang.String,java.lang.classfile.Signature$RefTypeSig,java.lang.classfile.Signature$RefTypeSig[])/1/0",
"java.lang.classfile.BufWriter/writeIndexOrZero(java.lang.classfile.constantpool.PoolEntry)/0/0"
-------------
Commit messages:
- Tiny changes, some variable renames
- Changes after JDK-8339214
- Merge remote-tracking branch 'upstream/master' into classfile-nulls
- - Passes all tests, a lot of comments for future work and questions
- Merge remote-tracking branch 'upstream/master' into classfile-nulls
- drop unused test file
- Add more null checks, added some todos for methods that need to be revisited.
- Merge remote-tracking branch 'upstream/master' into classfile-nulls
- add requiresnonnull where necessary
- Merge remote-tracking branch 'origin/master' into classfile-nulls
- ... and 1 more: https://git.openjdk.org/jdk/compare/ad40a122...2902e21d
Changes: https://git.openjdk.org/jdk/pull/20556/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=20556&range=00
Issue: https://bugs.openjdk.org/browse/JDK-8317356
Stats: 1458 lines in 85 files changed: 1388 ins; 20 del; 50 mod
Patch: https://git.openjdk.org/jdk/pull/20556.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/20556/head:pull/20556
PR: https://git.openjdk.org/jdk/pull/20556
More information about the core-libs-dev
mailing list