7127066: Class verifier accepts an invalid class file

Summary: For *store bytecodes, compare incoming, not outgoing, type state with exception handlers' stack maps.
Reviewed-by: acorn, dholmes

src/share/vm/classfile/verifier.cpp

@@ -655,6 +655,7 @@ void ClassVerifier::verify_method(methodHandle m, TRAPS) {
 
 
     bool this_uninit = false;  // Set to true when invokespecial <init> initialized 'this'
+    bool verified_exc_handlers = false;
 
     // Merge with the next instruction
     {
@@ -686,6 +687,18 @@ void ClassVerifier::verify_method(methodHandle m, TRAPS) {
         }
       }
 
+      // Look for possible jump target in exception handlers and see if it
+      // matches current_frame.  Do this check here for astore*, dstore*,
+      // fstore*, istore*, and lstore* opcodes because they can change the type
+      // state by adding a local.  JVM Spec says that the incoming type state
+      // should be used for this check.  So, do the check here before a possible
+      // local is added to the type state.
+      if (Bytecodes::is_store_into_local(opcode) && bci >= ex_min && bci < ex_max) {
+        verify_exception_handler_targets(
+          bci, this_uninit, &current_frame, &stackmap_table, CHECK_VERIFY(this));
+        verified_exc_handlers = true;
+      }
+
       switch (opcode) {
         case Bytecodes::_nop :
           no_control_flow = false; break;
@@ -1662,9 +1675,13 @@ void ClassVerifier::verify_method(methodHandle m, TRAPS) {
       }  // end switch
     }  // end Merge with the next instruction
 
-    // Look for possible jump target in exception handlers and see if it
-    // matches current_frame
-    if (bci >= ex_min && bci < ex_max) {
+    // Look for possible jump target in exception handlers and see if it matches
+    // current_frame.  Don't do this check if it has already been done (for
+    // ([a,d,f,i,l]store* opcodes).  This check cannot be done earlier because
+    // opcodes, such as invokespecial, may set the this_uninit flag.
+    assert(!(verified_exc_handlers && this_uninit),
+      "Exception handler targets got verified before this_uninit got set");
+    if (!verified_exc_handlers && bci >= ex_min && bci < ex_max) {
       verify_exception_handler_targets(
         bci, this_uninit, &current_frame, &stackmap_table, CHECK_VERIFY(this));
     }

src/share/vm/interpreter/bytecodes.hpp

 /*
- * Copyright (c) 1997, 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2015, 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
@@ -420,6 +420,7 @@ class Bytecodes: AllStatic {
   static bool        is_astore      (Code code)    { return (code == _astore || code == _astore_0 || code == _astore_1
                                                                              || code == _astore_2 || code == _astore_3); }
 
+  static bool        is_store_into_local(Code code){ return (_istore <= code && code <= _astore_3); }
   static bool        is_const       (Code code)    { return (_aconst_null <= code && code <= _ldc2_w); }
   static bool        is_zero_const  (Code code)    { return (code == _aconst_null || code == _iconst_0
                                                            || code == _fconst_0 || code == _dconst_0); }

test/runtime/stackMapCheck/BadMap.jasm

+ /*
+  * Copyright (c) 2015, 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.
+  *
+  */
+
+/*
+ * This class should throw VerifyError because the StackMap for bytecode index
+ * 45 (astore_2, line 123) is incorrect. The stack maps for bytecode indexes 45
+ * and 49 (astore, line 133) do not match because 45 does not supply enough
+ * locals to satisfy 49.
+ *
+ * The astore_2 bytecode at bytecode index 45 changes the type state,
+ * preventing the stackmap mismatch.  But, if the incoming type state is used,
+ * as required by JVM Spec 8, then the verifier will detected the stackmap
+ * mismatch, and throw VerifyError.
+ */
+
+super public class BadMap
+    version 51:0
+{
+
+
+public Method "<init>":"()V"
+    stack 1 locals 1
+{
+        aload_0;
+        invokespecial    Method java/lang/Object."<init>":"()V";
+        return;
+}
+
+public static Method main:"([Ljava/lang/String;)V"
+    throws java/lang/Throwable
+    stack 0 locals 1
+{
+        return;
+}
+
+public static Method foo:"()V"
+    stack 3 locals 5
+{
+        iconst_0;
+        ifne    L5;
+        nop;
+        try t7;
+    L5:    stack_frame_type full;
+        aconst_null;
+        dup;
+        astore_0;
+        astore_1;
+        try t0;
+        aconst_null;
+        astore_0;
+        endtry t0;
+        goto    L19;
+        catch t0 java/io/IOException;
+        stack_frame_type full;
+        locals_map class java/lang/Object, null;
+        stack_map class java/io/IOException;
+        astore_2;
+        aconst_null;
+        dup;
+        astore_1;
+        astore_0;
+        try t1;
+    L19:    stack_frame_type full;
+        locals_map class java/lang/Object, class java/lang/Object;
+        aconst_null;
+        astore_2;
+        endtry t1;
+        aload_1;
+        ifnonnull    L37;
+        nop;
+        goto    L37;
+        catch t1 #0;
+        catch t2 #0;
+        try t2;
+        stack_frame_type full;
+        locals_map class java/lang/Object, class java/lang/Object;
+        stack_map class java/lang/Throwable;
+        astore_3;
+        endtry t2;
+        aload_1;
+        ifnonnull    L35;
+        nop;
+    L35:    stack_frame_type full;
+        locals_map class java/lang/Object, class java/lang/Object, bogus, class java/lang/Throwable;
+        aload_3;
+        athrow;
+        try t3, t4;
+    L37:    stack_frame_type full;
+        locals_map class java/lang/Object, class java/lang/Object, class java/lang/Object;
+        aload_1;
+        ifnonnull    L42;
+        nop;
+        endtry t3, t4;
+    L42:    stack_frame_type full;
+        locals_map class java/lang/Object, class java/lang/Object, class java/lang/Object;
+        goto    L54;
+        catch t3 java/lang/Exception;
+        try t5;
+        stack_frame_type full;
+        locals_map class java/lang/Object, class java/lang/Object;
+        stack_map class java/lang/Exception;
+        astore_2;   // astore_2, at bci 45, that changes the type state.
+        endtry t5;
+        goto    L54;
+        catch t4 #0;
+        catch t5 #0;
+        catch t6 #0;
+        try t6;
+        stack_frame_type full;
+        locals_map class java/lang/Object, class java/lang/Object, class java/lang/Object;
+        stack_map class java/lang/Throwable;
+        astore    4;
+        endtry t6;
+        aload    4;
+        athrow;
+    L54:    stack_frame_type full;
+        locals_map class java/lang/Object, class java/lang/Object, class java/lang/Object;
+        goto    L57;
+    L57:    stack_frame_type full;
+        locals_map class java/lang/Object, class java/lang/Object, class java/lang/Object;
+        nop;
+        endtry t7;
+        return;
+        catch t7 #0;
+        stack_frame_type full;
+        stack_map class java/lang/Throwable;
+        nop;
+        athrow;
+}
+
+} // end Class BadMap

test/runtime/stackMapCheck/BadMapDstore.jasm

+ /*
+  * Copyright (c) 2015, 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.
+  *
+  */
+
+/*
+ * This class should throw VerifyError because the StackMap for bytecode index
+ * 9 (dstore_2, line 60) is incorrect. The stack maps for bytecode indexes 9
+ * and 18 (astore_2, line 70) do not match because 9 does not supply enough
+ * locals to satisfy 18.
+ *
+ * The dstore_2 bytecode at bytecode index 9 changes the type state,
+ * preventing the stackmap mismatch.  But, if the incoming type state is used,
+ * as required by JVM Spec 8, then the verifier will detected the stackmap
+ * mismatch, and throw VerifyError.
+ */
+
+super public class BadMapDstore
+    version 51:0
+{
+
+Field blah:I;
+
+public Method "<init>":"()V"
+    stack 1 locals 1
+{
+        aload_0;
+        invokespecial    Method java/lang/Object."<init>":"()V";
+        return;
+}
+
+public static Method main:"([Ljava/lang/String;)V"
+    stack 4 locals 4
+{
+        new    class BadMapDstore;
+        dup;
+        invokespecial    Method "<init>":"()V";
+        astore_1;
+        dconst_1;
+        try t0;
+        dstore_2;
+        aload_1;
+        iconst_5;
+        putfield    Field blah:"I";
+        endtry t0;
+        goto    L22;
+        catch t0 java/lang/Throwable;
+        stack_frame_type full;
+        locals_map class "[Ljava/lang/String;", class BadMapDstore, double;
+        stack_map class java/lang/Throwable;
+        astore_2;
+        aload_1;
+        dconst_0;
+        dstore_2;
+        pop;
+    L22:    stack_frame_type same;
+        return;
+}
+
+} // end Class BadMapDstore

test/runtime/stackMapCheck/BadMapIstore.jasm

+ /*
+  * Copyright (c) 2015, 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.
+  *
+  */
+
+/*
+ * This class should throw VerifyError because the StackMap for bytecode index
+ * 9 (istore_2, line 60) is incorrect. The stack maps for bytecode indexes 9
+ * and 18 (astore_2, line 70) do not match because 9 does not supply enough
+ * locals to satisfy 18.
+ *
+ * The istore_2 bytecode at bytecode index 9 changes the type state,
+ * preventing the stackmap mismatch.  But, if the incoming type state is used,
+ * as required by JVM Spec 8, then the verifier will detected the stackmap
+ * mismatch, and throw VerifyError.
+ */
+
+super public class BadMapIstore
+    version 51:0
+{
+
+Field blah:I;
+
+public Method "<init>":"()V"
+    stack 1 locals 1
+{
+        aload_0;
+        invokespecial    Method java/lang/Object."<init>":"()V";
+        return;
+}
+
+public static Method main:"([Ljava/lang/String;)V"
+    stack 2 locals 3
+{
+        new    class BadMapIstore;
+        dup;
+        invokespecial    Method "<init>":"()V";
+        astore_1;
+        iconst_2;
+        try t0;
+        istore_2;
+        aload_1;
+        iconst_5;
+        putfield    Field blah:"I";
+        endtry t0;
+        goto    L22;
+        catch t0 java/lang/Throwable;
+        stack_frame_type full;
+        locals_map class "[Ljava/lang/String;", class BadMapIstore, int;
+        stack_map class java/lang/Throwable;
+        astore_2;
+        aload_1;
+        iconst_4;
+        istore_2;
+        pop;
+    L22:    stack_frame_type same;
+        return;
+}
+
+} // end Class BadMapIstore

test/runtime/stackMapCheck/StackMapCheck.java

+ /*
+  * Copyright (c) 2015, 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.
+  *
+  */
+
+/*
+ * @test
+ * @bug 7127066
+ * @summary Class verifier accepts an invalid class file
+ * @compile BadMap.jasm
+ * @compile BadMapDstore.jasm
+ * @compile BadMapIstore.jasm
+ * @run main/othervm -Xverify:all StackMapCheck
+ */
+
+public class StackMapCheck {
+    public static void main(String args[]) throws Throwable {
+
+        System.out.println("Regression test for bug 7127066");
+        try {
+            Class newClass = Class.forName("BadMap");
+            throw new RuntimeException(
+                "StackMapCheck failed, BadMap did not throw VerifyError");
+        } catch (java.lang.VerifyError e) {
+            System.out.println("BadMap passed, VerifyError was thrown");
+        }
+
+        try {
+            Class newClass = Class.forName("BadMapDstore");
+            throw new RuntimeException(
+                "StackMapCheck failed, BadMapDstore did not throw VerifyError");
+        } catch (java.lang.VerifyError e) {
+            System.out.println("BadMapDstore passed, VerifyError was thrown");
+        }
+
+        try {
+            Class newClass = Class.forName("BadMapIstore");
+            throw new RuntimeException(
+                "StackMapCheck failed, BadMapIstore did not throw VerifyError");
+        } catch (java.lang.VerifyError e) {
+            System.out.println("BadMapIstore passed, VerifyError was thrown");
+        }
+    }
+}