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

Reply via email to