On Thu, 9 May 2024 10:11:22 GMT, Adam Sotona <asot...@openjdk.org> wrote:
>> ClassFile API `jdk.internal.classfile.verifier.VerifierImpl` performed only
>> bytecode-level class verification.
>> This patch adds `jdk.internal.classfile.verifier.ParserVerifier` with
>> additional class checks inspired by
>> `hotspot/share/classfile/classFileParser.cpp`.
>>
>> Also new `VerifierSelfTest::testParserVerifier` has been added.
>>
>> Please review.
>>
>> Thanks,
>> Adam
>
> Adam Sotona has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - fixed error thrown by VerifierImpl
> - applied suggested changes
src/java.base/share/classes/jdk/internal/classfile/impl/verifier/ParserVerifier.java
line 205:
> 203: private void verifyAttribute(AttributedElement ae, Attribute<?> a,
> List<VerifyError> errors) {
> 204: int payLoad = ((BoundAttribute)a).payloadLen();
> 205: if (payLoad != switch (a) {
Please break this up - e.g. with an intermediate `foundPayload` variable -
having a multi-line switch nested as an if condition looks very jarring!
src/java.base/share/classes/jdk/internal/classfile/impl/verifier/ParserVerifier.java
line 246:
> 244: }
> 245: case DeprecatedAttribute da -> 0;
> 246: case EnclosingMethodAttribute ema -> 4;
See 4.7.7:
> Otherwise, the value of the method_index item must be a valid index into the
> constant_pool table. The constant_pool entry at that index must be a
> CONSTANT_NameAndType_info structure
> ([§4.4.6](https://docs.oracle.com/javase/specs/jvms/se22/html/jvms-4.html#jvms-4.4.6))
> representing the name and type of a method in the class referenced by the
> class_index attribute above.
src/java.base/share/classes/jdk/internal/classfile/impl/verifier/ParserVerifier.java
line 247:
> 245: case DeprecatedAttribute da -> 0;
> 246: case EnclosingMethodAttribute ema -> 4;
> 247: case ExceptionsAttribute ea -> 2 + 2 *
> ea.exceptions().size();
See 4.7.5:
> Each value in the exception_index_table array must be a valid index into the
> constant_pool table. The constant_pool entry at that index must be a
> CONSTANT_Class_info structure
> ([§4.4.1](https://docs.oracle.com/javase/specs/jvms/se22/html/jvms-4.html#jvms-4.4.1))
> representing a class type that this method is declared to throw.
src/java.base/share/classes/jdk/internal/classfile/impl/verifier/ParserVerifier.java
line 283:
> 281: yield l;
> 282: }
> 283: case RuntimeVisibleAnnotationsAttribute aa -> {
I wonder if these 4 cases can be consolidated a bit. They all require an
annotation accessor, and then something that keeps computes the size of all the
annotations. E.g.
int annotationSize(List<Annotation> annos) {
long size = 0;
for (var an : annos) {
size += annotationSize(an);
}
return l;
}
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16809#discussion_r1598800394
PR Review Comment: https://git.openjdk.org/jdk/pull/16809#discussion_r1598806362
PR Review Comment: https://git.openjdk.org/jdk/pull/16809#discussion_r1598807397
PR Review Comment: https://git.openjdk.org/jdk/pull/16809#discussion_r1598803054
