RFR: 8341094: Clean up relax_verify in ClassFileParser [v2]
Coleen Phillimore
coleenp at openjdk.org
Fri Nov 8 11:44:29 UTC 2024
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.
Ioi, I like your suggestion. That would make it even clearer. We could set the value of _need_verify once in ClassFileStream constructor, and propagate the value of _need_verify to ClassFileParser. ClassFileParser has a lot of checks for _need_verify so it's still better for ClassFileParser to have one.
David, there are three different concepts of "trusted" in this code which caused the duplication that I'm trying to resolve. One is for package access which includes boot/platform/system class loader.
Two is for this relax_verify to check the names of class/fields/methods which includes boot/platform class loader.
Three is for checking stream truncation, doing classfile format checks and running the Verifier itself which only includes the boot class loader.
My change removes #2 since checking names is a fast enough operation (doesn't call java unless < JDK 1.5), and most , and maybe all platform class loading is called with ClassFileStream::_need_verify == true anyway, so this code checked these names anyway.
Further, I'm trying to only check for when we want verification #3 in only one place as it was set and reset between ClassFileStream and ClassFileParser, and the same code is called again effectively ignoring the value of should_verify_class value passed into the Verifier but getting the same result as the last call.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/21954#issuecomment-2464509323
More information about the hotspot-runtime-dev
mailing list