8175383: JVM should throw NCDFE if ACC_MODULE and CONSTANT_Module/Package are set
authorhseigel
Mon, 06 Mar 2017 09:45:58 -0500
changeset 44239 fdd97dc2367b
parent 44238 a81138486a91
child 44240 c884d44d62d8
8175383: JVM should throw NCDFE if ACC_MODULE and CONSTANT_Module/Package are set Summary: If bad constant is seen, save it to throw CFE if ACC_MODULE is not in access_flags Reviewed-by: dholmes, acorn, lfoltan, gtriantafill
hotspot/src/share/vm/classfile/classFileParser.cpp
hotspot/src/share/vm/classfile/classFileParser.hpp
hotspot/test/runtime/constantPool/ACCModule52.java
hotspot/test/runtime/constantPool/ConstModule.java
--- a/hotspot/src/share/vm/classfile/classFileParser.cpp	Sun Mar 05 22:25:52 2017 -0800
+++ b/hotspot/src/share/vm/classfile/classFileParser.cpp	Mon Mar 06 09:45:58 2017 -0500
@@ -105,6 +105,12 @@
 
 #define JAVA_9_VERSION                    53
 
+void ClassFileParser::set_class_bad_constant_seen(short bad_constant) {
+  assert((bad_constant == 19 || bad_constant == 20) && _major_version >= JAVA_9_VERSION,
+         "Unexpected bad constant pool entry");
+  if (_bad_constant_seen == 0) _bad_constant_seen = bad_constant;
+}
+
 void ClassFileParser::parse_constant_pool_entries(const ClassFileStream* const stream,
                                                   ConstantPool* cp,
                                                   const int length,
@@ -302,6 +308,18 @@
         }
         break;
       }
+      case 19:
+      case 20: {
+        // Record that an error occurred in these two cases but keep parsing so
+        // that ACC_Module can be checked for in the access_flags.  Need to
+        // throw NoClassDefFoundError in that case.
+        if (_major_version >= JAVA_9_VERSION) {
+          cfs->guarantee_more(3, CHECK);
+          cfs->get_u2_fast();
+          set_class_bad_constant_seen(tag);
+          break;
+        }
+      }
       default: {
         classfile_parse_error("Unknown constant tag %u in class file %s",
                               tag,
@@ -359,14 +377,18 @@
 #endif
 
 void ClassFileParser::parse_constant_pool(const ClassFileStream* const stream,
-                                          ConstantPool* const cp,
-                                          const int length,
-                                          TRAPS) {
+                                         ConstantPool* const cp,
+                                         const int length,
+                                         TRAPS) {
   assert(cp != NULL, "invariant");
   assert(stream != NULL, "invariant");
 
   // parsing constant pool entries
   parse_constant_pool_entries(stream, cp, length, CHECK);
+  if (class_bad_constant_seen() != 0) {
+    // a bad CP entry has been detected previously so stop parsing and just return.
+    return;
+  }
 
   int index = 1;  // declared outside of loops for portability
 
@@ -5558,6 +5580,7 @@
   _protection_domain(protection_domain),
   _access_flags(),
   _pub_level(pub_level),
+  _bad_constant_seen(0),
   _synthetic_flag(false),
   _sde_length(false),
   _sde_buffer(NULL),
@@ -5765,9 +5788,15 @@
 
   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, CHECK);
+  }
+
   _access_flags.set_flags(flags);
 
-
   // This class and superclass
   _this_class_index = stream->get_u2_fast();
   check_property(
--- a/hotspot/src/share/vm/classfile/classFileParser.hpp	Sun Mar 05 22:25:52 2017 -0800
+++ b/hotspot/src/share/vm/classfile/classFileParser.hpp	Mon Mar 06 09:45:58 2017 -0500
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2017, 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
@@ -122,6 +122,15 @@
   // for tracing and notifications
   Publicity _pub_level;
 
+  // Used to keep track of whether a constant pool item 19 or 20 is found.  These
+  // correspond to CONSTANT_Module and CONSTANT_Package tags and are not allowed
+  // in regular class files.  For class file version >= 53, a CFE cannot be thrown
+  // immediately when these are seen because a NCDFE must be thrown if the class's
+  // access_flags have ACC_MODULE set.  But, the access_flags haven't been looked
+  // at yet.  So, the bad constant pool item is cached here.  A value of zero
+  // means that no constant pool item 19 or 20 was found.
+  short _bad_constant_seen;
+
   // class attributes parsed before the instance klass is created:
   bool _synthetic_flag;
   int _sde_length;
@@ -161,6 +170,8 @@
   void fill_instance_klass(InstanceKlass* ik, bool cf_changed_in_CFLH, TRAPS);
   void set_klass(InstanceKlass* instance);
 
+  void set_class_bad_constant_seen(short bad_constant);
+  short class_bad_constant_seen() { return  _bad_constant_seen; }
   void set_class_synthetic_flag(bool x)        { _synthetic_flag = x; }
   void set_class_sourcefile_index(u2 x)        { _sourcefile_index = x; }
   void set_class_generic_signature_index(u2 x) { _generic_signature_index = x; }
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/hotspot/test/runtime/constantPool/ACCModule52.java	Mon Mar 06 09:45:58 2017 -0500
@@ -0,0 +1,69 @@
+/*
+ * Copyright (c) 2017, 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+import jdk.internal.org.objectweb.asm.*;
+
+/*
+ * @test
+ * @summary Test that the JVM ignores ACC_MODULE if it is set for a version
+ *          52 class file.
+ * @bug 8175383
+ * @library /test/lib
+ * @modules java.base/jdk.internal.org.objectweb.asm
+ * @compile -XDignore.symbol.file ACCModule52.java
+ * @run main ACCModule52
+ */
+
+public class ACCModule52 {
+
+    static final String CLASS_NAME = "ACCModule52Pkg";
+
+    public static void main(String[] args) throws Exception {
+        int ACC_MODULE = 0x8000;
+        ClassWriter cw = new ClassWriter(0);
+        cw.visit(Opcodes.V1_8,
+                Opcodes.ACC_INTERFACE + Opcodes.ACC_ABSTRACT + Opcodes.ACC_SYNTHETIC + ACC_MODULE,
+                CLASS_NAME,
+                null,
+                "java/lang/Object",
+                null);
+
+        cw.visitEnd();
+        byte[] bytes = cw.toByteArray();
+
+
+        ClassLoader loader = new ClassLoader(ACCModule52.class.getClassLoader()) {
+            @Override
+            protected Class<?> findClass(String cn)throws ClassNotFoundException {
+                if (cn.equals(CLASS_NAME)) {
+                    Class superClass = super.defineClass(cn, bytes, 0, bytes.length);
+                } else {
+                    throw new ClassNotFoundException(cn);
+                }
+                return null;
+            }
+        };
+
+        Class<?> clazz = loader.loadClass(CLASS_NAME);
+    }
+}
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/hotspot/test/runtime/constantPool/ConstModule.java	Mon Mar 06 09:45:58 2017 -0500
@@ -0,0 +1,255 @@
+/*
+ * Copyright (c) 2017, 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+import jdk.internal.org.objectweb.asm.*;
+
+/*
+ * @test
+ * @summary Test scenarios for constant pool CONSTANT_Module and CONSTANT_Package
+ *          types, for class file versions 53 and 52, when ACC_MODULE is set and
+ *          not set in the access_flags.
+ * @bug 8175383
+ * @library /test/lib
+ * @modules java.base/jdk.internal.org.objectweb.asm
+ * @compile -XDignore.symbol.file ConstModule.java
+ * @run main ConstModule
+ */
+
+public class ConstModule {
+
+    static final int ACC_MODULE = 0x8000;
+    static final boolean MODULE_TEST = true;
+    static final boolean PACKAGE_TEST = false;
+    static final boolean CFE_EXCEPTION = true;
+    static final boolean NCDFE_EXCEPTION = false;
+
+    public static void main(String[] args) throws Exception {
+
+        // Test that the JVM throws CFE for constant pool CONSTANT_Module type, for
+        // class file version 53, when ACC_MODULE is not set in the access_flags.
+        ConstModule.write_and_load(Opcodes.V1_9,
+            Opcodes.ACC_INTERFACE + Opcodes.ACC_ABSTRACT + Opcodes.ACC_SYNTHETIC,
+            "jdk.fooMod", "FooMod", MODULE_TEST, CFE_EXCEPTION);
+
+        // Test that the JVM throws NCDFE for constant pool CONSTANT_Module type,
+        // for class file version 53, when ACC_MODULE is set in the access_flags.
+        ConstModule.write_and_load(Opcodes.V1_9,
+            Opcodes.ACC_INTERFACE + Opcodes.ACC_ABSTRACT + Opcodes.ACC_SYNTHETIC + ACC_MODULE,
+            "jdk.fooModACC", "FooModACC", MODULE_TEST, NCDFE_EXCEPTION);
+
+        // Test that the JVM throws CFE for constant pool CONSTANT_Module type, for
+        // class file version 52, even when ACC_MODULE is set in the access_flags.
+        ConstModule.write_and_load(Opcodes.V1_8,
+            Opcodes.ACC_INTERFACE + Opcodes.ACC_ABSTRACT + Opcodes.ACC_SYNTHETIC + ACC_MODULE,
+            "jdk.fooModACC52", "FooModACC52", MODULE_TEST, CFE_EXCEPTION);
+
+        // Test that the JVM throws CFE for constant pool CONSTANT_Package type, for
+        // class file version 53, when ACC_MODULE is not set in the access_flags.
+        ConstModule.write_and_load(Opcodes.V1_9,
+            Opcodes.ACC_INTERFACE + Opcodes.ACC_ABSTRACT + Opcodes.ACC_SYNTHETIC,
+            "jdk.fooPkg", "FooPkg", PACKAGE_TEST, CFE_EXCEPTION);
+
+        // Test that the JVM throws NCDFE for constant pool CONSTANT_Package type,
+        // for class file version 53, when ACC_MODULE is set in the access_flags.
+        ConstModule.write_and_load(Opcodes.V1_9,
+            Opcodes.ACC_INTERFACE + Opcodes.ACC_ABSTRACT + Opcodes.ACC_SYNTHETIC + ACC_MODULE,
+            "jdk.fooModACC", "FooModACC", PACKAGE_TEST, NCDFE_EXCEPTION);
+
+        // Test that the JVM throws CFE for constant pool CONSTANT_Package type, for
+        // class file version 52, even when ACC_MODULE is set in the access_flags.
+        ConstModule.write_and_load(Opcodes.V1_8,
+            Opcodes.ACC_INTERFACE + Opcodes.ACC_ABSTRACT + Opcodes.ACC_SYNTHETIC + ACC_MODULE,
+            "jdk.fooModACC52", "FooModACC52", PACKAGE_TEST, CFE_EXCEPTION);
+
+    }
+
+    public static void write_and_load(int version,
+                                      int access_flags,
+                                      String attr,
+                                      String class_name,
+                                      boolean module_test,
+                                      boolean throwCFE) throws Exception {
+        ClassWriter cw = new ClassWriter(0);
+        cw.visit(version,
+                 access_flags,
+                 class_name,
+                 null,
+                 "java/lang/Object",
+                 null);
+
+        if (module_test)
+            cw.visitAttribute(new TestModuleAttribute(attr));
+        else
+            cw.visitAttribute(new TestPackageAttribute(attr));
+
+        cw.visitEnd();
+        byte[] bytes = cw.toByteArray();
+
+
+        ClassLoader loader = new ClassLoader(ConstModule.class.getClassLoader()) {
+            @Override
+            protected Class<?> findClass(String cn)throws ClassNotFoundException {
+                if (cn.equals(class_name)) {
+                    try {
+                        Class superClass = super.defineClass(cn, bytes, 0, bytes.length);
+                        throw new RuntimeException("Expected ClassFormatError not thrown");
+                    } catch (java.lang.ClassFormatError e) {
+                       if (!throwCFE) {
+                           throw new RuntimeException("Unexpected ClassFormatError exception: " + e.getMessage());
+                       }
+                       if (module_test && !e.getMessage().contains(
+                           "Unknown constant tag 19 in class file")) {
+                           throw new RuntimeException("Wrong ClassFormatError exception: " + e.getMessage());
+                       } else if (!module_test && !e.getMessage().contains(
+                           "Unknown constant tag 20 in class file")) {
+                           throw new RuntimeException("Wrong ClassFormatError exception: " + e.getMessage());
+                       }
+                    } catch (java.lang.NoClassDefFoundError f) {
+                       if (throwCFE) {
+                           throw new RuntimeException("Unexpected NoClassDefFoundError exception: " + f.getMessage());
+                       }
+                       if (!f.getMessage().contains(
+                           "is not a class because access_flag ACC_MODULE is set")) {
+                           throw new RuntimeException("Wrong NoClassDefFoundError exception: " + f.getMessage());
+                       }
+                    }
+                } else {
+                    throw new ClassNotFoundException(cn);
+                }
+                return null;
+            }
+        };
+
+        Class<?> clazz = loader.loadClass(class_name);
+    }
+
+    /**
+     * ConstModuleAttr attribute.
+     *
+     * <pre> {@code
+     *
+     * MainClass_attribute {
+     *   // index to CONSTANT_utf8_info structure in constant pool representing
+     *   // the string "ConstModuleAttr"
+     *   u2 attribute_name_index;
+     *   u4 attribute_length;
+     *
+     *   // index to CONSTANT_Module_info structure
+     *   u2 module_name_index
+     * }
+     *
+     * } </pre>
+     */
+    public static class TestModuleAttribute extends Attribute {
+        private final String moduleName;
+
+        public TestModuleAttribute(String moduleName) {
+            super("ConstModuleAttr");
+            this.moduleName = moduleName;
+        }
+
+        public TestModuleAttribute() {
+            this(null);
+        }
+
+        @Override
+        protected Attribute read(ClassReader cr,
+                                 int off,
+                                 int len,
+                                 char[] buf,
+                                 int codeOff,
+                                 Label[] labels)
+        {
+            String mn = cr.readModule(off, buf);
+            off += 2;
+            return new TestModuleAttribute(mn);
+        }
+
+        @Override
+        protected ByteVector write(ClassWriter cw,
+                                   byte[] code,
+                                   int len,
+                                   int maxStack,
+                                   int maxLocals)
+        {
+            ByteVector attr = new ByteVector();
+            attr.putShort(cw.newModule(moduleName));
+            return attr;
+        }
+    }
+
+    /**
+     * ConstPackageAttr attribute.
+     *
+     * <pre> {@code
+     *
+     * MainClass_attribute {
+     *   // index to CONSTANT_utf8_info structure in constant pool representing
+     *   // the string "ConstPackageAttr"
+     *   u2 attribute_name_index;
+     *   u4 attribute_length;
+     *
+     *   // index to CONSTANT_Package_info structure
+     *   u2 module_name_index
+     * }
+     *
+     * } </pre>
+     */
+    public static class TestPackageAttribute extends Attribute {
+        private final String packageName;
+
+        public TestPackageAttribute(String packageName) {
+            super("ConstPackageAttr");
+            this.packageName = packageName;
+        }
+
+        public TestPackageAttribute() {
+            this(null);
+        }
+
+        @Override
+        protected Attribute read(ClassReader cr,
+                                 int off,
+                                 int len,
+                                 char[] buf,
+                                 int codeOff,
+                                 Label[] labels)
+        {
+            String mn = cr.readPackage(off, buf);
+            off += 2;
+            return new TestPackageAttribute(mn);
+        }
+
+        @Override
+        protected ByteVector write(ClassWriter cw,
+                                   byte[] code,
+                                   int len,
+                                   int maxStack,
+                                   int maxLocals)
+        {
+            ByteVector attr = new ByteVector();
+            attr.putShort(cw.newPackage(packageName));
+            return attr;
+        }
+    }
+}