From 01aa3df2f2a04ac4bee2517ff977d4d95c4a2c40 Mon Sep 17 00:00:00 2001 From: ksrini Date: Fri, 1 Feb 2013 22:12:52 -0800 Subject: [PATCH] 8003549: (pack200) assertion errors when processing lambda class files with IMethods Summary: add more check for opcode, sketch provided by jrose Reviewed-by: jrose --- .../com/sun/java/util/jar/pack/Attribute.java | 23 +++-- .../sun/java/util/jar/pack/ClassReader.java | 53 +++++++--- .../sun/java/util/jar/pack/ConstantPool.java | 30 +++++- .../sun/java/util/jar/pack/Instruction.java | 14 ++- .../sun/java/util/jar/pack/PackageWriter.java | 10 ++ .../sun/java/util/jar/pack/PackerImpl.java | 14 ++- .../com/sun/java/util/jar/pack/PropMap.java | 7 +- .../com/sun/java/util/jar/pack/Utils.java | 8 +- test/ProblemList.txt | 3 - test/tools/pack200/InstructionTests.java | 99 +++++++++++++++++++ .../src/xmlkit/ClassReader.java | 17 ++++ 11 files changed, 241 insertions(+), 37 deletions(-) create mode 100644 test/tools/pack200/InstructionTests.java diff --git a/src/share/classes/com/sun/java/util/jar/pack/Attribute.java b/src/share/classes/com/sun/java/util/jar/pack/Attribute.java index 9e891fc7c..f6ff03026 100644 --- a/src/share/classes/com/sun/java/util/jar/pack/Attribute.java +++ b/src/share/classes/com/sun/java/util/jar/pack/Attribute.java @@ -1287,19 +1287,24 @@ class Attribute implements Comparable { 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); diff --git a/src/share/classes/com/sun/java/util/jar/pack/ClassReader.java b/src/share/classes/com/sun/java/util/jar/pack/ClassReader.java index 2ca2d0926..3df8706da 100644 --- a/src/share/classes/com/sun/java/util/jar/pack/ClassReader.java +++ b/src/share/classes/com/sun/java/util/jar/pack/ClassReader.java @@ -1,5 +1,5 @@ /* - * 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 attrDefs; Map 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); } diff --git a/src/share/classes/com/sun/java/util/jar/pack/ConstantPool.java b/src/share/classes/com/sun/java/util/jar/pack/ConstantPool.java index be4da54d2..ad2175f99 100644 --- a/src/share/classes/com/sun/java/util/jar/pack/ConstantPool.java +++ b/src/share/classes/com/sun/java/util/jar/pack/ConstantPool.java @@ -1,5 +1,5 @@ /* - * 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() { diff --git a/src/share/classes/com/sun/java/util/jar/pack/Instruction.java b/src/share/classes/com/sun/java/util/jar/pack/Instruction.java index f488bede3..31ee22b05 100644 --- a/src/share/classes/com/sun/java/util/jar/pack/Instruction.java +++ b/src/share/classes/com/sun/java/util/jar/pack/Instruction.java @@ -1,5 +1,5 @@ /* - * 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(); } } diff --git a/src/share/classes/com/sun/java/util/jar/pack/PackageWriter.java b/src/share/classes/com/sun/java/util/jar/pack/PackageWriter.java index e69904511..47959294f 100644 --- a/src/share/classes/com/sun/java/util/jar/pack/PackageWriter.java +++ b/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 diff --git a/src/share/classes/com/sun/java/util/jar/pack/PackerImpl.java b/src/share/classes/com/sun/java/util/jar/pack/PackerImpl.java index c7ef4ee20..0dc88a108 100644 --- a/src/share/classes/com/sun/java/util/jar/pack/PackerImpl.java +++ b/src/share/classes/com/sun/java/util/jar/pack/PackerImpl.java @@ -1,5 +1,5 @@ /* - * 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 attrDefs; final Map 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); diff --git a/src/share/classes/com/sun/java/util/jar/pack/PropMap.java b/src/share/classes/com/sun/java/util/jar/pack/PropMap.java index 6199c77b5..244850403 100644 --- a/src/share/classes/com/sun/java/util/jar/pack/PropMap.java +++ b/src/share/classes/com/sun/java/util/jar/pack/PropMap.java @@ -1,5 +1,5 @@ /* - * 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 { // 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"); diff --git a/src/share/classes/com/sun/java/util/jar/pack/Utils.java b/src/share/classes/com/sun/java/util/jar/pack/Utils.java index 5edd73e21..ebe0960db 100644 --- a/src/share/classes/com/sun/java/util/jar/pack/Utils.java +++ b/src/share/classes/com/sun/java/util/jar/pack/Utils.java @@ -1,5 +1,5 @@ /* - * 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. diff --git a/test/ProblemList.txt b/test/ProblemList.txt index 5e181ac7e..7b7634771 100644 --- a/test/ProblemList.txt +++ b/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 diff --git a/test/tools/pack200/InstructionTests.java b/test/tools/pack200/InstructionTests.java new file mode 100644 index 000000000..ce92c0ed5 --- /dev/null +++ b/test/tools/pack200/InstructionTests.java @@ -0,0 +1,99 @@ +/* + * 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 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 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 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"); + } +} diff --git a/test/tools/pack200/pack200-verifier/src/xmlkit/ClassReader.java b/test/tools/pack200/pack200-verifier/src/xmlkit/ClassReader.java index 177849872..db7d04657 100644 --- a/test/tools/pack200/pack200-verifier/src/xmlkit/ClassReader.java +++ b/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 { 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 { -- GitLab