Re: RFR: 8320396: Class-File API ClassModel::verify should include checks from hotspot/share/classfile/classFileParser.cpp [v9]

2024-05-30 Thread Maurizio Cimadamore
On Fri, 24 May 2024 16:17:41 GMT, Adam Sotona  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 with a new target base due to a 
> merge or a rebase. The pull request now contains 37 commits:
> 
>  - fixed ParserVerifier and VerifierSelfTest
>  - Merge branch 'master' into JDK-8320396-verifier-extension
>  - added verification of TypeAnnotation attributes
>  - added verification of SMT attribute
>  - added verification of module-related attributes
>  - applied the suggested changes
>  - applied the suggested changes
>  - fixed error thrown by VerifierImpl
>  - applied suggested changes
>  - Merge branch 'master' into JDK-8320396-verifier-extension
>  - ... and 27 more: https://git.openjdk.org/jdk/compare/cfdc64fc...b352b794

Great work!

-

Marked as reviewed by mcimadamore (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16809#pullrequestreview-2089511401


Re: RFR: 8320396: Class-File API ClassModel::verify should include checks from hotspot/share/classfile/classFileParser.cpp [v9]

2024-05-28 Thread Chen Liang
On Fri, 24 May 2024 16:17:41 GMT, Adam Sotona  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 with a new target base due to a 
> merge or a rebase. The pull request now contains 37 commits:
> 
>  - fixed ParserVerifier and VerifierSelfTest
>  - Merge branch 'master' into JDK-8320396-verifier-extension
>  - added verification of TypeAnnotation attributes
>  - added verification of SMT attribute
>  - added verification of module-related attributes
>  - applied the suggested changes
>  - applied the suggested changes
>  - fixed error thrown by VerifierImpl
>  - applied suggested changes
>  - Merge branch 'master' into JDK-8320396-verifier-extension
>  - ... and 27 more: https://git.openjdk.org/jdk/compare/cfdc64fc...b352b794

Marked as reviewed by liach (Author).

-

PR Review: https://git.openjdk.org/jdk/pull/16809#pullrequestreview-2082798275


Re: RFR: 8320396: Class-File API ClassModel::verify should include checks from hotspot/share/classfile/classFileParser.cpp [v9]

2024-05-28 Thread Adam Sotona
On Fri, 24 May 2024 16:17:41 GMT, Adam Sotona  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 with a new target base due to a 
> merge or a rebase. The pull request now contains 37 commits:
> 
>  - fixed ParserVerifier and VerifierSelfTest
>  - Merge branch 'master' into JDK-8320396-verifier-extension
>  - added verification of TypeAnnotation attributes
>  - added verification of SMT attribute
>  - added verification of module-related attributes
>  - applied the suggested changes
>  - applied the suggested changes
>  - fixed error thrown by VerifierImpl
>  - applied suggested changes
>  - Merge branch 'master' into JDK-8320396-verifier-extension
>  - ... and 27 more: https://git.openjdk.org/jdk/compare/cfdc64fc...b352b794

Please review!

This verifier extension is actually blocking new `javap` feature for 23, see:  
https://github.com/openjdk/jdk/pull/18629 
Missing all of these class file verifications in `javap -verify` would be sad.

Thank you!

Adam

-

PR Comment: https://git.openjdk.org/jdk/pull/16809#issuecomment-2135209023


Re: RFR: 8320396: Class-File API ClassModel::verify should include checks from hotspot/share/classfile/classFileParser.cpp [v9]

2024-05-24 Thread Adam Sotona
> 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 with a new target base due to a merge 
or a rebase. The pull request now contains 37 commits:

 - fixed ParserVerifier and VerifierSelfTest
 - Merge branch 'master' into JDK-8320396-verifier-extension
 - added verification of TypeAnnotation attributes
 - added verification of SMT attribute
 - added verification of module-related attributes
 - applied the suggested changes
 - applied the suggested changes
 - fixed error thrown by VerifierImpl
 - applied suggested changes
 - Merge branch 'master' into JDK-8320396-verifier-extension
 - ... and 27 more: https://git.openjdk.org/jdk/compare/cfdc64fc...b352b794

-

Changes: https://git.openjdk.org/jdk/pull/16809/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk=16809=08
  Stats: 869 lines in 6 files changed: 835 ins; 7 del; 27 mod
  Patch: https://git.openjdk.org/jdk/pull/16809.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16809/head:pull/16809

PR: https://git.openjdk.org/jdk/pull/16809