RFR: 8317356: Test ClassFile API if it deals with nulls correctly across the whole API [v2]

Nizar Benalla nbenalla at openjdk.org
Wed Sep 4 15:32:41 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 initiali...

Nizar Benalla has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 12 commits:

 - Merge remote-tracking branch 'upstream/master' into classfile-nulls
   
   # Conflicts:
   #	src/java.base/share/classes/java/lang/classfile/CodeBuilder.java
 - Tiny changes, some variable renames
 - Changes after JDK-8339214
 - Merge remote-tracking branch 'upstream/master' into classfile-nulls
   
   # Conflicts:
   #	src/java.base/share/classes/java/lang/classfile/CodeBuilder.java
   #	src/java.base/share/classes/jdk/internal/classfile/impl/DirectCodeBuilder.java
 - - Passes all tests, a lot of comments for future work and questions
   
   - Added (C)
 - 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.
   
   Some tests need to be updated if we want to integrate this change
   
   use regex to switch Object.require non null with the static import.
   Just for nicer syntax
 - Merge remote-tracking branch 'upstream/master' into classfile-nulls
   
   # Conflicts:
   #	src/java.base/share/classes/jdk/internal/classfile/impl/DirectCodeBuilder.java
 - add requiresnonnull where necessary
 - ... and 2 more: https://git.openjdk.org/jdk/compare/6f8714ee...cf75c679

-------------

Changes: https://git.openjdk.org/jdk/pull/20556/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=20556&range=01
  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