8341094: Clean up relax_verify in ClassFileParser

Reviewed-by: dholmes, iklam

src/hotspot/share/cds/lambdaFormInvokers.cpp

@@ -166,7 +166,7 @@ void LambdaFormInvokers::regenerate_holder_classes(TRAPS) {
       // make a copy of class bytes so GC will not affect us.
       char *buf = NEW_RESOURCE_ARRAY(char, len);
       memcpy(buf, (char*)h_bytes->byte_at_addr(0), len);
-      ClassFileStream st((u1*)buf, len, nullptr, ClassFileStream::verify);
+      ClassFileStream st((u1*)buf, len, nullptr);
       regenerate_class(class_name, st, CHECK);
     }
   }

src/hotspot/share/classfile/classFileParser.cpp

@@ -4659,7 +4659,7 @@ const char* ClassFileParser::skip_over_field_signature(const char* signature,
 
 // Checks if name is a legal class name.
 void ClassFileParser::verify_legal_class_name(const Symbol* name, TRAPS) const {
-  if (!_need_verify || _relax_verify) { return; }
+  if (!_need_verify) { return; }
 
   assert(name->refcount() > 0, "symbol must be kept alive");
   char* bytes = (char*)name->bytes();
@@ -4699,7 +4699,7 @@ void ClassFileParser::verify_legal_class_name(const Symbol* name, TRAPS) const {
 
 // Checks if name is a legal field name.
 void ClassFileParser::verify_legal_field_name(const Symbol* name, TRAPS) const {
-  if (!_need_verify || _relax_verify) { return; }
+  if (!_need_verify) { return; }
 
   char* bytes = (char*)name->bytes();
   unsigned int length = name->utf8_length();
@@ -4732,7 +4732,7 @@ void ClassFileParser::verify_legal_field_name(const Symbol* name, TRAPS) const {
 
 // 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; }
+  if (!_need_verify) { return; }
 
   assert(name != nullptr, "method name is null");
   char* bytes = (char*)name->bytes();
@@ -5236,17 +5236,6 @@ void ClassFileParser::update_class_name(Symbol* new_class_name) {
   _class_name->increment_refcount();
 }
 
-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;
-}
-
 ClassFileParser::ClassFileParser(ClassFileStream* stream,
                                  Symbol* name,
                                  ClassLoaderData* loader_data,
@@ -5303,7 +5292,6 @@ ClassFileParser::ClassFileParser(ClassFileStream* stream,
   _itfs_len(0),
   _java_fields_count(0),
   _need_verify(false),
-  _relax_verify(false),
   _has_nonstatic_concrete_methods(false),
   _declares_nonstatic_concrete_methods(false),
   _has_localvariable_table(false),
@@ -5324,24 +5312,10 @@ ClassFileParser::ClassFileParser(ClassFileStream* stream,
   assert(0 == _access_flags.as_int(), "invariant");
 
   // Figure out whether we can skip format checking (matching classic VM behavior)
-  if (CDSConfig::is_dumping_static_archive()) {
+  _need_verify = Verifier::should_verify_for(_loader_data->class_loader());
-    // verify == true means it's a 'remote' class (i.e., non-boot class)
-    // Verification decision is based on BytecodeVerificationRemote flag
-    // for those classes.
-    _need_verify = (stream->need_verify()) ? BytecodeVerificationRemote :
-                                              BytecodeVerificationLocal;
-  }
-  else {
-    _need_verify = Verifier::should_verify_for(_loader_data->class_loader(),
-                                               stream->need_verify());
-  }
 
-  // synch back verification state to stream
+  // synch back verification state to stream to check for truncation.
-  stream->set_verify(_need_verify);
+  stream->set_need_verify(_need_verify);
-
-  // Check if verification needs to be relaxed for this class file
-  // Do not restrict it to jdk1.0 or jdk1.1 to maintain backward compatibility (4982376)
-  _relax_verify = relax_format_check_for(_loader_data);
 
   parse_stream(stream, CHECK);
 

src/hotspot/share/classfile/classFileParser.hpp

@@ -185,7 +185,6 @@ class ClassFileParser {
   u2 _java_fields_count;
 
   bool _need_verify;
-  bool _relax_verify;
 
   bool _has_nonstatic_concrete_methods;
   bool _declares_nonstatic_concrete_methods;

src/hotspot/share/classfile/classFileStream.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -28,8 +28,6 @@
 #include "classfile/vmSymbols.hpp"
 #include "memory/resourceArea.hpp"
 
-const bool ClassFileStream::verify = true;
-
 void ClassFileStream::truncated_file_error(TRAPS) const {
   THROW_MSG(vmSymbols::java_lang_ClassFormatError(), "Truncated class file");
 }
@@ -37,13 +35,12 @@ void ClassFileStream::truncated_file_error(TRAPS) const {
 ClassFileStream::ClassFileStream(const u1* buffer,
                                  int length,
                                  const char* source,
-                                 bool verify_stream,
                                  bool from_boot_loader_modules_image) :
   _buffer_start(buffer),
   _buffer_end(buffer + length),
   _current(buffer),
   _source(source),
-  _need_verify(verify_stream),
+  _need_verify(true),  // may be reset by ClassFileParser when this stream is parsed.
   _from_boot_loader_modules_image(from_boot_loader_modules_image) {
     assert(buffer != nullptr, "caller should throw NPE");
 }
@@ -72,6 +69,5 @@ const ClassFileStream* ClassFileStream::clone() const {
   return new ClassFileStream(new_buffer_start,
                              length(),
                              clone_source(),
-                             need_verify(),
                              from_boot_loader_modules_image());
 }

src/hotspot/share/classfile/classFileStream.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -43,7 +43,7 @@ class ClassFileStream: public ResourceObj {
   const u1* const _buffer_end;   // Buffer top (one past last element)
   mutable const u1* _current;    // Current buffer position
   const char* const _source;     // Source of stream (directory name, ZIP/JAR archive name)
-  bool _need_verify;             // True if verification is on for the class file
+  bool _need_verify;             // True if we need to verify and check truncation of stream bytes.
   bool _from_boot_loader_modules_image;  // True if this was created by ClassPathImageEntry.
   void truncated_file_error(TRAPS) const ;
 
@@ -52,12 +52,9 @@ class ClassFileStream: public ResourceObj {
   const char* clone_source() const;
 
  public:
-  static const bool verify;
-
   ClassFileStream(const u1* buffer,
                   int length,
                   const char* source,
-                  bool verify_stream = verify,  // to be verified by default
                   bool from_boot_loader_modules_image = false);
 
   virtual const ClassFileStream* clone() const;
@@ -77,7 +74,7 @@ class ClassFileStream: public ResourceObj {
   }
   const char* source() const { return _source; }
   bool need_verify() const { return _need_verify; }
-  void set_verify(bool flag) { _need_verify = flag; }
+  void set_need_verify(bool flag) { _need_verify = flag; }
   bool from_boot_loader_modules_image() const { return _from_boot_loader_modules_image; }
 
   void check_truncated_file(bool b, TRAPS) const {

src/hotspot/share/classfile/classLoader.cpp

@@ -297,8 +297,7 @@ ClassFileStream* ClassPathDirEntry::open_stream(JavaThread* current, const char*
         // Resource allocated
         return new ClassFileStream(buffer,
                                    checked_cast<int>(st.st_size),
-                                   _dir,
+                                   _dir);
-                                   ClassFileStream::verify);
       }
     }
   }
@@ -366,8 +365,7 @@ ClassFileStream* ClassPathZipEntry::open_stream(JavaThread* current, const char*
   // Resource allocated
   return new ClassFileStream(buffer,
                              filesize,
-                             _zip_name,
+                             _zip_name);
-                             ClassFileStream::verify);
 }
 
 DEBUG_ONLY(ClassPathImageEntry* ClassPathImageEntry::_singleton = nullptr;)
@@ -449,7 +447,6 @@ ClassFileStream* ClassPathImageEntry::open_stream_for_loader(JavaThread* current
     return new ClassFileStream((u1*)data,
                                checked_cast<int>(size),
                                _name,
-                               ClassFileStream::verify,
                                true); // from_boot_loader_modules_image
   }
 
@@ -1198,8 +1195,6 @@ InstanceKlass* ClassLoader::load_class(Symbol* name, PackageEntry* pkg_entry, bo
     return nullptr;
   }
 
-  stream->set_verify(ClassLoaderExt::should_verify(classpath_index));
-
   ClassLoaderData* loader_data = ClassLoaderData::the_null_class_loader_data();
   Handle protection_domain;
   ClassLoadInfo cl_info(protection_domain);

src/hotspot/share/classfile/classLoaderExt.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -33,11 +33,6 @@ class ClassListParser;
 
 class ClassLoaderExt: public ClassLoader { // AllStatic
 public:
-  static bool should_verify(int classpath_index) {
-    CDS_ONLY(return (classpath_index >= _app_class_paths_start_index);)
-    NOT_CDS(return false;)
-  }
-
 #if INCLUDE_CDS
 private:
   enum SomeConstants {

src/hotspot/share/classfile/klassFactory.cpp

@@ -1,5 +1,5 @@
 /*
-* Copyright (c) 2015, 2023, Oracle and/or its affiliates. All rights reserved.
+* Copyright (c) 2015, 2024, Oracle and/or its affiliates. All rights reserved.
 * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
 *
 * This code is free software; you can redistribute it and/or modify it
@@ -77,8 +77,7 @@ InstanceKlass* KlassFactory::check_shared_class_file_load_hook(
       s2 path_index = ik->shared_classpath_index();
       ClassFileStream* stream = new ClassFileStream(ptr,
                                                     pointer_delta_as_int(end_ptr, ptr),
-                                                    cfs->source(),
+                                                    cfs->source());
-                                                    ClassFileStream::verify);
       ClassLoadInfo cl_info(protection_domain);
       ClassFileParser parser(stream,
                              class_name,
@@ -157,8 +156,7 @@ static ClassFileStream* check_class_file_load_hook(ClassFileStream* stream,
       // Set new class file stream using JVMTI agent modified class file data.
       stream = new ClassFileStream(ptr,
                                    pointer_delta_as_int(end_ptr, ptr),
-                                   stream->source(),
+                                   stream->source());
-                                   stream->need_verify());
     }
   }
 

src/hotspot/share/classfile/systemDictionary.cpp

@@ -825,7 +825,6 @@ InstanceKlass* SystemDictionary::resolve_hidden_class_from_stream(
   loader_data = register_loader(class_loader, create_mirror_cld);
 
   assert(st != nullptr, "invariant");
-  assert(st->need_verify(), "invariant");
 
   // Parse stream and create a klass.
   InstanceKlass* k = KlassFactory::create_from_stream(st,

src/hotspot/share/classfile/verifier.cpp

@@ -105,8 +105,8 @@ static verify_byte_codes_fn_t verify_byte_codes_fn() {
 
 // Methods in Verifier
 
-bool Verifier::should_verify_for(oop class_loader, bool should_verify_class) {
+bool Verifier::should_verify_for(oop class_loader) {
-  return (class_loader == nullptr || !should_verify_class) ?
+  return class_loader == nullptr ?
     BytecodeVerificationLocal : BytecodeVerificationRemote;
 }
 
@@ -276,7 +276,7 @@ bool Verifier::verify(InstanceKlass* klass, bool should_verify_class, TRAPS) {
 bool Verifier::is_eligible_for_verification(InstanceKlass* klass, bool should_verify_class) {
   Symbol* name = klass->name();
 
-  return (should_verify_for(klass->class_loader(), should_verify_class) &&
+  return (should_verify_class &&
     // return if the class is a bootstrapping class
     // or defineClass specified not to verify by default (flags override passed arg)
     // We need to skip the following four for bootstraping

src/hotspot/share/classfile/verifier.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1998, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1998, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -52,7 +52,7 @@ class Verifier : AllStatic {
   // Return false if the class is loaded by the bootstrap loader,
   // or if defineClass was called requesting skipping verification
   // -Xverify:all overrides this value
-  static bool should_verify_for(oop class_loader, bool should_verify_class);
+  static bool should_verify_for(oop class_loader);
 
   // Relax certain access checks to enable some broken 1.1 apps to run on 1.2.
   static bool relax_access_for(oop class_loader);

src/hotspot/share/jfr/instrumentation/jfrEventClassTransformer.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -1202,7 +1202,7 @@ static ClassFileStream* schema_extend_event_klass_bytes(const InstanceKlass* ik,
       orig_stream->skip_u1_fast(attrib_len);
     }
   }
-  return new ClassFileStream(new_buffer, orig_stream_length, nullptr, ClassFileStream::verify);
+  return new ClassFileStream(new_buffer, orig_stream_length, nullptr);
 }
 
 // Attempt to locate an existing UTF8_INFO mapping the utf8_constant.
@@ -1510,7 +1510,7 @@ static ClassFileStream* schema_extend_event_subklass_bytes(const InstanceKlass*
     size_of_new_bytes = size_of_instrumented_bytes;
     is_instrumented = true;
   }
-  return new ClassFileStream(new_bytes, size_of_new_bytes, nullptr, ClassFileStream::verify);
+  return new ClassFileStream(new_bytes, size_of_new_bytes, nullptr);
 }
 
 static bool _force_instrumentation = false;
@@ -1547,7 +1547,7 @@ static ClassFileStream* retransform_bytes(const Klass* existing_klass, const Cla
   assert(new_bytes != nullptr, "invariant");
   assert(size_of_new_bytes > 0, "invariant");
   is_instrumented = true;
-  return new ClassFileStream(new_bytes, size_of_new_bytes, nullptr, ClassFileStream::verify);
+  return new ClassFileStream(new_bytes, size_of_new_bytes, nullptr);
 }
 
 // On initial class load.

src/hotspot/share/prims/jni.cpp

@@ -288,7 +288,7 @@ JNI_ENTRY(jclass, jni_DefineClass(JNIEnv *env, const char *name, jobject loaderR
                                         CHECK_NULL);
 
   ResourceMark rm(THREAD);
-  ClassFileStream st((u1*)buf, bufLen, nullptr, ClassFileStream::verify);
+  ClassFileStream st((u1*)buf, bufLen, nullptr);
   Handle class_loader (THREAD, JNIHandles::resolve(loaderRef));
   Handle protection_domain;
   ClassLoadInfo cl_info(protection_domain);

src/hotspot/share/prims/jvm.cpp

@@ -917,7 +917,7 @@ static jclass jvm_define_class_common(const char *name,
                                            CHECK_NULL);
 
   ResourceMark rm(THREAD);
-  ClassFileStream st((u1*)buf, len, source, ClassFileStream::verify);
+  ClassFileStream st((u1*)buf, len, source);
   Handle class_loader (THREAD, JNIHandles::resolve(loader));
   Handle protection_domain (THREAD, JNIHandles::resolve(pd));
   ClassLoadInfo cl_info(protection_domain);
@@ -1003,7 +1003,7 @@ static jclass jvm_lookup_define_class(jclass lookup, const char *name,
 
   Handle protection_domain (THREAD, JNIHandles::resolve(pd));
   const char* source = is_nestmate ? host_class->external_name() : "__JVM_LookupDefineClass__";
-  ClassFileStream st((u1*)buf, len, source, ClassFileStream::verify);
+  ClassFileStream st((u1*)buf, len, source);
 
   InstanceKlass* ik = nullptr;
   if (!is_hidden) {

src/hotspot/share/prims/jvmtiRedefineClasses.cpp

@@ -1362,8 +1362,7 @@ jvmtiError VM_RedefineClasses::load_new_class_versions() {
 
     ClassFileStream st((u1*)_class_defs[i].class_bytes,
                        _class_defs[i].class_byte_count,
-                       "__VM_RedefineClasses__",
+                       "__VM_RedefineClasses__");
-                       ClassFileStream::verify);
 
     // Set redefined class handle in JvmtiThreadState class.
     // This redefined class is sent to agent event handler for class file