8192936: RI does not follow the JVMTI RedefineClasses spec that is too strict in the definition
authorcoleenp
Fri, 19 Apr 2019 21:49:54 -0700
changeset 54585 3452d108d06d
parent 54584 2de1c3fa3e7d
child 54586 0ab35668b4f4
child 54592 4b4c8c11358f
child 57335 e90b4960bcd6
8192936: RI does not follow the JVMTI RedefineClasses spec that is too strict in the definition Summary: Introduce new flag fo compatibility: -XX:AllowRedefinitionToAddOrDeleteMethods Reviewed-by: jcbeyler, sspitsyn
src/hotspot/share/prims/jvmtiRedefineClasses.cpp
src/hotspot/share/runtime/arguments.cpp
src/hotspot/share/runtime/globals.hpp
test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/RedefineAddLambdaExpression.java
test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/RedefineDeleteJmethod.java
test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/RedefineSubtractLambdaExpression.java
test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/TestAddDeleteMethods.java
test/jdk/com/sun/jdi/RedefineAddPrivateMethod.java
test/jdk/com/sun/jdi/lib/jdb/Debuggee.java
test/jdk/com/sun/jdi/lib/jdb/JdbTest.java
test/jdk/java/lang/instrument/RedefineAddDeleteMethod/DeleteMethodHandle/MethodHandleDeletedMethod.java
test/jdk/java/lang/instrument/RedefineMethodAddInvoke.sh
test/jdk/java/lang/instrument/RedefineMethodDelInvoke.sh
test/jdk/java/lang/instrument/RedefineMethodInBacktrace.sh
--- a/src/hotspot/share/prims/jvmtiRedefineClasses.cpp	Fri Apr 19 10:59:09 2019 -0300
+++ b/src/hotspot/share/prims/jvmtiRedefineClasses.cpp	Fri Apr 19 21:49:54 2019 -0700
@@ -787,6 +787,12 @@
   return JVMTI_ERROR_NONE;
 }
 
+static bool can_add_or_delete(Method* m) {
+      // Compatibility mode
+  return (AllowRedefinitionToAddDeleteMethods &&
+          (m->is_private() && (m->is_static() || m->is_final())));
+}
+
 jvmtiError VM_RedefineClasses::compare_and_normalize_class_versions(
              InstanceKlass* the_class,
              InstanceKlass* scratch_class) {
@@ -992,12 +998,7 @@
       break;
     case added:
       // method added, see if it is OK
-      new_flags = (jushort) k_new_method->access_flags().get_flags();
-      if ((new_flags & JVM_ACC_PRIVATE) == 0
-           // hack: private should be treated as final, but alas
-          || (new_flags & (JVM_ACC_FINAL|JVM_ACC_STATIC)) == 0
-         ) {
-        // new methods must be private
+      if (!can_add_or_delete(k_new_method)) {
         return JVMTI_ERROR_UNSUPPORTED_REDEFINITION_METHOD_ADDED;
       }
       {
@@ -1026,12 +1027,7 @@
       break;
     case deleted:
       // method deleted, see if it is OK
-      old_flags = (jushort) k_old_method->access_flags().get_flags();
-      if ((old_flags & JVM_ACC_PRIVATE) == 0
-           // hack: private should be treated as final, but alas
-          || (old_flags & (JVM_ACC_FINAL|JVM_ACC_STATIC)) == 0
-         ) {
-        // deleted methods must be private
+      if (!can_add_or_delete(k_old_method)) {
         return JVMTI_ERROR_UNSUPPORTED_REDEFINITION_METHOD_DELETED;
       }
       log_trace(redefine, class, normalize)
--- a/src/hotspot/share/runtime/arguments.cpp	Fri Apr 19 10:59:09 2019 -0300
+++ b/src/hotspot/share/runtime/arguments.cpp	Fri Apr 19 21:49:54 2019 -0700
@@ -540,6 +540,7 @@
   { "FailOverToOldVerifier",        JDK_Version::jdk(13), JDK_Version::jdk(14), JDK_Version::undefined() },
   { "AllowJNIEnvProxy",             JDK_Version::jdk(13), JDK_Version::jdk(14), JDK_Version::jdk(15) },
   { "ThreadLocalHandshakes",        JDK_Version::jdk(13), JDK_Version::jdk(14), JDK_Version::jdk(15) },
+  { "AllowRedefinitionToAddDeleteMethods", JDK_Version::jdk(13), JDK_Version::undefined(), JDK_Version::undefined() },
 
   // --- Deprecated alias flags (see also aliased_jvm_flags) - sorted by obsolete_in then expired_in:
   { "DefaultMaxRAMFraction",        JDK_Version::jdk(8),  JDK_Version::undefined(), JDK_Version::undefined() },
--- a/src/hotspot/share/runtime/globals.hpp	Fri Apr 19 10:59:09 2019 -0300
+++ b/src/hotspot/share/runtime/globals.hpp	Fri Apr 19 21:49:54 2019 -0700
@@ -978,6 +978,10 @@
   product(bool, VerifyMergedCPBytecodes, true,                              \
           "Verify bytecodes after RedefineClasses constant pool merging")   \
                                                                             \
+  product(bool, AllowRedefinitionToAddDeleteMethods, false,                 \
+          "Allow redefinition to add and delete private static or "         \
+          "final methods for compatibility with old releases")              \
+                                                                            \
   develop(bool, TraceBytecodes, false,                                      \
           "Trace bytecode execution")                                       \
                                                                             \
--- a/test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/RedefineAddLambdaExpression.java	Fri Apr 19 10:59:09 2019 -0300
+++ b/test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/RedefineAddLambdaExpression.java	Fri Apr 19 21:49:54 2019 -0700
@@ -31,7 +31,7 @@
  *          java.instrument
  *          jdk.jartool/sun.tools.jar
  * @run main RedefineClassHelper
- * @run main/othervm -javaagent:redefineagent.jar -Xlog:redefine+class*=trace RedefineAddLambdaExpression
+ * @run main/othervm -javaagent:redefineagent.jar -XX:+AllowRedefinitionToAddDeleteMethods -Xlog:redefine+class*=trace RedefineAddLambdaExpression
  */
 
 interface MathOperation {
--- a/test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/RedefineDeleteJmethod.java	Fri Apr 19 10:59:09 2019 -0300
+++ b/test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/RedefineDeleteJmethod.java	Fri Apr 19 21:49:54 2019 -0700
@@ -31,7 +31,7 @@
  *          java.instrument
  *          jdk.jartool/sun.tools.jar
  * @run main RedefineClassHelper
- * @run main/native/othervm -javaagent:redefineagent.jar -Xlog:redefine+class*=trace RedefineDeleteJmethod
+ * @run main/native/othervm -javaagent:redefineagent.jar -XX:+AllowRedefinitionToAddDeleteMethods -Xlog:redefine+class*=trace RedefineDeleteJmethod
  */
 
 class B {
--- a/test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/RedefineSubtractLambdaExpression.java	Fri Apr 19 10:59:09 2019 -0300
+++ b/test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/RedefineSubtractLambdaExpression.java	Fri Apr 19 21:49:54 2019 -0700
@@ -31,7 +31,7 @@
  *          java.instrument
  *          jdk.jartool/sun.tools.jar
  * @run main RedefineClassHelper
- * @run main/othervm -javaagent:redefineagent.jar -Xlog:redefine+class*=trace RedefineSubtractLambdaExpression
+ * @run main/othervm -javaagent:redefineagent.jar -XX:+AllowRedefinitionToAddDeleteMethods -Xlog:redefine+class*=trace RedefineSubtractLambdaExpression
  */
 
 interface MathOperation {
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/TestAddDeleteMethods.java	Fri Apr 19 21:49:54 2019 -0700
@@ -0,0 +1,138 @@
+/*
+ * Copyright (c) 2019, 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 8192936
+ * @summary RI does not follow the JVMTI RedefineClasses spec; need to disallow adding and deleting methods
+ * @library /test/lib
+ * @modules java.base/jdk.internal.misc
+ * @modules java.compiler
+ *          java.instrument
+ *          jdk.jartool/sun.tools.jar
+ * @run main RedefineClassHelper
+ * @run main/othervm -javaagent:redefineagent.jar TestAddDeleteMethods
+ */
+
+import static jdk.test.lib.Asserts.assertEquals;
+
+// package access top-level class to avoid problem with RedefineClassHelper
+// and nested types.
+class A {
+    private static void foo()       { System.out.println("OLD foo called"); }
+    private final  void finalFoo()  { System.out.println("OLD finalFoo called"); }
+    public         void publicFoo() { foo(); finalFoo(); }
+}
+
+public class TestAddDeleteMethods {
+    static A a;
+
+    public static String newA =
+        "class A {" +
+            "private static void foo()       { System.out.println(\"NEW foo called\"); }" +
+            "private final  void finalFoo()  { System.out.println(\"NEW finalFoo called\"); }" +
+            "public         void publicFoo() { foo(); finalFoo(); }" +
+        "}";
+
+    public static String newAddBar =
+        "class A {" +
+            "private        void bar()       { System.out.println(\"NEW bar called\"); }" +
+            "private static void foo()       { System.out.println(\"NEW foo called\"); }" +
+            "private final  void finalFoo()  { System.out.println(\"NEW finalFoo called\"); }" +
+            "public         void publicFoo() { foo(); bar(); finalFoo(); }" +
+        "}";
+
+    public static String newAddFinalBar =
+        "class A {" +
+            "private final  void bar()       { System.out.println(\"NEW bar called\"); }" +
+            "private static void foo()       { System.out.println(\"NEW foo called\"); }" +
+            "private final  void finalFoo()  { System.out.println(\"NEW finalFoo called\"); }" +
+            "public         void publicFoo() { foo(); bar(); finalFoo(); }" +
+        "}";
+
+    public static String newAddPublicBar =
+        "class A {" +
+            "public         void bar()       { System.out.println(\"NEW public bar called\"); }" +
+            "private static void foo()       { System.out.println(\"NEW foo called\"); }" +
+            "private final  void finalFoo()  { System.out.println(\"NEW finalFoo called\"); }" +
+            "public         void publicFoo() { foo(); bar(); finalFoo(); }" +
+        "}";
+
+    public static String newDeleteFoo =
+        "class A {" +
+            "private final  void finalFoo()  { System.out.println(\"NEW finalFoo called\"); }" +
+            "public         void publicFoo() { finalFoo(); }" +
+        "}";
+
+    public static String newDeleteFinalFoo =
+        "class A {" +
+            "private static void foo()       { System.out.println(\"NEW foo called\"); }" +
+            "public         void publicFoo() { foo(); }" +
+        "}";
+
+    public static String newDeletePublicFoo =
+        "class A {" +
+            "private static void foo()       { System.out.println(\"NEW foo called\"); }" +
+            "private final  void finalFoo()  { System.out.println(\"NEW finalFoo called\"); }" +
+        "}";
+
+    static private final String ExpMsgPrefix = "attempted to ";
+    static private final String ExpMsgPostfix = " a method";
+
+    public static void test(String newBytes, String expSuffix) throws Exception {
+        String expectedMessage = ExpMsgPrefix + expSuffix + ExpMsgPostfix;
+
+        try {
+            RedefineClassHelper.redefineClass(A.class, newBytes);
+            a.publicFoo();
+            throw new RuntimeException("Failed, expected UOE");
+        } catch (UnsupportedOperationException uoe) {
+            String message = uoe.getMessage();
+            System.out.println("Got expected UOE " + message);
+            if (!message.endsWith(expectedMessage)) {
+                throw new RuntimeException("Expected UOE error message to end with: " + expectedMessage);
+            }
+        }
+    }
+
+    static {
+        a = new A();
+    }
+
+    public static void main(String[] args) throws Exception {
+
+        a.publicFoo();
+
+        // Should pass because this only changes bytes of methods.
+        RedefineClassHelper.redefineClass(A.class, newA);
+        a.publicFoo();
+
+        // Add private static bar
+        test(newAddBar,          "add");
+        test(newAddFinalBar,     "add");
+        test(newAddPublicBar,    "add");
+        test(newDeleteFoo,       "delete");
+        test(newDeleteFinalFoo,  "delete");
+        test(newDeletePublicFoo, "delete");
+    }
+}
--- a/test/jdk/com/sun/jdi/RedefineAddPrivateMethod.java	Fri Apr 19 10:59:09 2019 -0300
+++ b/test/jdk/com/sun/jdi/RedefineAddPrivateMethod.java	Fri Apr 19 21:49:54 2019 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2019, 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
@@ -46,8 +46,12 @@
 }
 
 public class RedefineAddPrivateMethod extends JdbTest {
+    static private final String ALLOW_ADD_DELETE_OPTION = "-XX:+AllowRedefinitionToAddDeleteMethods";
+
     public static void main(String argv[]) {
-        new RedefineAddPrivateMethod().run();
+        RedefineAddPrivateMethod test = new RedefineAddPrivateMethod();
+        test.launchOptions.addVMOptions(ALLOW_ADD_DELETE_OPTION);
+        test.run();
     }
 
     private RedefineAddPrivateMethod() {
--- a/test/jdk/com/sun/jdi/lib/jdb/Debuggee.java	Fri Apr 19 10:59:09 2019 -0300
+++ b/test/jdk/com/sun/jdi/lib/jdb/Debuggee.java	Fri Apr 19 21:49:54 2019 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2019, 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
@@ -65,6 +65,7 @@
     public static class Launcher {
         private final String mainClass;
         private final List<String> options = new LinkedList<>();
+        private String vmOptions = null;
         private String transport = "dt_socket";
         private String address = null;
         private boolean suspended = true;
@@ -81,6 +82,10 @@
             this.options.addAll(options);
             return this;
         }
+        public Launcher addVMOptions(String vmOptions) {
+            this.vmOptions = vmOptions;
+            return this;
+        }
         // default is "dt_socket"
         public Launcher setTransport(String value) {
             transport = value;
@@ -104,6 +109,9 @@
 
         public ProcessBuilder prepare() {
             List<String> debuggeeArgs = new LinkedList<>();
+            if (vmOptions != null) {
+                debuggeeArgs.add(vmOptions);
+            }
             debuggeeArgs.add("-agentlib:jdwp=transport=" + transport
                     + (address == null ? "" : ",address=" + address)
                     + ",server=y,suspend=" + (suspended ? "y" : "n"));
--- a/test/jdk/com/sun/jdi/lib/jdb/JdbTest.java	Fri Apr 19 10:59:09 2019 -0300
+++ b/test/jdk/com/sun/jdi/lib/jdb/JdbTest.java	Fri Apr 19 21:49:54 2019 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2019, 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
@@ -40,6 +40,7 @@
         public final String debuggeeClass;
         public final List<String> debuggeeOptions = new LinkedList<>();
         public String sourceFilename;
+        public String vmOptions = null;
 
         public LaunchOptions(String debuggeeClass) {
             this.debuggeeClass = debuggeeClass;
@@ -56,6 +57,10 @@
             sourceFilename = name;
             return this;
         }
+        public LaunchOptions addVMOptions(String vmOptions) {
+            this.vmOptions = vmOptions;
+            return this;
+        }
     }
 
     public JdbTest(LaunchOptions launchOptions) {
@@ -72,7 +77,7 @@
 
     protected Jdb jdb;
     protected Debuggee debuggee;
-    private final LaunchOptions launchOptions;
+    protected LaunchOptions launchOptions;
 
     // returns the whole jdb output as a string
     public String getJdbOutput() {
@@ -108,6 +113,7 @@
         // launch debuggee
         debuggee = Debuggee.launcher(launchOptions.debuggeeClass)
                 .addOptions(launchOptions.debuggeeOptions)
+                .addVMOptions(launchOptions.vmOptions)
                 .launch();
 
         // launch jdb
--- a/test/jdk/java/lang/instrument/RedefineAddDeleteMethod/DeleteMethodHandle/MethodHandleDeletedMethod.java	Fri Apr 19 10:59:09 2019 -0300
+++ b/test/jdk/java/lang/instrument/RedefineAddDeleteMethod/DeleteMethodHandle/MethodHandleDeletedMethod.java	Fri Apr 19 21:49:54 2019 -0700
@@ -33,7 +33,7 @@
  * @compile ../../NamedBuffer.java
  * @compile redef/Xost.java
  * @run main RedefineClassHelper
- * @run main/othervm -javaagent:redefineagent.jar -Xlog:redefine+class+update*=debug,membername+table=debug MethodHandleDeletedMethod
+ * @run main/othervm -XX:+AllowRedefinitionToAddDeleteMethods -javaagent:redefineagent.jar -Xlog:redefine+class+update*=debug,membername+table=debug MethodHandleDeletedMethod
  */
 
 import java.io.File;
--- a/test/jdk/java/lang/instrument/RedefineMethodAddInvoke.sh	Fri Apr 19 10:59:09 2019 -0300
+++ b/test/jdk/java/lang/instrument/RedefineMethodAddInvoke.sh	Fri Apr 19 21:49:54 2019 -0700
@@ -1,5 +1,5 @@
 #
-# Copyright (c) 2008, 2015, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2008, 2019, 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
@@ -71,6 +71,7 @@
 mv RedefineMethodAddInvokeTarget.class RedefineMethodAddInvokeTarget_2.class
 
 "${JAVA}" ${TESTVMOPTS} -javaagent:RedefineMethodAddInvokeAgent.jar \
+    -XX:+AllowRedefinitionToAddDeleteMethods \
     -classpath "${TESTCLASSES}" RedefineMethodAddInvokeApp > output.log 2>&1
 cat output.log
 
--- a/test/jdk/java/lang/instrument/RedefineMethodDelInvoke.sh	Fri Apr 19 10:59:09 2019 -0300
+++ b/test/jdk/java/lang/instrument/RedefineMethodDelInvoke.sh	Fri Apr 19 21:49:54 2019 -0700
@@ -1,5 +1,5 @@
 #
-# Copyright (c) 2014, 2015, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2014, 2019, 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
@@ -72,6 +72,7 @@
 mv RedefineMethodDelInvokeTarget.class RedefineMethodDelInvokeTarget_2.class
 
 "${JAVA}" ${TESTVMOPTS} -javaagent:RedefineMethodDelInvokeAgent.jar \
+    -XX:+AllowRedefinitionToAddDeleteMethods \
     -classpath "${TESTCLASSES}" RedefineMethodDelInvokeApp > output.log 2>&1
 
 result=$?
--- a/test/jdk/java/lang/instrument/RedefineMethodInBacktrace.sh	Fri Apr 19 10:59:09 2019 -0300
+++ b/test/jdk/java/lang/instrument/RedefineMethodInBacktrace.sh	Fri Apr 19 21:49:54 2019 -0700
@@ -1,5 +1,5 @@
 #
-# Copyright (c) 2013, 2016, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2013, 2019, 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
@@ -69,6 +69,7 @@
 "${JAVAC}" ${TESTJAVACOPTS} ${TESTTOOLVMOPTS} -d . RedefineMethodInBacktraceTargetB.java
 
 "${JAVA}" ${TESTVMOPTS} -javaagent:RedefineMethodInBacktraceAgent.jar \
+    -XX:+AllowRedefinitionToAddDeleteMethods \
     -classpath "${TESTCLASSES}" RedefineMethodInBacktraceApp > output.log 2>&1
 RUN_RESULT=$?