# HG changeset patch # User coleenp # Date 1368473828 14400 # Node ID 43083e670adf63c7741633404e5e289d8275ac03 # Parent 8b40495b9381130b5f06c083df3f56ba45e59b05 8005056: NPG: Crash after redefining java.lang.Object Summary: Need to walk array class vtables replacing old methods too if j.l.o redefined Reviewed-by: sspitsyn, dcubed, ctornqvi diff -r 8b40495b9381 -r 43083e670adf src/share/vm/classfile/dictionary.cpp --- a/src/share/vm/classfile/dictionary.cpp Mon May 13 18:08:13 2013 +0000 +++ b/src/share/vm/classfile/dictionary.cpp Mon May 13 15:37:08 2013 -0400 @@ -253,22 +253,6 @@ } } - -// All classes, and their class loaders -// (added for helpers that use HandleMarks and ResourceMarks) -// Don't iterate over placeholders -void Dictionary::classes_do(void f(Klass*, ClassLoaderData*, TRAPS), TRAPS) { - for (int index = 0; index < table_size(); index++) { - for (DictionaryEntry* probe = bucket(index); - probe != NULL; - probe = probe->next()) { - Klass* k = probe->klass(); - f(k, probe->loader_data(), CHECK); - } - } -} - - // All classes, and their class loaders // Don't iterate over placeholders void Dictionary::classes_do(void f(Klass*, ClassLoaderData*)) { diff -r 8b40495b9381 -r 43083e670adf src/share/vm/classfile/dictionary.hpp --- a/src/share/vm/classfile/dictionary.hpp Mon May 13 18:08:13 2013 +0000 +++ b/src/share/vm/classfile/dictionary.hpp Mon May 13 15:37:08 2013 -0400 @@ -90,7 +90,6 @@ void classes_do(void f(Klass*)); void classes_do(void f(Klass*, TRAPS), TRAPS); void classes_do(void f(Klass*, ClassLoaderData*)); - void classes_do(void f(Klass*, ClassLoaderData*, TRAPS), TRAPS); void methods_do(void f(Method*)); diff -r 8b40495b9381 -r 43083e670adf src/share/vm/classfile/systemDictionary.cpp --- a/src/share/vm/classfile/systemDictionary.cpp Mon May 13 18:08:13 2013 +0000 +++ b/src/share/vm/classfile/systemDictionary.cpp Mon May 13 15:37:08 2013 -0400 @@ -1747,13 +1747,6 @@ dictionary()->classes_do(f); } -// All classes, and their class loaders -// (added for helpers that use HandleMarks and ResourceMarks) -// Don't iterate over placeholders -void SystemDictionary::classes_do(void f(Klass*, ClassLoaderData*, TRAPS), TRAPS) { - dictionary()->classes_do(f, CHECK); -} - void SystemDictionary::placeholders_do(void f(Symbol*)) { placeholders()->entries_do(f); } diff -r 8b40495b9381 -r 43083e670adf src/share/vm/classfile/systemDictionary.hpp --- a/src/share/vm/classfile/systemDictionary.hpp Mon May 13 18:08:13 2013 +0000 +++ b/src/share/vm/classfile/systemDictionary.hpp Mon May 13 15:37:08 2013 -0400 @@ -313,10 +313,7 @@ static void classes_do(void f(Klass*, TRAPS), TRAPS); // All classes, and their class loaders static void classes_do(void f(Klass*, ClassLoaderData*)); - // All classes, and their class loaders - // (added for helpers that use HandleMarks and ResourceMarks) - static void classes_do(void f(Klass*, ClassLoaderData*, TRAPS), TRAPS); - // All entries in the placeholder table and their class loaders + static void placeholders_do(void f(Symbol*)); // Iterate over all methods in all klasses in dictionary diff -r 8b40495b9381 -r 43083e670adf src/share/vm/oops/klass.hpp --- a/src/share/vm/oops/klass.hpp Mon May 13 18:08:13 2013 +0000 +++ b/src/share/vm/oops/klass.hpp Mon May 13 15:37:08 2013 -0400 @@ -393,6 +393,7 @@ // vtables virtual klassVtable* vtable() const { return NULL; } + virtual int vtable_length() const { return 0; } // subclass check bool is_subclass_of(const Klass* k) const; diff -r 8b40495b9381 -r 43083e670adf src/share/vm/prims/jvmtiRedefineClasses.cpp --- a/src/share/vm/prims/jvmtiRedefineClasses.cpp Mon May 13 18:08:13 2013 +0000 +++ b/src/share/vm/prims/jvmtiRedefineClasses.cpp Mon May 13 15:37:08 2013 -0400 @@ -160,7 +160,8 @@ if (RC_TRACE_ENABLED(0x00004000)) { #endif RC_TRACE_WITH_THREAD(0x00004000, thread, ("calling check_class")); - SystemDictionary::classes_do(check_class, thread); + CheckClass check_class(thread); + ClassLoaderDataGraph::classes_do(&check_class); #ifdef PRODUCT } #endif @@ -2653,29 +2654,35 @@ } // end set_new_constant_pool() -void VM_RedefineClasses::adjust_array_vtable(Klass* k_oop) { - ArrayKlass* ak = ArrayKlass::cast(k_oop); - bool trace_name_printed = false; - ak->vtable()->adjust_method_entries(_matching_old_methods, - _matching_new_methods, - _matching_methods_length, - &trace_name_printed); -} - // Unevolving classes may point to methods of the_class directly // from their constant pool caches, itables, and/or vtables. We -// use the SystemDictionary::classes_do() facility and this helper +// use the ClassLoaderDataGraph::classes_do() facility and this helper // to fix up these pointers. -// -// Note: We currently don't support updating the vtable in -// arrayKlassOops. See Open Issues in jvmtiRedefineClasses.hpp. -void VM_RedefineClasses::adjust_cpool_cache_and_vtable(Klass* k_oop, - ClassLoaderData* initiating_loader, - TRAPS) { - Klass *k = k_oop; - if (k->oop_is_instance()) { - HandleMark hm(THREAD); - InstanceKlass *ik = (InstanceKlass *) k; + +// Adjust cpools and vtables closure +void VM_RedefineClasses::AdjustCpoolCacheAndVtable::do_klass(Klass* k) { + + // This is a very busy routine. We don't want too much tracing + // printed out. + bool trace_name_printed = false; + + // Very noisy: only enable this call if you are trying to determine + // that a specific class gets found by this routine. + // RC_TRACE macro has an embedded ResourceMark + // RC_TRACE_WITH_THREAD(0x00100000, THREAD, + // ("adjust check: name=%s", k->external_name())); + // trace_name_printed = true; + + // If the class being redefined is java.lang.Object, we need to fix all + // array class vtables also + if (k->oop_is_array() && _the_class_oop == SystemDictionary::Object_klass()) { + k->vtable()->adjust_method_entries(_matching_old_methods, + _matching_new_methods, + _matching_methods_length, + &trace_name_printed); + } else if (k->oop_is_instance()) { + HandleMark hm(_thread); + InstanceKlass *ik = InstanceKlass::cast(k); // HotSpot specific optimization! HotSpot does not currently // support delegation from the bootstrap class loader to a @@ -2695,23 +2702,6 @@ return; } - // If the class being redefined is java.lang.Object, we need to fix all - // array class vtables also - if (_the_class_oop == SystemDictionary::Object_klass()) { - ik->array_klasses_do(adjust_array_vtable); - } - - // This is a very busy routine. We don't want too much tracing - // printed out. - bool trace_name_printed = false; - - // Very noisy: only enable this call if you are trying to determine - // that a specific class gets found by this routine. - // RC_TRACE macro has an embedded ResourceMark - // RC_TRACE_WITH_THREAD(0x00100000, THREAD, - // ("adjust check: name=%s", ik->external_name())); - // trace_name_printed = true; - // Fix the vtable embedded in the_class and subclasses of the_class, // if one exists. We discard scratch_class and we don't keep an // InstanceKlass around to hold obsolete methods so we don't have @@ -2719,7 +2709,7 @@ // holds the Method*s for virtual (but not final) methods. if (ik->vtable_length() > 0 && ik->is_subtype_of(_the_class_oop)) { // ik->vtable() creates a wrapper object; rm cleans it up - ResourceMark rm(THREAD); + ResourceMark rm(_thread); ik->vtable()->adjust_method_entries(_matching_old_methods, _matching_new_methods, _matching_methods_length, @@ -2735,7 +2725,7 @@ if (ik->itable_length() > 0 && (_the_class_oop->is_interface() || ik->is_subclass_of(_the_class_oop))) { // ik->itable() creates a wrapper object; rm cleans it up - ResourceMark rm(THREAD); + ResourceMark rm(_thread); ik->itable()->adjust_method_entries(_matching_old_methods, _matching_new_methods, _matching_methods_length, @@ -2758,7 +2748,7 @@ constantPoolHandle other_cp; ConstantPoolCache* cp_cache; - if (k_oop != _the_class_oop) { + if (ik != _the_class_oop) { // this klass' constant pool cache may need adjustment other_cp = constantPoolHandle(ik->constants()); cp_cache = other_cp->cache(); @@ -2770,7 +2760,7 @@ } } { - ResourceMark rm(THREAD); + ResourceMark rm(_thread); // PreviousVersionInfo objects returned via PreviousVersionWalker // contain a GrowableArray of handles. We have to clean up the // GrowableArray _after_ the PreviousVersionWalker destructor @@ -3208,7 +3198,7 @@ // parts of the_class // - adjusting constant pool caches and vtables in other classes // that refer to methods in the_class. These adjustments use the -// SystemDictionary::classes_do() facility which only allows +// ClassLoaderDataGraph::classes_do() facility which only allows // a helper method to be specified. The interesting parameters // that we would like to pass to the helper method are saved in // static global fields in the VM operation. @@ -3442,7 +3432,8 @@ // Adjust constantpool caches and vtables for all classes // that reference methods of the evolved class. - SystemDictionary::classes_do(adjust_cpool_cache_and_vtable, THREAD); + AdjustCpoolCacheAndVtable adjust_cpool_cache_and_vtable(THREAD); + ClassLoaderDataGraph::classes_do(&adjust_cpool_cache_and_vtable); // JSR-292 support MemberNameTable* mnt = the_class->member_names(); @@ -3503,34 +3494,33 @@ } } -void VM_RedefineClasses::check_class(Klass* k_oop, - ClassLoaderData* initiating_loader, - TRAPS) { - Klass *k = k_oop; +void VM_RedefineClasses::CheckClass::do_klass(Klass* k) { + bool no_old_methods = true; // be optimistic + + // Both array and instance classes have vtables. + // a vtable should never contain old or obsolete methods + ResourceMark rm(_thread); + if (k->vtable_length() > 0 && + !k->vtable()->check_no_old_or_obsolete_entries()) { + if (RC_TRACE_ENABLED(0x00004000)) { + RC_TRACE_WITH_THREAD(0x00004000, _thread, + ("klassVtable::check_no_old_or_obsolete_entries failure" + " -- OLD or OBSOLETE method found -- class: %s", + k->signature_name())); + k->vtable()->dump_vtable(); + } + no_old_methods = false; + } + if (k->oop_is_instance()) { - HandleMark hm(THREAD); - InstanceKlass *ik = (InstanceKlass *) k; - bool no_old_methods = true; // be optimistic - ResourceMark rm(THREAD); - - // a vtable should never contain old or obsolete methods - if (ik->vtable_length() > 0 && - !ik->vtable()->check_no_old_or_obsolete_entries()) { - if (RC_TRACE_ENABLED(0x00004000)) { - RC_TRACE_WITH_THREAD(0x00004000, THREAD, - ("klassVtable::check_no_old_or_obsolete_entries failure" - " -- OLD or OBSOLETE method found -- class: %s", - ik->signature_name())); - ik->vtable()->dump_vtable(); - } - no_old_methods = false; - } + HandleMark hm(_thread); + InstanceKlass *ik = InstanceKlass::cast(k); // an itable should never contain old or obsolete methods if (ik->itable_length() > 0 && !ik->itable()->check_no_old_or_obsolete_entries()) { if (RC_TRACE_ENABLED(0x00004000)) { - RC_TRACE_WITH_THREAD(0x00004000, THREAD, + RC_TRACE_WITH_THREAD(0x00004000, _thread, ("klassItable::check_no_old_or_obsolete_entries failure" " -- OLD or OBSOLETE method found -- class: %s", ik->signature_name())); @@ -3544,7 +3534,7 @@ ik->constants()->cache() != NULL && !ik->constants()->cache()->check_no_old_or_obsolete_entries()) { if (RC_TRACE_ENABLED(0x00004000)) { - RC_TRACE_WITH_THREAD(0x00004000, THREAD, + RC_TRACE_WITH_THREAD(0x00004000, _thread, ("cp-cache::check_no_old_or_obsolete_entries failure" " -- OLD or OBSOLETE method found -- class: %s", ik->signature_name())); @@ -3552,19 +3542,21 @@ } no_old_methods = false; } - - if (!no_old_methods) { - if (RC_TRACE_ENABLED(0x00004000)) { - dump_methods(); - } else { - tty->print_cr("INFO: use the '-XX:TraceRedefineClasses=16384' option " - "to see more info about the following guarantee() failure."); - } - guarantee(false, "OLD and/or OBSOLETE method(s) found"); + } + + // print and fail guarantee if old methods are found. + if (!no_old_methods) { + if (RC_TRACE_ENABLED(0x00004000)) { + dump_methods(); + } else { + tty->print_cr("INFO: use the '-XX:TraceRedefineClasses=16384' option " + "to see more info about the following guarantee() failure."); } + guarantee(false, "OLD and/or OBSOLETE method(s) found"); } } + void VM_RedefineClasses::dump_methods() { int j; RC_TRACE(0x00004000, ("_old_methods --")); diff -r 8b40495b9381 -r 43083e670adf src/share/vm/prims/jvmtiRedefineClasses.hpp --- a/src/share/vm/prims/jvmtiRedefineClasses.hpp Mon May 13 18:08:13 2013 +0000 +++ b/src/share/vm/prims/jvmtiRedefineClasses.hpp Mon May 13 15:37:08 2013 -0400 @@ -87,7 +87,7 @@ // parts of the_class // - adjusting constant pool caches and vtables in other classes // that refer to methods in the_class. These adjustments use the -// SystemDictionary::classes_do() facility which only allows +// ClassLoaderDataGraph::classes_do() facility which only allows // a helper method to be specified. The interesting parameters // that we would like to pass to the helper method are saved in // static global fields in the VM operation. @@ -333,8 +333,8 @@ class VM_RedefineClasses: public VM_Operation { private: - // These static fields are needed by SystemDictionary::classes_do() - // facility and the adjust_cpool_cache_and_vtable() helper: + // These static fields are needed by ClassLoaderDataGraph::classes_do() + // facility and the AdjustCpoolCacheAndVtable helper: static Array* _old_methods; static Array* _new_methods; static Method** _matching_old_methods; @@ -408,13 +408,6 @@ int * emcp_method_count_p); void transfer_old_native_function_registrations(instanceKlassHandle the_class); - // Unevolving classes may point to methods of the_class directly - // from their constant pool caches, itables, and/or vtables. We - // use the SystemDictionary::classes_do() facility and this helper - // to fix up these pointers. - static void adjust_cpool_cache_and_vtable(Klass* k_oop, ClassLoaderData* initiating_loader, TRAPS); - static void adjust_array_vtable(Klass* k_oop); - // Install the redefinition of a class void redefine_single_class(jclass the_jclass, Klass* scratch_class_oop, TRAPS); @@ -480,10 +473,27 @@ void flush_dependent_code(instanceKlassHandle k_h, TRAPS); - static void check_class(Klass* k_oop, ClassLoaderData* initiating_loader, - TRAPS); static void dump_methods(); + // Check that there are no old or obsolete methods + class CheckClass : public KlassClosure { + Thread* _thread; + public: + CheckClass(Thread* t) : _thread(t) {} + void do_klass(Klass* k); + }; + + // Unevolving classes may point to methods of the_class directly + // from their constant pool caches, itables, and/or vtables. We + // use the ClassLoaderDataGraph::classes_do() facility and this helper + // to fix up these pointers. + class AdjustCpoolCacheAndVtable : public KlassClosure { + Thread* _thread; + public: + AdjustCpoolCacheAndVtable(Thread* t) : _thread(t) {} + void do_klass(Klass* k); + }; + public: VM_RedefineClasses(jint class_count, const jvmtiClassDefinition *class_defs, diff -r 8b40495b9381 -r 43083e670adf test/runtime/RedefineObject/Agent.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/runtime/RedefineObject/Agent.java Mon May 13 15:37:08 2013 -0400 @@ -0,0 +1,61 @@ +/* + * Copyright (c) 2013, 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.*; + +public class Agent implements ClassFileTransformer { + 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 premain(String agentArgs, Instrumentation instrumentation) { + + Agent transformer = new Agent(); + + instrumentation.addTransformer(transformer, true); + + Class c = Object.class; + try { + instrumentation.retransformClasses(c); + } catch (Exception e) { + e.printStackTrace(); + } + + instrumentation.removeTransformer(transformer); + } + + public static void main(String[] args) { + byte[] ba = new byte[0]; + + // If it survives 1000 GC's, it's good. + for (int i = 0; i < 1000 ; i++) { + System.gc(); + ba.clone(); + } + } +} diff -r 8b40495b9381 -r 43083e670adf test/runtime/RedefineObject/TestRedefineObject.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/runtime/RedefineObject/TestRedefineObject.java Mon May 13 15:37:08 2013 -0400 @@ -0,0 +1,50 @@ +/* + * Copyright (c) 2013, 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 to redefine java/lang/Object and verify that it doesn't crash on vtable + * call on basic array type. + * + * @test + * @bug 8005056 + * @library /testlibrary + * @build Agent + * @run main ClassFileInstaller Agent + * @run main Test + * @run main/othervm -javaagent:agent.jar Agent + */ +public class Test { + public static void main(String[] args) throws Exception { + + PrintWriter pw = new PrintWriter("MANIFEST.MF"); + pw.println("Premain-Class: Agent"); + pw.println("Can-Retransform-Classes: true"); + pw.close(); + + ProcessBuilder pb = new ProcessBuilder(); + pb.command(new String[] { JDKToolFinder.getJDKTool("jar"), "cmf", "MANIFEST.MF", "agent.jar", "Agent.class"}); + pb.start().waitFor(); + } +} diff -r 8b40495b9381 -r 43083e670adf test/testlibrary/ClassFileInstaller.java --- a/test/testlibrary/ClassFileInstaller.java Mon May 13 18:08:13 2013 +0000 +++ b/test/testlibrary/ClassFileInstaller.java Mon May 13 15:37:08 2013 -0400 @@ -45,7 +45,9 @@ // Create the class file's package directory Path p = Paths.get(pathName); - Files.createDirectories(p.getParent()); + if (pathName.contains("/")) { + Files.createDirectories(p.getParent()); + } // Create the class file Files.copy(is, p, StandardCopyOption.REPLACE_EXISTING); }