RFR: 8341094: Clean up relax_verify in ClassFileParser [v2]

[Ioi Lam]

On Thu, 7 Nov 2024 23:03:26 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> Removed ClassFileParser::_relax_verify and cleaned up setting need_verify in the ClassFileStream code and ClassFileParser.  Wrote up how I found control to flow to this setting with -Xverify:all/none/remote settings, which become BytecodeVerificationRemote/Local in the CR comments.
>> 
>> Tested with tier1-4.  Valid class, field and method name testing is done through the JCK and there are CDS tests that test verification in runtime/cds/appcds/VerifierTest.java.
>
> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Rename ClassFileStream::_need_verify to _check_truncation and set based on one call in ClassFileParser.

I am honestly very confused by this code:

- We always create ClassFileStream with _need_verify = true
- Sometimes we set it back to false in ClassFileParser::ClassFileParser()


  _need_verify = Verifier::should_verify_for(_loader_data->class_loader());
  stream->set_check_truncation(_need_verify);


Can we instead set `ClassFileStream::_need_verify`once and for all inside the `ClassFileStream` constructor, and use it as a constant afterwards (and propagate to ClassFilePaser, etc)? This can be done by passing a `ClassLoader` into the `ClassFileStream` constructor and call `Verifier::should_verify_for` in there.

Then, you can change `_check_truncation` back to `_need_verify`.

`ClassFileStream::verify` can be removed as it doesn't seem to serve any purpose anymore.

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

PR Comment: https://git.openjdk.org/jdk/pull/21954#issuecomment-2463683553