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:54 UTC 2024
On Tue, 3 Sep 2024 01:58:59 GMT, Chen Liang <liach at openjdk.org> wrote:
>> 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...
>
> test/jdk/jdk/classfile/testdata/TestClass.java line 1:
>
>> 1: package testdata;
>
> File seems redundant? Or did you forget to upload the test driver?
I thought I removed `testClass.java` as I no longer use it.
I will include a regression test tomorrow with the next patch.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20556#discussion_r1741345363
More information about the core-libs-dev
mailing list