8171132: Improve class reading of invalid or out-of-range ConstantValue attributes

Reviewed-by: mcimadamore

langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/code/TypeTag.java

@@ -241,4 +241,21 @@ public enum TypeTag {
         }
     }
 
+    /** 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();
+        }
+    }
 }

langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java

@@ -2050,19 +2050,12 @@ public class Types {
             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)
-                    return true;
-                break;
             case SHORT:
-                if (Short.MIN_VALUE <= value && value <= Short.MAX_VALUE)
+            case INT:
+                if (s.getTag().checkRange(value))
                     return true;
                 break;
-            case INT:
-                return true;
             case CLASS:
                 switch (unboxedType(s).getTag()) {
                 case BYTE:

langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java

@@ -1028,8 +1028,45 @@ public class ClassReader {
                 protected void read(Symbol sym, int attrLen) {
                     Object v = readPool(nextChar());
                     // Ignore ConstantValue attribute if field not final.
-                    if ((sym.flags() & FINAL) != 0)
+                    if ((sym.flags() & FINAL) == 0) {
-                        ((VarSymbol) sym).setData(v);
+                        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());
+                    }
                 }
             },
 

langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties

@@ -1913,6 +1913,14 @@ compiler.misc.module.info.invalid.super.class=\
 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

langtools/test/tools/javac/classreader/8171132/BadConstantValue.java

@@ -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));
+    }
+}

langtools/test/tools/javac/diags/examples.not-yet.txt

@@ -43,6 +43,8 @@ compiler.err.unsupported.cross.fp.lit                   # Scanner: host system d
 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