8268522: InstanceKlass::can_be_verified_at_dumptime() returns opposite value

calvinccheung · calvinccheung · commit c088d093e2e7 · 2021-06-14T15:02:55.000Z
Reviewed-by: dholmes, minqi, iklam
diff --git a/src/hotspot/share/cds/metaspaceShared.cpp b/src/hotspot/share/cds/metaspaceShared.cpp
@@ -390,7 +390,7 @@ static void rewrite_nofast_bytecode(const methodHandle& method) {
 void MetaspaceShared::rewrite_nofast_bytecodes_and_calculate_fingerprints(Thread* thread, InstanceKlass* ik) {
   for (int i = 0; i < ik->methods()->length(); i++) {
     methodHandle m(thread, ik->methods()->at(i));
-    if (!ik->can_be_verified_at_dumptime()) {
+    if (ik->can_be_verified_at_dumptime()) {
       rewrite_nofast_bytecode(m);
     }
     Fingerprinter fp(m);
@@ -574,7 +574,7 @@ class CollectCLDClosure : public CLDClosure {
 bool MetaspaceShared::linking_required(InstanceKlass* ik) {
   // For static CDS dump, do not link old classes.
   // For dynamic CDS dump, only link classes loaded by the builtin class loaders.
-  return DumpSharedSpaces ? !ik->can_be_verified_at_dumptime() : !ik->is_shared_unregistered_class();
+  return DumpSharedSpaces ? ik->can_be_verified_at_dumptime() : !ik->is_shared_unregistered_class();
 }
 
 bool MetaspaceShared::link_class_for_cds(InstanceKlass* ik, TRAPS) {
@@ -750,7 +750,7 @@ bool MetaspaceShared::try_link_class(JavaThread* current, InstanceKlass* ik) {
   ExceptionMark em(current);
   JavaThread* THREAD = current; // For exception macros.
   Arguments::assert_is_dumping_archive();
-  if (ik->is_loaded() && !ik->is_linked() && !ik->can_be_verified_at_dumptime() &&
+  if (ik->is_loaded() && !ik->is_linked() && ik->can_be_verified_at_dumptime() &&
       !SystemDictionaryShared::has_class_failed_verification(ik)) {
     bool saved = BytecodeVerificationLocal;
     if (ik->is_shared_unregistered_class() && ik->class_loader() == NULL) {
diff --git a/src/hotspot/share/classfile/systemDictionaryShared.cpp b/src/hotspot/share/classfile/systemDictionaryShared.cpp
@@ -1411,7 +1411,7 @@ bool SystemDictionaryShared::check_for_exclusion_impl(InstanceKlass* k) {
     if (has_class_failed_verification(k)) {
       return warn_excluded(k, "Failed verification");
     } else {
-      if (!k->can_be_verified_at_dumptime()) {
+      if (k->can_be_verified_at_dumptime()) {
         return warn_excluded(k, "Not linked");
       }
     }
@@ -1425,7 +1425,7 @@ bool SystemDictionaryShared::check_for_exclusion_impl(InstanceKlass* k) {
     return true;
   }
 
-  if (k->can_be_verified_at_dumptime() && k->is_linked()) {
+  if (!k->can_be_verified_at_dumptime() && k->is_linked()) {
     return warn_excluded(k, "Old class has been linked");
   }
 
diff --git a/src/hotspot/share/interpreter/rewriter.cpp b/src/hotspot/share/interpreter/rewriter.cpp
@@ -571,7 +571,7 @@ void Rewriter::rewrite(InstanceKlass* klass, TRAPS) {
 #if INCLUDE_CDS
   if (klass->is_shared()) {
     assert(!klass->is_rewritten(), "rewritten shared classes cannot be rewritten again");
-    assert(klass->can_be_verified_at_dumptime(), "only shared old classes aren't rewritten");
+    assert(!klass->can_be_verified_at_dumptime(), "only shared old classes aren't rewritten");
   }
 #endif // INCLUDE_CDS
   ResourceMark rm(THREAD);
diff --git a/src/hotspot/share/oops/constantPool.cpp b/src/hotspot/share/oops/constantPool.cpp
@@ -370,7 +370,7 @@ void ConstantPool::restore_unshareable_info(TRAPS) {
 }
 
 void ConstantPool::remove_unshareable_info() {
-  if (!_pool_holder->is_linked() && _pool_holder->is_shared_old_klass()) {
+  if (!_pool_holder->is_linked() && !_pool_holder->verified_at_dump_time()) {
     return;
   }
   // Resolved references are not in the shared archive.
diff --git a/src/hotspot/share/oops/instanceKlass.cpp b/src/hotspot/share/oops/instanceKlass.cpp
@@ -2403,8 +2403,8 @@ void InstanceKlass::metaspace_pointers_do(MetaspaceClosure* it) {
 void InstanceKlass::remove_unshareable_info() {
 
   if (can_be_verified_at_dumptime()) {
-    // Set the old class bit.
-    set_is_shared_old_klass();
+    // Remember this so we can avoid walking the hierarchy at runtime.
+    set_verified_at_dump_time();
   }
 
   Klass::remove_unshareable_info();
@@ -2549,19 +2549,19 @@ void InstanceKlass::restore_unshareable_info(ClassLoaderData* loader_data, Handl
 // Verification of archived old classes will be performed during run time.
 bool InstanceKlass::can_be_verified_at_dumptime() const {
   if (major_version() < 50 /*JAVA_6_VERSION*/) {
-    return true;
+    return false;
   }
-  if (java_super() != NULL && java_super()->can_be_verified_at_dumptime()) {
-    return true;
+  if (java_super() != NULL && !java_super()->can_be_verified_at_dumptime()) {
+    return false;
   }
   Array<InstanceKlass*>* interfaces = local_interfaces();
   int len = interfaces->length();
   for (int i = 0; i < len; i++) {
-    if (interfaces->at(i)->can_be_verified_at_dumptime()) {
-      return true;
+    if (!interfaces->at(i)->can_be_verified_at_dumptime()) {
+      return false;
     }
   }
-  return false;
+  return true;
 }
 
 void InstanceKlass::set_shared_class_loader_type(s2 loader_type) {
diff --git a/src/hotspot/share/oops/klass.hpp b/src/hotspot/share/oops/klass.hpp
@@ -178,7 +178,7 @@ class Klass : public Metadata {
   enum {
     _archived_lambda_proxy_is_available = 2,
     _has_value_based_class_annotation = 4,
-    _is_shared_old_klass = 8
+    _verified_at_dump_time = 8
   };
 #endif
 
@@ -334,11 +334,11 @@ class Klass : public Metadata {
     NOT_CDS(return false;)
   }
 
-  void set_is_shared_old_klass() {
-    CDS_ONLY(_shared_class_flags |= _is_shared_old_klass;)
+  void set_verified_at_dump_time() {
+    CDS_ONLY(_shared_class_flags |= _verified_at_dump_time;)
   }
-  bool is_shared_old_klass() const {
-    CDS_ONLY(return (_shared_class_flags & _is_shared_old_klass) != 0;)
+  bool verified_at_dump_time() const {
+    CDS_ONLY(return (_shared_class_flags & _verified_at_dump_time) != 0;)
     NOT_CDS(return false;)
   }
 
diff --git a/src/hotspot/share/oops/klassVtable.cpp b/src/hotspot/share/oops/klassVtable.cpp
@@ -51,7 +51,7 @@ inline InstanceKlass* klassVtable::ik() const {
 }
 
 bool klassVtable::is_preinitialized_vtable() {
-  return _klass->is_shared() && !MetaspaceShared::remapped_readwrite() && !_klass->is_shared_old_klass();
+  return _klass->is_shared() && !MetaspaceShared::remapped_readwrite() && _klass->verified_at_dump_time();
 }
 
 
@@ -1094,8 +1094,8 @@ void itableMethodEntry::initialize(InstanceKlass* klass, Method* m) {
 #ifdef ASSERT
   if (MetaspaceShared::is_in_shared_metaspace((void*)&_method) &&
      !MetaspaceShared::remapped_readwrite() &&
-     !m->method_holder()->can_be_verified_at_dumptime() &&
-     !klass->can_be_verified_at_dumptime()) {
+     m->method_holder()->verified_at_dump_time() &&
+     klass->verified_at_dump_time()) {
     // At runtime initialize_itable is rerun as part of link_class_impl()
     // for a shared class loaded by the non-boot loader.
     // The dumptime itable method entry should be the same as the runtime entry.
diff --git a/src/hotspot/share/oops/method.cpp b/src/hotspot/share/oops/method.cpp
@@ -409,7 +409,7 @@ void Method::remove_unshareable_info() {
 }
 
 void Method::set_vtable_index(int index) {
-  if (is_shared() && !MetaspaceShared::remapped_readwrite() && !method_holder()->is_shared_old_klass()) {
+  if (is_shared() && !MetaspaceShared::remapped_readwrite() && method_holder()->verified_at_dump_time()) {
     // At runtime initialize_vtable is rerun as part of link_class_impl()
     // for a shared class loaded by the non-boot loader to obtain the loader
     // constraints based on the runtime classloaders' context.
@@ -420,7 +420,7 @@ void Method::set_vtable_index(int index) {
 }
 
 void Method::set_itable_index(int index) {
-  if (is_shared() && !MetaspaceShared::remapped_readwrite() && !method_holder()->is_shared_old_klass()) {
+  if (is_shared() && !MetaspaceShared::remapped_readwrite() && method_holder()->verified_at_dump_time()) {
     // At runtime initialize_itable is rerun as part of link_class_impl()
     // for a shared class loaded by the non-boot loader to obtain the loader
     // constraints based on the runtime classloaders' context. The dumptime

Original file line number	Diff line number	Diff line change
`@@ -1411,7 +1411,7 @@ bool SystemDictionaryShared::check_for_exclusion_impl(InstanceKlass* k) {`
`1411`	`1411`	`if (has_class_failed_verification(k)) {`
`1412`	`1412`	`return warn_excluded(k, "Failed verification");`
`1413`	`1413`	`} else {`
`1414`		`- if (!k->can_be_verified_at_dumptime()) {`
	`1414`	`+ if (k->can_be_verified_at_dumptime()) {`
`1415`	`1415`	`return warn_excluded(k, "Not linked");`
`1416`	`1416`	`}`
`1417`	`1417`	`}`
`@@ -1425,7 +1425,7 @@ bool SystemDictionaryShared::check_for_exclusion_impl(InstanceKlass* k) {`
`1425`	`1425`	`return true;`
`1426`	`1426`	`}`
`1427`	`1427`
`1428`		`- if (k->can_be_verified_at_dumptime() && k->is_linked()) {`
	`1428`	`+ if (!k->can_be_verified_at_dumptime() && k->is_linked()) {`
`1429`	`1429`	`return warn_excluded(k, "Old class has been linked");`
`1430`	`1430`	`}`
`1431`	`1431`
Original file line number	Diff line number	Diff line change
`@@ -571,7 +571,7 @@ void Rewriter::rewrite(InstanceKlass* klass, TRAPS) {`
`571`	`571`	`#if INCLUDE_CDS`
`572`	`572`	`if (klass->is_shared()) {`
`573`	`573`	`assert(!klass->is_rewritten(), "rewritten shared classes cannot be rewritten again");`
`574`		`- assert(klass->can_be_verified_at_dumptime(), "only shared old classes aren't rewritten");`
	`574`	`+ assert(!klass->can_be_verified_at_dumptime(), "only shared old classes aren't rewritten");`
`575`	`575`	`}`
`576`	`576`	`#endif // INCLUDE_CDS`
`577`	`577`	`ResourceMark rm(THREAD);`
Original file line number	Diff line number	Diff line change
`@@ -370,7 +370,7 @@ void ConstantPool::restore_unshareable_info(TRAPS) {`
`370`	`370`	`}`
`371`	`371`
`372`	`372`	`void ConstantPool::remove_unshareable_info() {`
`373`		`- if (!_pool_holder->is_linked() && _pool_holder->is_shared_old_klass()) {`
	`373`	`+ if (!_pool_holder->is_linked() && !_pool_holder->verified_at_dump_time()) {`
`374`	`374`	`return;`
`375`	`375`	`}`
`376`	`376`	`// Resolved references are not in the shared archive.`
Original file line number	Diff line number	Diff line change
`@@ -2403,8 +2403,8 @@ void InstanceKlass::metaspace_pointers_do(MetaspaceClosure* it) {`
`2403`	`2403`	`void InstanceKlass::remove_unshareable_info() {`
`2404`	`2404`
`2405`	`2405`	`if (can_be_verified_at_dumptime()) {`
`2406`		`- // Set the old class bit.`
`2407`		`- set_is_shared_old_klass();`
	`2406`	`+ // Remember this so we can avoid walking the hierarchy at runtime.`
	`2407`	`+ set_verified_at_dump_time();`
`2408`	`2408`	`}`
`2409`	`2409`
`2410`	`2410`	`Klass::remove_unshareable_info();`
`@@ -2549,19 +2549,19 @@ void InstanceKlass::restore_unshareable_info(ClassLoaderData* loader_data, Handl`
`2549`	`2549`	`// Verification of archived old classes will be performed during run time.`
`2550`	`2550`	`bool InstanceKlass::can_be_verified_at_dumptime() const {`
`2551`	`2551`	`if (major_version() < 50 /JAVA_6_VERSION/) {`
`2552`		`- return true;`
	`2552`	`+ return false;`
`2553`	`2553`	`}`
`2554`		`- if (java_super() != NULL && java_super()->can_be_verified_at_dumptime()) {`
`2555`		`- return true;`
	`2554`	`+ if (java_super() != NULL && !java_super()->can_be_verified_at_dumptime()) {`
	`2555`	`+ return false;`
`2556`	`2556`	`}`
`2557`	`2557`	`Array<InstanceKlass> interfaces = local_interfaces();`
`2558`	`2558`	`int len = interfaces->length();`
`2559`	`2559`	`for (int i = 0; i < len; i++) {`
`2560`		`- if (interfaces->at(i)->can_be_verified_at_dumptime()) {`
`2561`		`- return true;`
	`2560`	`+ if (!interfaces->at(i)->can_be_verified_at_dumptime()) {`
	`2561`	`+ return false;`
`2562`	`2562`	`}`
`2563`	`2563`	`}`
`2564`		`- return false;`
	`2564`	`+ return true;`
`2565`	`2565`	`}`
`2566`	`2566`
`2567`	`2567`	`void InstanceKlass::set_shared_class_loader_type(s2 loader_type) {`