8294051: [lworld] runtime/AccModule/ConstModule.java fails with incorrect error message

Reviewed-by: dsimms

Author: coleenp

src/hotspot/share/classfile/classFileParser.cpp

@@ -3063,15 +3063,15 @@ u2 ClassFileParser::parse_classfile_inner_classes_attribute(const ClassFileStrea
                          "Class is both outer and inner class in class file %s", CHECK_0);
     }
 
-    u2 recognized_modifiers = RECOGNIZED_INNER_CLASS_MODIFIERS;
+    // Access flags
+    u2 flags;
     // JVM_ACC_MODULE is defined in JDK-9 and later.
     if (_major_version >= JAVA_9_VERSION) {
-      recognized_modifiers |= JVM_ACC_MODULE;
+      flags = cfs->get_u2_fast() & (RECOGNIZED_INNER_CLASS_MODIFIERS | JVM_ACC_MODULE);
+    } else {
+      flags = cfs->get_u2_fast() & RECOGNIZED_INNER_CLASS_MODIFIERS;
     }
 
-    // Access flags
-    u2 flags = cfs->get_u2_fast() & recognized_modifiers;
-
     if ((flags & JVM_ACC_INTERFACE) && _major_version < JAVA_6_VERSION) {
       // Set abstract bit for old class files for backward compatibility
       flags |= JVM_ACC_ABSTRACT;
@@ -3085,8 +3085,7 @@ u2 ClassFileParser::parse_classfile_inner_classes_attribute(const ClassFileStrea
       }
     }
 
-    const char* name = inner_name_index == 0 ? "unnamed" : cp->symbol_at(inner_name_index)->as_utf8();
-    verify_legal_class_modifiers(flags, name, false, CHECK_0);
+    verify_legal_class_modifiers(flags, CHECK_0);
     AccessFlags inner_access_flags(flags);
 
     inner_classes->at_put(index++, inner_class_info_index);
@@ -4427,9 +4426,8 @@ static void check_illegal_static_method(const InstanceKlass* this_klass, TRAPS)
 
 // utility methods for format checking
 
-void ClassFileParser::verify_legal_class_modifiers(jint flags, const char* name, bool is_Object, TRAPS) const {
+void ClassFileParser::verify_legal_class_modifiers(jint flags, TRAPS) const {
   const bool is_module = (flags & JVM_ACC_MODULE) != 0;
-  const bool is_inner_class = name != nullptr;
   assert(_major_version >= JAVA_9_VERSION || !is_module, "JVM_ACC_MODULE should not be set");
   if (is_module) {
     ResourceMark rm(THREAD);
@@ -4464,24 +4462,13 @@ void ClassFileParser::verify_legal_class_modifiers(jint flags, const char* name,
     if (!valid_value_class) {
       class_note = " (a value class must be final or else abstract)";
     }
-    if (name == nullptr) { // Not an inner class
-      Exceptions::fthrow(
-        THREAD_AND_LOCATION,
-        vmSymbols::java_lang_ClassFormatError(),
-        "Illegal class modifiers in class %s%s: 0x%X",
-        _class_name->as_C_string(), class_note, flags
-      );
-      return;
-    } else {
-      // Names are all known to be < 64k so we know this formatted message is not excessively large.
-      Exceptions::fthrow(
-        THREAD_AND_LOCATION,
-        vmSymbols::java_lang_ClassFormatError(),
-        "Illegal class modifiers in declaration of inner class %s%s of class %s: 0x%X",
-        name, class_note, _class_name->as_C_string(), flags
-      );
-      return;
-    }
+    Exceptions::fthrow(
+      THREAD_AND_LOCATION,
+      vmSymbols::java_lang_ClassFormatError(),
+      "Illegal class modifiers in class %s%s: 0x%X",
+      _class_name->as_C_string(), class_note, flags
+    );
+    return;
   }
 }
 
@@ -5793,15 +5780,15 @@ void ClassFileParser::parse_stream(const ClassFileStream* const stream,
   // ACCESS FLAGS
   stream->guarantee_more(8, CHECK);  // flags, this_class, super_class, infs_len
 
-  u2 recognized_modifiers = JVM_RECOGNIZED_CLASS_MODIFIERS;
+  // Access flags
+  u2 flags;
   // JVM_ACC_MODULE is defined in JDK-9 and later.
   if (_major_version >= JAVA_9_VERSION) {
-    recognized_modifiers |= JVM_ACC_MODULE;
+    flags = stream->get_u2_fast() & (JVM_RECOGNIZED_CLASS_MODIFIERS | JVM_ACC_MODULE);
+  } else {
+    flags = stream->get_u2_fast() & JVM_RECOGNIZED_CLASS_MODIFIERS;
   }
 
-  // Access flags
-  u2 flags = stream->get_u2_fast() & recognized_modifiers;
-
   if ((flags & JVM_ACC_INTERFACE) && _major_version < JAVA_6_VERSION) {
     // Set abstract bit for old class files for backward compatibility
     flags |= JVM_ACC_ABSTRACT;
@@ -5816,6 +5803,17 @@ void ClassFileParser::parse_stream(const ClassFileStream* const stream,
     }
   }
 
+  verify_legal_class_modifiers(flags, CHECK);
+
+  short bad_constant = class_bad_constant_seen();
+  if (bad_constant != 0) {
+    // Do not throw CFE until after the access_flags are checked because if
+    // ACC_MODULE is set in the access flags, then NCDFE must be thrown, not CFE.
+    classfile_parse_error("Unknown constant tag %u in class file %s", bad_constant, THREAD);
+    return;
+  }
+
+  _access_flags.set_flags(flags);
 
   // This class and superclass
   _this_class_index = stream->get_u2_fast();
@@ -5828,20 +5826,6 @@ void ClassFileParser::parse_stream(const ClassFileStream* const stream,
   Symbol* const class_name_in_cp = cp->klass_name_at(_this_class_index);
   assert(class_name_in_cp != nullptr, "class_name can't be null");
 
-  bool is_java_lang_Object = class_name_in_cp == vmSymbols::java_lang_Object();
-
-  verify_legal_class_modifiers(flags, nullptr, is_java_lang_Object, CHECK);
-
-  _access_flags.set_flags(flags);
-
-  short bad_constant = class_bad_constant_seen();
-  if (bad_constant != 0) {
-    // Do not throw CFE until after the access_flags are checked because if
-    // ACC_MODULE is set in the access flags, then NCDFE must be thrown, not CFE.
-    classfile_parse_error("Unknown constant tag %u in class file %s", bad_constant, THREAD);
-    return;
-  }
-
   // Don't need to check whether this class name is legal or not.
   // It has been checked when constant pool is parsed.
   // However, make sure it is not an array type.

src/hotspot/share/classfile/classFileParser.hpp

@@ -464,7 +464,7 @@ class ClassFileParser {
 
   void verify_class_version(u2 major, u2 minor, Symbol* class_name, TRAPS);
 
-  void verify_legal_class_modifiers(jint flags, const char* name, bool is_Object, TRAPS) const;
+  void verify_legal_class_modifiers(jint flags, TRAPS) const;
   void verify_legal_field_modifiers(jint flags,
                                     AccessFlags class_access_flags,
                                     TRAPS) const;

test/hotspot/jtreg/ProblemList.txt

@@ -135,7 +135,6 @@ containers/docker/TestJcmdWithSideCar.java 8341518 linux-x64
 
 
 # Valhalla
-runtime/AccModule/ConstModule.java 8294051 generic-all
 runtime/valhalla/inlinetypes/CircularityTest.java 8349037 generic-all
 runtime/valhalla/inlinetypes/classloading/ConcurrentClassLoadingTest.java 8367412 linux-aarch64
 runtime/valhalla/inlinetypes/verifier/StrictInstanceFieldsTest.java CODETOOLS-7904031 generic-all