8171132: Improve class reading of invalid or out-of-range ConstantValue attributes
Reviewed-by: mcimadamore
--- a/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/code/TypeTag.java Mon Jan 02 18:31:30 2017 -0800
+++ b/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/code/TypeTag.java Wed Dec 21 15:40:08 2016 -0800
@@ -241,4 +241,21 @@
}
}
+ /** Returns true if the given value is within the allowed range for this type. */
+ public boolean checkRange(int value) {
+ switch (this) {
+ case BOOLEAN:
+ return 0 <= value && value <= 1;
+ case BYTE:
+ return Byte.MIN_VALUE <= value && value <= Byte.MAX_VALUE;
+ case CHAR:
+ return Character.MIN_VALUE <= value && value <= Character.MAX_VALUE;
+ case SHORT:
+ return Short.MIN_VALUE <= value && value <= Short.MAX_VALUE;
+ case INT:
+ return true;
+ default:
+ throw new AssertionError();
+ }
+ }
}
--- a/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java Mon Jan 02 18:31:30 2017 -0800
+++ b/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java Wed Dec 21 15:40:08 2016 -0800
@@ -2050,19 +2050,12 @@
int value = ((Number)t.constValue()).intValue();
switch (s.getTag()) {
case BYTE:
- if (Byte.MIN_VALUE <= value && value <= Byte.MAX_VALUE)
- return true;
- break;
case CHAR:
- if (Character.MIN_VALUE <= value && value <= Character.MAX_VALUE)
+ case SHORT:
+ case INT:
+ if (s.getTag().checkRange(value))
return true;
break;
- case SHORT:
- if (Short.MIN_VALUE <= value && value <= Short.MAX_VALUE)
- return true;
- break;
- case INT:
- return true;
case CLASS:
switch (unboxedType(s).getTag()) {
case BYTE:
--- a/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java Mon Jan 02 18:31:30 2017 -0800
+++ b/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java Wed Dec 21 15:40:08 2016 -0800
@@ -1028,8 +1028,45 @@
protected void read(Symbol sym, int attrLen) {
Object v = readPool(nextChar());
// Ignore ConstantValue attribute if field not final.
- if ((sym.flags() & FINAL) != 0)
- ((VarSymbol) sym).setData(v);
+ if ((sym.flags() & FINAL) == 0) {
+ return;
+ }
+ VarSymbol var = (VarSymbol) sym;
+ switch (var.type.getTag()) {
+ case BOOLEAN:
+ case BYTE:
+ case CHAR:
+ case SHORT:
+ case INT:
+ checkType(var, Integer.class, v);
+ break;
+ case LONG:
+ checkType(var, Long.class, v);
+ break;
+ case FLOAT:
+ checkType(var, Float.class, v);
+ break;
+ case DOUBLE:
+ checkType(var, Double.class, v);
+ break;
+ case CLASS:
+ Assert.check(var.type.tsym == syms.stringType.tsym);
+ checkType(var, String.class, v);
+ break;
+ default:
+ // ignore ConstantValue attribute if type is not primitive or String
+ return;
+ }
+ if (v instanceof Integer && !var.type.getTag().checkRange((Integer) v)) {
+ throw badClassFile("bad.constant.range", v, var, var.type);
+ }
+ var.setData(v);
+ }
+
+ void checkType(Symbol var, Class<?> clazz, Object value) {
+ if (!clazz.isInstance(value)) {
+ throw badClassFile("bad.constant.value", value, var, clazz.getSimpleName());
+ }
}
},
--- a/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties Mon Jan 02 18:31:30 2017 -0800
+++ b/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties Wed Dec 21 15:40:08 2016 -0800
@@ -1913,6 +1913,14 @@
compiler.misc.class.file.not.found=\
class file for {0} not found
+# 0: string (constant value), 1: symbol (constant field), 2: type (field type)
+compiler.misc.bad.constant.range=\
+ constant value ''{0}'' for {1} is outside the expected range for {2}
+
+# 0: string (constant value), 1: symbol (constant field), 2: string (expected class)
+compiler.misc.bad.constant.value=\
+ bad constant value ''{0}'' for {1}, expected {2}
+
# 0: string (classfile major version), 1: string (classfile minor version)
compiler.misc.invalid.default.interface=\
default method found in version {0}.{1} classfile
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/langtools/test/tools/javac/classreader/8171132/BadConstantValue.java Wed Dec 21 15:40:08 2016 -0800
@@ -0,0 +1,226 @@
+/*
+ * Copyright 2016 Google, Inc. 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. Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * 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.
+ */
+
+/*
+ * @test
+ * @bug 8171132
+ * @summary Improve class reading of invalid or out-of-range ConstantValue attributes
+ * @modules jdk.jdeps/com.sun.tools.classfile
+ * jdk.compiler/com.sun.tools.javac.api
+ * jdk.compiler/com.sun.tools.javac.code
+ * jdk.compiler/com.sun.tools.javac.jvm
+ * jdk.compiler/com.sun.tools.javac.main
+ * jdk.compiler/com.sun.tools.javac.util
+ * @build BadConstantValue
+ * @run main BadConstantValue
+ */
+
+import com.sun.tools.classfile.Attribute;
+import com.sun.tools.classfile.ClassFile;
+import com.sun.tools.classfile.ClassWriter;
+import com.sun.tools.classfile.ConstantPool.CONSTANT_Integer_info;
+import com.sun.tools.classfile.ConstantValue_attribute;
+import com.sun.tools.classfile.Field;
+import com.sun.tools.javac.api.JavacTaskImpl;
+import com.sun.tools.javac.code.ClassFinder.BadClassFile;
+import com.sun.tools.javac.code.Symtab;
+import com.sun.tools.javac.jvm.Target;
+import com.sun.tools.javac.util.Assert;
+import com.sun.tools.javac.util.JCDiagnostic;
+import java.io.File;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.util.Arrays;
+import java.util.Objects;
+import javax.tools.JavaCompiler;
+import javax.tools.ToolProvider;
+
+public class BadConstantValue {
+
+ static final File classesdir = new File("badconstants");
+
+ public static void main(String[] args) throws Exception {
+ // report errors for ConstantValues of the wrong type
+ testInvalidConstantType("int");
+ testInvalidConstantType("short");
+ testInvalidConstantType("byte");
+ testInvalidConstantType("char");
+ testInvalidConstantType("boolean");
+
+ // report errors for ConstantValues outside the expected range
+ testValidConstRange("int", Integer.MAX_VALUE);
+ testValidConstRange("int", Integer.MIN_VALUE);
+
+ testValidConstRange("short", Short.MAX_VALUE);
+ testValidConstRange("short", Short.MIN_VALUE);
+ testInvalidConstRange("short", Short.MAX_VALUE + 1);
+ testInvalidConstRange("short", Short.MIN_VALUE - 1);
+
+ testValidConstRange("byte", Byte.MAX_VALUE);
+ testValidConstRange("byte", Byte.MIN_VALUE);
+ testInvalidConstRange("byte", Byte.MAX_VALUE + 1);
+ testInvalidConstRange("byte", Byte.MIN_VALUE - 1);
+
+ testValidConstRange("char", Character.MAX_VALUE);
+ testValidConstRange("char", Character.MIN_VALUE);
+ testInvalidConstRange("char", Character.MAX_VALUE + 1);
+ testInvalidConstRange("char", Character.MIN_VALUE - 1);
+
+ testValidConstRange("boolean", 0);
+ testValidConstRange("boolean", 1);
+ testInvalidConstRange("boolean", 2);
+ testInvalidConstRange("boolean", Integer.MIN_VALUE);
+ testInvalidConstRange("boolean", Integer.MAX_VALUE);
+ }
+
+ /**
+ * Tests that a constant value of the given {@code type} and initialized with an out-of-range
+ * {@code value} is rejected.
+ */
+ private static void testInvalidConstRange(String type, int value) throws Exception {
+ createConstantWithValue(type, value);
+ BadClassFile badClassFile = loadBadClass("Lib");
+ if (badClassFile == null) {
+ throw new AssertionError("did not see expected error");
+ }
+ JCDiagnostic diagnostic = (JCDiagnostic) badClassFile.getDiagnostic().getArgs()[1];
+ assertEquals("compiler.misc.bad.constant.range", diagnostic.getCode());
+ assertEquals(3, diagnostic.getArgs().length);
+ assertEquals(value, diagnostic.getArgs()[0]);
+ assertEquals("B", diagnostic.getArgs()[1].toString());
+ assertEquals(type, String.valueOf(diagnostic.getArgs()[2]));
+ }
+
+ /**
+ * Tests that a constant value of the given {@code type} and initialized with {@code value} is
+ * accepted.
+ */
+ private static void testValidConstRange(String type, int value) throws Exception {
+ createConstantWithValue(type, value);
+ BadClassFile badClassFile = loadBadClass("Lib");
+ if (badClassFile != null) {
+ throw new AssertionError("saw unexpected error", badClassFile);
+ }
+ }
+
+ /**
+ * Creates a class file containing a constant field with the given type and value, which may be
+ * outside the expected range.
+ */
+ private static void createConstantWithValue(String type, int value) throws Exception {
+ // Create a class with two constants, A and B. A is of type int and has value "actual";
+ // B is of type "type" and is initialized to that type's default value.
+ File lib = writeFile(classesdir, "Lib.java", String.format(
+ "class Lib { static final int A = %s; static final %s B = %s; }",
+ value, type, (type.equals("boolean") ? "false" : "0")));
+ compile("-d", classesdir.getPath(), lib.getPath());
+ File libClass = new File(classesdir, "Lib.class");
+ // Rewrite the class to only have field B of type "type" and with "value" (potentially
+ // out of range).
+ swapConstantValues(libClass);
+ }
+
+ /** Tests that a field of the given integral type with a constant string value is rejected. */
+ private static void testInvalidConstantType(String type) throws Exception {
+ // create a class file with field that has an invalid CONSTANT_String ConstantValue
+ File lib = writeFile(classesdir, "Lib.java", String.format(
+ "class Lib { static final String A = \"hello\"; static final %s CONST = %s; }",
+ type, type.equals("boolean") ? "false" : "0"));
+ compile("-d", classesdir.getPath(), lib.getPath());
+ File libClass = new File(classesdir, "Lib.class");
+ swapConstantValues(libClass);
+
+ BadClassFile badClassFile = loadBadClass("Lib");
+
+ JCDiagnostic diagnostic = (JCDiagnostic) badClassFile.getDiagnostic().getArgs()[1];
+ assertEquals("compiler.misc.bad.constant.value", diagnostic.getCode());
+ assertEquals(3, diagnostic.getArgs().length);
+ assertEquals("hello", diagnostic.getArgs()[0]);
+ assertEquals("CONST", diagnostic.getArgs()[1].toString());
+ assertEquals("Integer", diagnostic.getArgs()[2]);
+ }
+
+ private static BadClassFile loadBadClass(String className) {
+ // load the class, and save the thrown BadClassFile exception
+ JavaCompiler c = ToolProvider.getSystemJavaCompiler();
+ JavacTaskImpl task = (JavacTaskImpl) c.getTask(null, null, null,
+ Arrays.asList("-classpath", classesdir.getPath()), null, null);
+ Symtab syms = Symtab.instance(task.getContext());
+ task.ensureEntered();
+ BadClassFile badClassFile;
+ try {
+ com.sun.tools.javac.main.JavaCompiler.instance(task.getContext())
+ .resolveIdent(syms.unnamedModule, className).complete();
+ } catch (BadClassFile e) {
+ return e;
+ }
+ return null;
+ }
+
+ /**
+ * Given a class file with two constant fields A and B, replaces both with a single field with
+ * B's type and A's ConstantValue attribute.
+ */
+ private static void swapConstantValues(File file) throws Exception {
+ ClassFile cf = ClassFile.read(file);
+ Field a = cf.fields[0];
+ Field b = cf.fields[1];
+ Field[] fields = {
+ new Field(b.access_flags, b.name_index, b.descriptor, a.attributes),
+ };
+ cf = new ClassFile(cf.magic, Target.JDK1_7.minorVersion, Target.JDK1_7.majorVersion,
+ cf.constant_pool, cf.access_flags, cf.this_class, cf.super_class, cf.interfaces,
+ fields, cf.methods, cf.attributes);
+ new ClassWriter().write(cf, file);
+ }
+
+ static String compile(String... args) throws Exception {
+ System.err.println("compile: " + Arrays.asList(args));
+ StringWriter sw = new StringWriter();
+ PrintWriter pw = new PrintWriter(sw);
+ int rc = com.sun.tools.javac.Main.compile(args, pw);
+ pw.close();
+ String out = sw.toString();
+ if (out.length() > 0) {
+ System.err.println(out);
+ }
+ if (rc != 0) {
+ throw new AssertionError("compilation failed, rc=" + rc);
+ }
+ return out;
+ }
+
+ static File writeFile(File dir, String path, String body) throws IOException {
+ File f = new File(dir, path);
+ f.getParentFile().mkdirs();
+ FileWriter out = new FileWriter(f);
+ out.write(body);
+ out.close();
+ return f;
+ }
+
+ static void assertEquals(Object expected, Object actual) {
+ Assert.check(Objects.equals(expected, actual),
+ String.format("expected: %s, but was: %s", expected, actual));
+ }
+}
--- a/langtools/test/tools/javac/diags/examples.not-yet.txt Mon Jan 02 18:31:30 2017 -0800
+++ b/langtools/test/tools/javac/diags/examples.not-yet.txt Wed Dec 21 15:40:08 2016 -0800
@@ -43,6 +43,8 @@
compiler.misc.bad.class.signature # bad class file
compiler.misc.bad.const.pool.tag # bad class file
compiler.misc.bad.const.pool.tag.at # bad class file
+compiler.misc.bad.constant.range # bad class file
+compiler.misc.bad.constant.value # bad class file
compiler.misc.bad.enclosing.class # bad class file
compiler.misc.bad.enclosing.method # bad class file
compiler.misc.bad.runtime.invisible.param.annotations # bad class file