8038636: speculative traps break when classes are redefined

Summary: remove speculative traps that point to methods that are redefined
Reviewed-by: kvn, twisti

src/share/vm/oops/instanceKlass.cpp

@@ -3682,6 +3682,10 @@ static void purge_previous_versions_internal(InstanceKlass* ik, int emcp_method_
               ("purge: %s(%s): prev method @%d in version @%d is alive",
               method->name()->as_C_string(),
               method->signature()->as_C_string(), j, i));
+            if (method->method_data() != NULL) {
+              // Clean out any weak method links
+              method->method_data()->clean_weak_method_links();
+            }
           }
         }
       }
@@ -3691,6 +3695,14 @@ static void purge_previous_versions_internal(InstanceKlass* ik, int emcp_method_
       ("purge: previous version stats: live=%d, deleted=%d", live_count,
       deleted_count));
   }
+
+  Array<Method*>* methods = ik->methods();
+  int num_methods = methods->length();
+  for (int index2 = 0; index2 < num_methods; ++index2) {
+    if (methods->at(index2)->method_data() != NULL) {
+      methods->at(index2)->method_data()->clean_weak_method_links();
+    }
+  }
 }
 
 // External interface for use during class unloading.

src/share/vm/oops/methodData.cpp

@@ -1559,9 +1559,35 @@ void MethodData::clean_extra_data_helper(DataLayout* dp, int shift, bool reset)
   }
 }
 
-// Remove SpeculativeTrapData entries that reference an unloaded
-// method
-void MethodData::clean_extra_data(BoolObjectClosure* is_alive) {
+class CleanExtraDataClosure : public StackObj {
+public:
+  virtual bool is_live(Method* m) = 0;
+};
+
+// Check for entries that reference an unloaded method
+class CleanExtraDataKlassClosure : public CleanExtraDataClosure {
+private:
+  BoolObjectClosure* _is_alive;
+public:
+  CleanExtraDataKlassClosure(BoolObjectClosure* is_alive) : _is_alive(is_alive) {}
+  bool is_live(Method* m) {
+    return m->method_holder()->is_loader_alive(_is_alive);
+  }
+};
+
+// Check for entries that reference a redefined method
+class CleanExtraDataMethodClosure : public CleanExtraDataClosure {
+public:
+  CleanExtraDataMethodClosure() {}
+  bool is_live(Method* m) {
+    return m->on_stack();
+  }
+};
+
+
+// Remove SpeculativeTrapData entries that reference an unloaded or
+// redefined method
+void MethodData::clean_extra_data(CleanExtraDataClosure* cl) {
   DataLayout* dp  = extra_data_base();
   DataLayout* end = extra_data_limit();
 
@@ -1572,7 +1598,7 @@ void MethodData::clean_extra_data(BoolObjectClosure* is_alive) {
       SpeculativeTrapData* data = new SpeculativeTrapData(dp);
       Method* m = data->method();
       assert(m != NULL, "should have a method");
-      if (!m->method_holder()->is_loader_alive(is_alive)) {
+      if (!cl->is_live(m)) {
         // "shift" accumulates the number of cells for dead
         // SpeculativeTrapData entries that have been seen so
         // far. Following entries must be shifted left by that many
@@ -1603,9 +1629,9 @@ void MethodData::clean_extra_data(BoolObjectClosure* is_alive) {
   }
 }
 
-// Verify there's no unloaded method referenced by a
+// Verify there's no unloaded or redefined method referenced by a
 // SpeculativeTrapData entry
-void MethodData::verify_extra_data_clean(BoolObjectClosure* is_alive) {
+void MethodData::verify_extra_data_clean(CleanExtraDataClosure* cl) {
 #ifdef ASSERT
   DataLayout* dp  = extra_data_base();
   DataLayout* end = extra_data_limit();
@@ -1615,7 +1641,7 @@ void MethodData::verify_extra_data_clean(BoolObjectClosure* is_alive) {
     case DataLayout::speculative_trap_data_tag: {
       SpeculativeTrapData* data = new SpeculativeTrapData(dp);
       Method* m = data->method();
-      assert(m != NULL && m->method_holder()->is_loader_alive(is_alive), "Method should exist");
+      assert(m != NULL && cl->is_live(m), "Method should exist");
       break;
     }
     case DataLayout::bit_data_tag:
@@ -1641,6 +1667,19 @@ void MethodData::clean_method_data(BoolObjectClosure* is_alive) {
     parameters->clean_weak_klass_links(is_alive);
   }
 
-  clean_extra_data(is_alive);
-  verify_extra_data_clean(is_alive);
+  CleanExtraDataKlassClosure cl(is_alive);
+  clean_extra_data(&cl);
+  verify_extra_data_clean(&cl);
+}
+
+void MethodData::clean_weak_method_links() {
+  for (ProfileData* data = first_data();
+       is_valid(data);
+       data = next_data(data)) {
+    data->clean_weak_method_links();
+  }
+
+  CleanExtraDataMethodClosure cl;
+  clean_extra_data(&cl);
+  verify_extra_data_clean(&cl);
 }

src/share/vm/oops/methodData.hpp

@@ -251,6 +251,9 @@ public:
 
   // GC support
   void clean_weak_klass_links(BoolObjectClosure* cl);
+
+  // Redefinition support
+  void clean_weak_method_links();
 };
 
 
@@ -508,6 +511,9 @@ public:
   // GC support
   virtual void clean_weak_klass_links(BoolObjectClosure* is_alive_closure) {}
 
+  // Redefinition support
+  virtual void clean_weak_method_links() {}
+
   // CI translation: ProfileData can represent both MethodDataOop data
   // as well as CIMethodData data. This function is provided for translating
   // an oop in a ProfileData to the ci equivalent. Generally speaking,
@@ -2030,6 +2036,7 @@ public:
 //
 
 CC_INTERP_ONLY(class BytecodeInterpreter;)
+class CleanExtraDataClosure;
 
 class MethodData : public Metadata {
   friend class VMStructs;
@@ -2183,9 +2190,9 @@ private:
   static bool profile_parameters_jsr292_only();
   static bool profile_all_parameters();
 
-  void clean_extra_data(BoolObjectClosure* is_alive);
+  void clean_extra_data(CleanExtraDataClosure* cl);
   void clean_extra_data_helper(DataLayout* dp, int shift, bool reset = false);
-  void verify_extra_data_clean(BoolObjectClosure* is_alive);
+  void verify_extra_data_clean(CleanExtraDataClosure* cl);
 
 public:
   static int header_size() {
@@ -2477,6 +2484,8 @@ public:
   static bool profile_return_jsr292_only();
 
   void clean_method_data(BoolObjectClosure* is_alive);
+
+  void clean_weak_method_links();
 };
 
 #endif // SHARE_VM_OOPS_METHODDATAOOP_HPP

test/compiler/profiling/spectrapredefineclass/Agent.java

+/*
+ * Copyright (c) 2014, 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.security.*;
+import java.lang.instrument.*;
+import java.lang.reflect.*;
+import java.lang.management.ManagementFactory;
+import com.sun.tools.attach.VirtualMachine;
+
+class A {
+    void m() {
+    }
+}
+
+class B extends A {
+    void m() {
+    }
+}
+
+class C extends A {
+    void m() {
+    }
+}
+
+class Test {
+
+    static public void m() throws Exception {
+        for (int i = 0; i < 20000; i++) {
+            m1(a);
+        }
+        for (int i = 0; i < 4; i++) {
+            m1(b);
+        }
+    }
+
+    static boolean m1(A a) {
+        boolean res =  Agent.m2(a);
+        return res;
+    }
+
+    static public A a = new A();
+    static public B b = new B();
+    static public C c = new C();
+}
+
+public class Agent implements ClassFileTransformer {
+
+
+    static class MemoryChunk {
+        MemoryChunk other;
+        long[] array;
+        MemoryChunk(MemoryChunk other) {
+            other = other;
+            array = new long[1024 * 1024 * 1024];
+        }
+    }
+
+    static public boolean m2(A a) {
+        boolean res = false;
+        if (a.getClass() == B.class) {
+            a.m();
+        } else {
+            res = true;
+        }
+        return res;
+    }
+
+    static public void main(String[] args) throws Exception {
+        // Create speculative trap entries
+        Test.m();
+
+        String nameOfRunningVM = ManagementFactory.getRuntimeMXBean().getName();
+        int p = nameOfRunningVM.indexOf('@');
+        String pid = nameOfRunningVM.substring(0, p);
+
+        // Make the nmethod go away
+        for (int i = 0; i < 10; i++) {
+            System.gc();
+        }
+
+        // Redefine class
+        try {
+            VirtualMachine vm = VirtualMachine.attach(pid);
+            vm.loadAgent(System.getProperty("test.classes",".") + "/agent.jar", "");
+            vm.detach();
+        } catch (Exception e) {
+            throw new RuntimeException(e);
+        }
+
+        Test.m();
+        // GC will hit dead method pointer
+        for (int i = 0; i < 10; i++) {
+            System.gc();
+        }
+    }
+
+    public synchronized byte[] transform(final ClassLoader classLoader,
+                                         final String className,
+                                         Class<?> classBeingRedefined,
+                                         ProtectionDomain protectionDomain,
+                                         byte[] classfileBuffer) {
+        System.out.println("Transforming class " + className);
+        return classfileBuffer;
+    }
+
+    public static void redefine(String agentArgs, Instrumentation instrumentation, Class to_redefine) {
+
+        try {
+            instrumentation.retransformClasses(to_redefine);
+        } catch (Exception e) {
+            e.printStackTrace();
+        }
+
+    }
+
+    public static void agentmain(String agentArgs, Instrumentation instrumentation) throws Exception {
+        Agent transformer = new Agent();
+        instrumentation.addTransformer(transformer, true);
+
+        redefine(agentArgs, instrumentation, Test.class);
+    }
+}

test/compiler/profiling/spectrapredefineclass/Launcher.java

+/*
+ * Copyright (c) 2014, 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.PrintWriter;
+import com.oracle.java.testlibrary.*;
+
+/*
+ * @test
+ * @bug 8038636
+ * @library /testlibrary
+ * @build Agent
+ * @run main ClassFileInstaller Agent
+ * @run main Launcher
+ * @run main/othervm -XX:-TieredCompilation -XX:-BackgroundCompilation -XX:-UseOnStackReplacement -XX:TypeProfileLevel=222 -Xmx1M -XX:ReservedCodeCacheSize=3M Agent
+ */
+public class Launcher {
+    public static void main(String[] args) throws Exception  {
+
+      PrintWriter pw = new PrintWriter("MANIFEST.MF");
+      pw.println("Agent-Class: Agent");
+      pw.println("Can-Retransform-Classes: true");
+      pw.close();
+
+      ProcessBuilder pb = new ProcessBuilder();
+      pb.command(new String[] { JDKToolFinder.getJDKTool("jar"), "cmf", "MANIFEST.MF", System.getProperty("test.classes",".") + "/agent.jar", "Agent.class"});
+      pb.start().waitFor();
+    }
+}