JDK-8341094 : Clean up relax_verify in ClassFileParser
  • Type: Enhancement
  • Component: hotspot
  • Sub-Component: runtime
  • Priority: P4
  • Status: New
  • Resolution: Unresolved
  • Submitted: 2024-09-27
  • Updated: 2024-09-30
Related Reports
Relates :  
Description
I'm revisiting fix for JDK-8148854 because I want to clean up some duplicated code.  There's no bug in this fix so nothing to change.  This fix added the function:

static bool relax_format_check_for(ClassLoaderData* loader_data) {
  bool trusted = loader_data->is_boot_class_loader_data() ||
                 loader_data->is_platform_class_loader_data();
  bool need_verify =
    // verifyAll
    (BytecodeVerificationLocal && BytecodeVerificationRemote) ||
    // verifyRemote
    (!BytecodeVerificationLocal && BytecodeVerificationRemote && !trusted);
  return !need_verify;
}

To call instead of

bool Verifier::relax_access_for(oop loader) {
  bool trusted = java_lang_ClassLoader::is_trusted_loader(loader);
  bool need_verify =
    // verifyAll
    (BytecodeVerificationLocal && BytecodeVerificationRemote) ||
    // verifyRemote      
    (!BytecodeVerificationLocal && BytecodeVerificationRemote && !trusted);
  return !need_verify;   
} 

The essential difference in these two functions is java_lang_ClassLoader::is_trusted_loader() returns true for the "system" class loader, also known as the app class loader.  The bug in this report should verify the name for the app class loader and only elide verification for the boot and platform class loader.   Eliding the name verification for the platform class loader seems like an unimportant optimization and not necessary for correctness.  Therefore, we could use the value of _need_verify in classFileParser to decide name verification, which is already tested, eg:

// Checks if name is a legal method name.
void ClassFileParser::verify_legal_method_name(const Symbol* name, TRAPS) const {
  if (!_need_verify || _relax_verify) { return; }


Comments
> bool trusted = java_lang_ClassLoader::is_trusted_loader(loader); is_trusted_loader has always considered the system/app loader as "trusted", yet it was added to control the name checks and the refactoring placed it in Verifier::relax_access_for instead of relax_format_check_for. So it does look like a bug was introduced with that refactoring - not that anyone apparently ever noticed given this was addressing a change in naming rules between JDK 1.1 and 1.2.
30-09-2024