8003549: (pack200) assertion errors when processing lambda class files with IMethods

Summary: add more check for opcode, sketch provided by jrose
Reviewed-by: jrose

src/share/classes/com/sun/java/util/jar/pack/Attribute.java

@@ -1287,19 +1287,24 @@ class Attribute implements Comparable<Attribute> {
                 if (localRef == 0) {
                     globalRef = null;  // N.B. global null reference is -1
                 } else {
-                    globalRef = holder.getCPMap()[localRef];
-                    if (e.refKind == CONSTANT_Signature
+                    Entry[] cpMap = holder.getCPMap();
+                    globalRef = (localRef >= 0 && localRef < cpMap.length
+                                    ? cpMap[localRef]
+                                    : null);
+                    byte tag = e.refKind;
+                    if (globalRef != null && tag == CONSTANT_Signature
                         && globalRef.getTag() == CONSTANT_Utf8) {
                         // Cf. ClassReader.readSignatureRef.
                         String typeName = globalRef.stringValue();
                         globalRef = ConstantPool.getSignatureEntry(typeName);
-                    } else if (e.refKind == CONSTANT_FieldSpecific) {
-                        assert(globalRef.getTag() >= CONSTANT_Integer);
-                        assert(globalRef.getTag() <= CONSTANT_String ||
-                               globalRef.getTag() >= CONSTANT_MethodHandle);
-                        assert(globalRef.getTag() <= CONSTANT_MethodType);
-                    } else if (e.refKind < CONSTANT_GroupFirst) {
-                        assert(e.refKind == globalRef.getTag());
+                    }
+                    String got = (globalRef == null
+                        ? "invalid CP index"
+                        : "type=" + ConstantPool.tagName(globalRef.tag));
+                    if (globalRef == null || !globalRef.tagMatches(tag)) {
+                        throw new IllegalArgumentException(
+                                "Bad constant, expected type=" +
+                                ConstantPool.tagName(tag) + " got " + got);
                     }
                 }
                 out.putRef(bandIndex, globalRef);

src/share/classes/com/sun/java/util/jar/pack/ClassReader.java

 /*
- * Copyright (c) 2001, 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2013, 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
@@ -54,6 +54,7 @@ class ClassReader {
     Package pkg;
     Class cls;
     long inPos;
+    long constantPoolLimit = -1;
     DataInputStream in;
     Map<Attribute.Layout, Attribute> attrDefs;
     Map<Attribute.Layout, String> attrCommands;
@@ -117,15 +118,33 @@ class ClassReader {
 
     private Entry readRef(byte tag) throws IOException {
         Entry e = readRef();
-        assert(e != null);
         assert(!(e instanceof UnresolvedEntry));
-        assert(e.tagMatches(tag));
+        checkTag(e, tag);
         return e;
     }
 
+    /** Throw a ClassFormatException if the entry does not match the expected tag type. */
+    private Entry checkTag(Entry e, byte tag) throws ClassFormatException {
+        if (e == null || !e.tagMatches(tag)) {
+            String where = (inPos == constantPoolLimit
+                                ? " in constant pool"
+                                : " at pos: " + inPos);
+            String got = (e == null
+                            ? "null CP index"
+                            : "type=" + ConstantPool.tagName(e.tag));
+            throw new ClassFormatException("Bad constant, expected type=" +
+                    ConstantPool.tagName(tag) +
+                    " got "+ got + ", in File: " + cls.file.nameString + where);
+        }
+        return e;
+    }
+    private Entry checkTag(Entry e, byte tag, boolean nullOK) throws ClassFormatException {
+        return nullOK && e == null ? null : checkTag(e, tag);
+    }
+
     private Entry readRefOrNull(byte tag) throws IOException {
         Entry e = readRef();
-        assert(e == null || e.tagMatches(tag));
+        checkTag(e, tag, true);
         return e;
     }
 
@@ -143,8 +162,10 @@ class ClassReader {
 
     private SignatureEntry readSignatureRef() throws IOException {
         // The class file stores a Utf8, but we want a Signature.
-        Entry e = readRef(CONSTANT_Utf8);
-        return ConstantPool.getSignatureEntry(e.stringValue());
+        Entry e = readRef(CONSTANT_Signature);
+        return (e != null && e.getTag() == CONSTANT_Utf8)
+                ? ConstantPool.getSignatureEntry(e.stringValue())
+                : (SignatureEntry) e;
     }
 
     void read() throws IOException {
@@ -279,6 +300,7 @@ class ClassReader {
                             " at pos: " + inPos);
             }
         }
+        constantPoolLimit = inPos;
 
         // Fix up refs, which might be out of order.
         while (fptr > 0) {
@@ -311,25 +333,25 @@ class ClassReader {
                 case CONSTANT_Fieldref:
                 case CONSTANT_Methodref:
                 case CONSTANT_InterfaceMethodref:
-                    ClassEntry      mclass = (ClassEntry)      cpMap[ref];
-                    DescriptorEntry mdescr = (DescriptorEntry) cpMap[ref2];
+                    ClassEntry      mclass = (ClassEntry)      checkTag(cpMap[ref],  CONSTANT_Class);
+                    DescriptorEntry mdescr = (DescriptorEntry) checkTag(cpMap[ref2], CONSTANT_NameandType);
                     cpMap[cpi] = ConstantPool.getMemberEntry((byte)tag, mclass, mdescr);
                     break;
                 case CONSTANT_NameandType:
-                    Utf8Entry mname = (Utf8Entry) cpMap[ref];
-                    Utf8Entry mtype = (Utf8Entry) cpMap[ref2];
+                    Utf8Entry mname = (Utf8Entry) checkTag(cpMap[ref],  CONSTANT_Utf8);
+                    Utf8Entry mtype = (Utf8Entry) checkTag(cpMap[ref2], CONSTANT_Signature);
                     cpMap[cpi] = ConstantPool.getDescriptorEntry(mname, mtype);
                     break;
                 case CONSTANT_MethodType:
-                    cpMap[cpi] = ConstantPool.getMethodTypeEntry((Utf8Entry) cpMap[ref]);
+                    cpMap[cpi] = ConstantPool.getMethodTypeEntry((Utf8Entry) checkTag(cpMap[ref], CONSTANT_Signature));
                     break;
                 case CONSTANT_MethodHandle:
                     byte refKind = (byte)(-1 ^ ref);
-                    MemberEntry memRef = (MemberEntry) cpMap[ref2];
+                    MemberEntry memRef = (MemberEntry) checkTag(cpMap[ref2], CONSTANT_AnyMember);
                     cpMap[cpi] = ConstantPool.getMethodHandleEntry(refKind, memRef);
                     break;
                 case CONSTANT_InvokeDynamic:
-                    DescriptorEntry idescr = (DescriptorEntry) cpMap[ref2];
+                    DescriptorEntry idescr = (DescriptorEntry) checkTag(cpMap[ref2], CONSTANT_NameandType);
                     cpMap[cpi] = new UnresolvedEntry((byte)tag, (-1 ^ ref), idescr);
                     // Note that ref must be resolved later, using the BootstrapMethods attribute.
                     break;
@@ -541,7 +563,8 @@ class ClassReader {
         code.max_locals = readUnsignedShort();
         code.bytes = new byte[readInt()];
         in.readFully(code.bytes);
-        Instruction.opcodeChecker(code.bytes);
+        Entry[] cpMap = cls.getCPMap();
+        Instruction.opcodeChecker(code.bytes, cpMap);
         int nh = readUnsignedShort();
         code.setHandlerCount(nh);
         for (int i = 0; i < nh; i++) {
@@ -559,7 +582,7 @@ class ClassReader {
             MethodHandleEntry bsmRef = (MethodHandleEntry) readRef(CONSTANT_MethodHandle);
             Entry[] argRefs = new Entry[readUnsignedShort()];
             for (int j = 0; j < argRefs.length; j++) {
-                argRefs[j] = readRef();
+                argRefs[j] = readRef(CONSTANT_LoadableValue);
             }
             bsms[i] = ConstantPool.getBootstrapMethodEntry(bsmRef, argRefs);
         }

src/share/classes/com/sun/java/util/jar/pack/ConstantPool.java

 /*
- * Copyright (c) 2001, 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2013, 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
@@ -243,8 +243,32 @@ class ConstantPool {
             return tag == CONSTANT_Double || tag == CONSTANT_Long;
         }
 
-        public final boolean tagMatches(int tag) {
-            return (this.tag == tag);
+        public final boolean tagMatches(int matchTag) {
+            if (tag == matchTag)
+                return true;
+            byte[] allowedTags;
+            switch (matchTag) {
+                case CONSTANT_All:
+                    return true;
+                case CONSTANT_Signature:
+                    return tag == CONSTANT_Utf8;  // format check also?
+                case CONSTANT_LoadableValue:
+                    allowedTags = LOADABLE_VALUE_TAGS;
+                    break;
+                case CONSTANT_AnyMember:
+                    allowedTags = ANY_MEMBER_TAGS;
+                    break;
+                case CONSTANT_FieldSpecific:
+                    allowedTags = FIELD_SPECIFIC_TAGS;
+                    break;
+                default:
+                    return false;
+            }
+            for (byte b : allowedTags) {
+                if (b == tag)
+                    return true;
+            }
+            return false;
         }
 
         public String toString() {

src/share/classes/com/sun/java/util/jar/pack/Instruction.java

 /*
- * Copyright (c) 2001, 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2013, 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
@@ -647,7 +647,7 @@ class Instruction  {
         }
     }
 
-    public static void opcodeChecker(byte[] code) throws FormatException {
+    public static void opcodeChecker(byte[] code, ConstantPool.Entry[] cpMap) throws FormatException {
         Instruction i = at(code, 0);
         while (i != null) {
             int opcode = i.getBC();
@@ -655,6 +655,16 @@ class Instruction  {
                 String message = "illegal opcode: " + opcode + " " + i;
                 throw new FormatException(message);
             }
+            ConstantPool.Entry e = i.getCPRef(cpMap);
+            if (e != null) {
+                byte tag = i.getCPTag();
+                if (!e.tagMatches(tag)) {
+                    String message = "illegal reference, expected type=" +
+                                     ConstantPool.tagName(tag) + ": " +
+                                     i.toString(cpMap);
+                    throw new FormatException(message);
+                }
+            }
             i = i.next();
         }
     }

src/share/classes/com/sun/java/util/jar/pack/PackageWriter.java

@@ -1618,6 +1618,16 @@ class PackageWriter extends BandStructure {
                     bc_which = null;
                     assert(false);
                 }
+                if (ref != null && bc_which.index != null && !bc_which.index.contains(ref)) {
+                    // Crash and burn with a complaint if there are funny
+                    // references for this bytecode instruction.
+                    // Example:  invokestatic of a CONSTANT_InterfaceMethodref.
+                    String complaint = code.getMethod() +
+                        " contains a bytecode " + i +
+                        " with an unsupported constant reference; please use the pass-file option on this class.";
+                    Utils.log.warning(complaint);
+                    throw new IOException(complaint);
+                }
                 bc_codes.putByte(vbc);
                 bc_which.putRef(ref);
                 // handle trailing junk

src/share/classes/com/sun/java/util/jar/pack/PackerImpl.java

 /*
- * Copyright (c) 2003, 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2013, 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
@@ -180,6 +180,15 @@ public class PackerImpl  extends TLGlobals implements Pack200.Packer {
             }
             unknownAttrCommand = uaMode.intern();
         }
+        final String classFormatCommand;
+        {
+            String fmtMode = props.getProperty(Utils.CLASS_FORMAT_ERROR, Pack200.Packer.PASS);
+            if (!(Pack200.Packer.PASS.equals(fmtMode) ||
+                  Pack200.Packer.ERROR.equals(fmtMode))) {
+                throw new RuntimeException("Bad option: " + Utils.CLASS_FORMAT_ERROR + " = " + fmtMode);
+            }
+            classFormatCommand = fmtMode.intern();
+        }
 
         final Map<Attribute.Layout, Attribute> attrDefs;
         final Map<Attribute.Layout, String> attrCommands;
@@ -505,8 +514,7 @@ public class PackerImpl  extends TLGlobals implements Pack200.Packer {
                     }
                 } else if (ioe instanceof ClassReader.ClassFormatException) {
                     ClassReader.ClassFormatException ce = (ClassReader.ClassFormatException) ioe;
-                    // %% TODO: Do we invent a new property for this or reuse %%
-                    if (unknownAttrCommand.equals(Pack200.Packer.PASS)) {
+                    if (classFormatCommand.equals(Pack200.Packer.PASS)) {
                         Utils.log.info(ce.toString());
                         Utils.log.warning(message + " unknown class format: " +
                                 fname);

src/share/classes/com/sun/java/util/jar/pack/PropMap.java

 /*
- * Copyright (c) 2003, 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2013, 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
@@ -112,6 +112,11 @@ final class PropMap implements SortedMap<String, String>  {
         // Pass through files with unrecognized attributes by default.
         props.put(Pack200.Packer.UNKNOWN_ATTRIBUTE, Pack200.Packer.PASS);
 
+        // Pass through files with unrecognized format by default, also
+        // allow system property to be set
+        props.put(Utils.CLASS_FORMAT_ERROR,
+                System.getProperty(Utils.CLASS_FORMAT_ERROR, Pack200.Packer.PASS));
+
         // Default effort is 5, midway between 1 and 9.
         props.put(Pack200.Packer.EFFORT, "5");
 

src/share/classes/com/sun/java/util/jar/pack/Utils.java

 /*
- * Copyright (c) 2003, 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2013, 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,12 @@ class Utils {
      */
     static final String PACK_ZIP_ARCHIVE_MARKER_COMMENT = "PACK200";
 
+    /*
+     * behaviour when we hit a class format error, but not necessarily
+     * an unknown attribute, by default it is allowed to PASS.
+     */
+    static final String CLASS_FORMAT_ERROR = COM_PREFIX+"class.format.error";
+
     // Keep a TLS point to the global data and environment.
     // This makes it simpler to supply environmental options
     // to the engine code, especially the native code.

test/ProblemList.txt

@@ -321,9 +321,6 @@ sun/jvmstat/monitor/MonitoredVm/CR6672135.java                  generic-all
 tools/pack200/CommandLineTests.java                             generic-all
 tools/pack200/Pack200Test.java                                  generic-all
 
-# 8001163
-tools/pack200/AttributeTests.java                               generic-all
-
 # 7150569
 tools/launcher/UnicodeTest.java                                 macosx-all
 

test/tools/pack200/InstructionTests.java

+/*
+ * Copyright (c) 2013, 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 java.io.File;
+import java.nio.charset.Charset;
+import java.nio.file.Files;
+import java.util.ArrayList;
+import java.util.List;
+import static java.nio.file.StandardOpenOption.*;
+import java.util.regex.Pattern;
+
+/*
+ * @test
+ * @bug 8003549
+ * @summary tests class files instruction formats introduced in JSR-335
+ * @compile -XDignore.symbol.file Utils.java InstructionTests.java
+ * @run main InstructionTests
+ * @author ksrini
+ */
+public class InstructionTests {
+    public static void main(String... args) throws Exception {
+        testInvokeOpCodes();
+    }
+    /*
+     * the following should produce invokestatic and invokespecial
+     * on InterfaceMethodRefs vs. MethodRefs, packer/unpacker should work
+     */
+    static void testInvokeOpCodes() throws Exception {
+        List<String> scratch = new ArrayList<>();
+        final String fname = "A";
+        String javaFileName = fname + Utils.JAVA_FILE_EXT;
+        scratch.add("interface IntIterator {");
+        scratch.add("    default void forEach(){}");
+        scratch.add("    static void next() {}");
+        scratch.add("}");
+        scratch.add("class A implements IntIterator {");
+        scratch.add("public void forEach(Object o){");
+        scratch.add("IntIterator.super.forEach();");
+        scratch.add("IntIterator.next();");
+        scratch.add("}");
+        scratch.add("}");
+        File cwd = new File(".");
+        File javaFile = new File(cwd, javaFileName);
+        Files.write(javaFile.toPath(), scratch, Charset.defaultCharset(),
+                CREATE, TRUNCATE_EXISTING);
+
+        // make sure we have -g so that we  compare LVT and LNT entries
+        Utils.compiler("-g", javaFile.getName());
+
+        // jar the file up
+        File testjarFile = new File(cwd, "test" + Utils.JAR_FILE_EXT);
+        Utils.jar("cvf", testjarFile.getName(), ".");
+
+        // pack using --repack
+        File outjarFile = new File(cwd, "out" + Utils.JAR_FILE_EXT);
+        scratch.clear();
+        scratch.add(Utils.getPack200Cmd());
+        scratch.add("-J-ea");
+        scratch.add("-J-esa");
+        scratch.add("--repack");
+        scratch.add(outjarFile.getName());
+        scratch.add(testjarFile.getName());
+        List<String> output = Utils.runExec(scratch);
+        // TODO remove this when we get bc escapes working correctly
+        // this test anyhow would  fail  at that time
+        findString("WARNING: Passing.*" + fname + Utils.CLASS_FILE_EXT,
+                        output);
+
+        Utils.doCompareVerify(testjarFile, outjarFile);
+    }
+
+    static boolean findString(String str, List<String> list) {
+        Pattern p = Pattern.compile(str);
+        for (String x : list) {
+            if (p.matcher(x).matches())
+                return true;
+        }
+        throw new RuntimeException("Error: " + str + " not found in output");
+    }
+}

test/tools/pack200/pack200-verifier/src/xmlkit/ClassReader.java

@@ -57,8 +57,10 @@ import com.sun.tools.classfile.MethodParameters_attribute;
 import com.sun.tools.classfile.Opcode;
 import com.sun.tools.classfile.RuntimeInvisibleAnnotations_attribute;
 import com.sun.tools.classfile.RuntimeInvisibleParameterAnnotations_attribute;
+import com.sun.tools.classfile.RuntimeInvisibleTypeAnnotations_attribute;
 import com.sun.tools.classfile.RuntimeVisibleAnnotations_attribute;
 import com.sun.tools.classfile.RuntimeVisibleParameterAnnotations_attribute;
+import com.sun.tools.classfile.RuntimeVisibleTypeAnnotations_attribute;
 import com.sun.tools.classfile.Signature_attribute;
 import com.sun.tools.classfile.SourceDebugExtension_attribute;
 import com.sun.tools.classfile.SourceFile_attribute;
@@ -1214,6 +1216,21 @@ class AttributeVisitor implements Attribute.Visitor<Element, Element> {
         p.add(e);
         return null;
     }
+
+    /*
+     * TODO
+     * add these two for now to keep the compiler happy, we will implement
+     * these along with the JSR-308 changes.
+     */
+    @Override
+    public Element visitRuntimeVisibleTypeAnnotations(RuntimeVisibleTypeAnnotations_attribute rvta, Element p) {
+        throw new UnsupportedOperationException("Not supported yet.");
+    }
+
+    @Override
+    public Element visitRuntimeInvisibleTypeAnnotations(RuntimeInvisibleTypeAnnotations_attribute rita, Element p) {
+        throw new UnsupportedOperationException("Not supported yet.");
+    }
 }
 
 class StackMapVisitor implements StackMapTable_attribute.stack_map_frame.Visitor<Element, Void> {