# HG changeset patch # User coleenp # Date 1485989782 18000 # Node ID 3e95288ce4ca737a3cc3463009bd945ae3b024c0 # Parent 07baf498d588eaf67325e148209403a4cf84270b 8140685: Fix backtrace building to not rely on constant pool merging Summary: Store Symbol* for the name in the backtrace Reviewed-by: gtriantafill, dholmes, kbarrett, lfoltan diff -r 07baf498d588 -r 3e95288ce4ca hotspot/src/share/vm/classfile/javaClasses.cpp --- a/hotspot/src/share/vm/classfile/javaClasses.cpp Tue Jan 31 19:26:50 2017 -0500 +++ b/hotspot/src/share/vm/classfile/javaClasses.cpp Wed Feb 01 17:56:22 2017 -0500 @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2016, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 2017, 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 @@ -1578,7 +1578,7 @@ typeArrayOop _methods; typeArrayOop _bcis; objArrayOop _mirrors; - typeArrayOop _cprefs; // needed to insulate method name against redefinition + typeArrayOop _names; // needed to insulate method name against redefinition int _index; NoSafepointVerifier _nsv; @@ -1586,7 +1586,7 @@ trace_methods_offset = java_lang_Throwable::trace_methods_offset, trace_bcis_offset = java_lang_Throwable::trace_bcis_offset, trace_mirrors_offset = java_lang_Throwable::trace_mirrors_offset, - trace_cprefs_offset = java_lang_Throwable::trace_cprefs_offset, + trace_names_offset = java_lang_Throwable::trace_names_offset, trace_next_offset = java_lang_Throwable::trace_next_offset, trace_size = java_lang_Throwable::trace_size, trace_chunk_size = java_lang_Throwable::trace_chunk_size @@ -1608,16 +1608,16 @@ assert(mirrors != NULL, "mirror array should be initialized in backtrace"); return mirrors; } - static typeArrayOop get_cprefs(objArrayHandle chunk) { - typeArrayOop cprefs = typeArrayOop(chunk->obj_at(trace_cprefs_offset)); - assert(cprefs != NULL, "cprefs array should be initialized in backtrace"); - return cprefs; + static typeArrayOop get_names(objArrayHandle chunk) { + typeArrayOop names = typeArrayOop(chunk->obj_at(trace_names_offset)); + assert(names != NULL, "names array should be initialized in backtrace"); + return names; } public: // constructor for new backtrace - BacktraceBuilder(TRAPS): _methods(NULL), _bcis(NULL), _head(NULL), _mirrors(NULL), _cprefs(NULL) { + BacktraceBuilder(TRAPS): _methods(NULL), _bcis(NULL), _head(NULL), _mirrors(NULL), _names(NULL) { expand(CHECK); _backtrace = _head; _index = 0; @@ -1627,9 +1627,10 @@ _methods = get_methods(backtrace); _bcis = get_bcis(backtrace); _mirrors = get_mirrors(backtrace); - _cprefs = get_cprefs(backtrace); + _names = get_names(backtrace); assert(_methods->length() == _bcis->length() && - _methods->length() == _mirrors->length(), + _methods->length() == _mirrors->length() && + _mirrors->length() == _names->length(), "method and source information arrays should match"); // head is the preallocated backtrace @@ -1653,8 +1654,8 @@ objArrayOop mirrors = oopFactory::new_objectArray(trace_chunk_size, CHECK); objArrayHandle new_mirrors(THREAD, mirrors); - typeArrayOop cprefs = oopFactory::new_shortArray(trace_chunk_size, CHECK); - typeArrayHandle new_cprefs(THREAD, cprefs); + typeArrayOop names = oopFactory::new_symbolArray(trace_chunk_size, CHECK); + typeArrayHandle new_names(THREAD, names); if (!old_head.is_null()) { old_head->obj_at_put(trace_next_offset, new_head()); @@ -1662,13 +1663,13 @@ new_head->obj_at_put(trace_methods_offset, new_methods()); new_head->obj_at_put(trace_bcis_offset, new_bcis()); new_head->obj_at_put(trace_mirrors_offset, new_mirrors()); - new_head->obj_at_put(trace_cprefs_offset, new_cprefs()); + new_head->obj_at_put(trace_names_offset, new_names()); _head = new_head(); _methods = new_methods(); _bcis = new_bcis(); _mirrors = new_mirrors(); - _cprefs = new_cprefs(); + _names = new_names(); _index = 0; } @@ -1690,7 +1691,11 @@ _methods->short_at_put(_index, method->orig_method_idnum()); _bcis->int_at_put(_index, Backtrace::merge_bci_and_version(bci, method->constants()->version())); - _cprefs->short_at_put(_index, method->name_index()); + + // Note:this doesn't leak symbols because the mirror in the backtrace keeps the + // klass owning the symbols alive so their refcounts aren't decremented. + Symbol* name = method->name(); + _names->symbol_at_put(_index, name); // We need to save the mirrors in the backtrace to keep the class // from being unloaded while we still have this stack trace. @@ -1705,10 +1710,10 @@ int _method_id; int _bci; int _version; - int _cpref; + Symbol* _name; Handle _mirror; - BacktraceElement(Handle mirror, int mid, int version, int bci, int cpref) : - _mirror(mirror), _method_id(mid), _version(version), _bci(bci), _cpref(cpref) {} + BacktraceElement(Handle mirror, int mid, int version, int bci, Symbol* name) : + _mirror(mirror), _method_id(mid), _version(version), _bci(bci), _name(name) {} }; class BacktraceIterator : public StackObj { @@ -1717,7 +1722,7 @@ objArrayHandle _mirrors; typeArrayHandle _methods; typeArrayHandle _bcis; - typeArrayHandle _cprefs; + typeArrayHandle _names; void init(objArrayHandle result, Thread* thread) { // Get method id, bci, version and mirror from chunk @@ -1726,7 +1731,7 @@ _methods = typeArrayHandle(thread, BacktraceBuilder::get_methods(_result)); _bcis = typeArrayHandle(thread, BacktraceBuilder::get_bcis(_result)); _mirrors = objArrayHandle(thread, BacktraceBuilder::get_mirrors(_result)); - _cprefs = typeArrayHandle(thread, BacktraceBuilder::get_cprefs(_result)); + _names = typeArrayHandle(thread, BacktraceBuilder::get_names(_result)); _index = 0; } } @@ -1741,7 +1746,7 @@ _methods->short_at(_index), Backtrace::version_at(_bcis->int_at(_index)), Backtrace::bci_at(_bcis->int_at(_index)), - _cprefs->short_at(_index)); + _names->symbol_at(_index)); _index++; if (_index >= java_lang_Throwable::trace_chunk_size) { @@ -1761,7 +1766,7 @@ // Print stack trace element to resource allocated buffer static void print_stack_element_to_stream(outputStream* st, Handle mirror, int method_id, - int version, int bci, int cpref) { + int version, int bci, Symbol* name) { ResourceMark rm; // Get strings and string lengths @@ -1769,11 +1774,7 @@ const char* klass_name = holder->external_name(); int buf_len = (int)strlen(klass_name); - Method* method = holder->method_with_orig_idnum(method_id, version); - - // The method can be NULL if the requested class version is gone - Symbol* sym = (method != NULL) ? method->name() : holder->constants()->symbol_at(cpref); - char* method_name = sym->as_C_string(); + char* method_name = name->as_C_string(); buf_len += (int)strlen(method_name); char* source_file_name = NULL; @@ -1809,6 +1810,8 @@ } } + // The method can be NULL if the requested class version is gone + Method* method = holder->method_with_orig_idnum(method_id, version); if (!version_matches(method, version)) { strcat(buf, "Redefined)"); } else { @@ -1840,8 +1843,7 @@ Handle mirror = method->method_holder()->java_mirror(); int method_id = method->orig_method_idnum(); int version = method->constants()->version(); - int cpref = method->name_index(); - print_stack_element_to_stream(st, mirror, method_id, version, bci, cpref); + print_stack_element_to_stream(st, mirror, method_id, version, bci, method->name()); } /** @@ -1865,7 +1867,7 @@ while (iter.repeat()) { BacktraceElement bte = iter.next(THREAD); - print_stack_element_to_stream(st, bte._mirror, bte._method_id, bte._version, bte._bci, bte._cpref); + print_stack_element_to_stream(st, bte._mirror, bte._method_id, bte._version, bte._bci, bte._name); } { // Call getCause() which doesn't necessarily return the _cause field. @@ -2144,7 +2146,7 @@ method, bte._version, bte._bci, - bte._cpref, CHECK); + bte._name, CHECK); } } @@ -2159,15 +2161,14 @@ Handle element = ik->allocate_instance_handle(CHECK_0); - int cpref = method->name_index(); int version = method->constants()->version(); - fill_in(element, method->method_holder(), method, version, bci, cpref, CHECK_0); + fill_in(element, method->method_holder(), method, version, bci, method->name(), CHECK_0); return element(); } void java_lang_StackTraceElement::fill_in(Handle element, InstanceKlass* holder, const methodHandle& method, - int version, int bci, int cpref, TRAPS) { + int version, int bci, Symbol* name, TRAPS) { assert(element->is_a(SystemDictionary::StackTraceElement_klass()), "sanity check"); // Fill in class name @@ -2184,11 +2185,8 @@ java_lang_StackTraceElement::set_classLoaderName(element(), loader_name); } - // The method can be NULL if the requested class version is gone - Symbol* sym = !method.is_null() ? method->name() : holder->constants()->symbol_at(cpref); - // Fill in method name - oop methodname = StringTable::intern(sym, CHECK); + oop methodname = StringTable::intern(name, CHECK); java_lang_StackTraceElement::set_methodName(element(), methodname); // Fill in module name and version @@ -2205,7 +2203,7 @@ java_lang_StackTraceElement::set_moduleVersion(element(), module_version); } - if (!version_matches(method(), version)) { + if (method() == NULL || !version_matches(method(), version)) { // The method was redefined, accurate line number information isn't available java_lang_StackTraceElement::set_fileName(element(), NULL); java_lang_StackTraceElement::set_lineNumber(element(), -1); @@ -2252,8 +2250,8 @@ short version = stackFrame->short_field(_version_offset); short bci = stackFrame->short_field(_bci_offset); - int cpref = method->name_index(); - java_lang_StackTraceElement::fill_in(stack_trace_element, holder, method, version, bci, cpref, CHECK); + Symbol* name = method->name(); + java_lang_StackTraceElement::fill_in(stack_trace_element, holder, method, version, bci, name, CHECK); } void java_lang_StackFrameInfo::compute_offsets() { diff -r 07baf498d588 -r 3e95288ce4ca hotspot/src/share/vm/classfile/javaClasses.hpp --- a/hotspot/src/share/vm/classfile/javaClasses.hpp Tue Jan 31 19:26:50 2017 -0500 +++ b/hotspot/src/share/vm/classfile/javaClasses.hpp Wed Feb 01 17:56:22 2017 -0500 @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2016, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 2017, 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 @@ -466,7 +466,7 @@ trace_methods_offset = 0, trace_bcis_offset = 1, trace_mirrors_offset = 2, - trace_cprefs_offset = 3, + trace_names_offset = 3, trace_next_offset = 4, trace_size = 5, trace_chunk_size = 32 @@ -1331,7 +1331,7 @@ static oop create(const methodHandle& method, int bci, TRAPS); static void fill_in(Handle element, InstanceKlass* holder, const methodHandle& method, - int version, int bci, int cpref, TRAPS); + int version, int bci, Symbol* name, TRAPS); // Debugging friend class JavaClasses; diff -r 07baf498d588 -r 3e95288ce4ca hotspot/src/share/vm/memory/oopFactory.cpp --- a/hotspot/src/share/vm/memory/oopFactory.cpp Tue Jan 31 19:26:50 2017 -0500 +++ b/hotspot/src/share/vm/memory/oopFactory.cpp Wed Feb 01 17:56:22 2017 -0500 @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2015, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 2017, 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 @@ -57,17 +57,15 @@ return result; } -// Create a Java array that points to metadata. -// As far as Java code is concerned, a metaData array is either an array of -// int or long depending on pointer size. Only a few things use this, like -// stack trace elements in Throwable. They cast Method* into this type. -// Note:can't point to symbols because there's no way to unreference count -// them when this object goes away. -typeArrayOop oopFactory::new_metaDataArray(int length, TRAPS) { +// Create a Java array that points to Symbol. +// As far as Java code is concerned, a Symbol array is either an array of +// int or long depending on pointer size. Only stack trace elements in Throwable use +// this. They cast Symbol* into this type. +typeArrayOop oopFactory::new_symbolArray(int length, TRAPS) { BasicType type = LP64_ONLY(T_LONG) NOT_LP64(T_INT); Klass* type_asKlassOop = Universe::typeArrayKlassObj(type); TypeArrayKlass* type_asArrayKlass = TypeArrayKlass::cast(type_asKlassOop); - typeArrayOop result = type_asArrayKlass->allocate_common(length, true, THREAD); + typeArrayOop result = type_asArrayKlass->allocate(length, THREAD); return result; } diff -r 07baf498d588 -r 3e95288ce4ca hotspot/src/share/vm/memory/oopFactory.hpp --- a/hotspot/src/share/vm/memory/oopFactory.hpp Tue Jan 31 19:26:50 2017 -0500 +++ b/hotspot/src/share/vm/memory/oopFactory.hpp Wed Feb 01 17:56:22 2017 -0500 @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2013, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 2017, 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 @@ -62,7 +62,7 @@ static typeArrayOop new_typeArray(BasicType type, int length, TRAPS); static typeArrayOop new_typeArray_nozero(BasicType type, int length, TRAPS); - static typeArrayOop new_metaDataArray(int length, TRAPS); + static typeArrayOop new_symbolArray(int length, TRAPS); // Regular object arrays static objArrayOop new_objArray(Klass* klass, int length, TRAPS); diff -r 07baf498d588 -r 3e95288ce4ca hotspot/src/share/vm/oops/typeArrayOop.hpp --- a/hotspot/src/share/vm/oops/typeArrayOop.hpp Tue Jan 31 19:26:50 2017 -0500 +++ b/hotspot/src/share/vm/oops/typeArrayOop.hpp Wed Feb 01 17:56:22 2017 -0500 @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2016, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 2017, 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 @@ -122,19 +122,19 @@ jbyte byte_at_acquire(int which) const { return OrderAccess::load_acquire(byte_at_addr(which)); } void release_byte_at_put(int which, jbyte contents) { OrderAccess::release_store(byte_at_addr(which), contents); } - // Java thinks metadata arrays are just arrays of either long or int, since + // Java thinks Symbol arrays are just arrays of either long or int, since // there doesn't seem to be T_ADDRESS, so this is a bit of unfortunate // casting #ifdef _LP64 - Metadata* metadata_at(int which) const { - return (Metadata*)*long_at_addr(which); } - void metadata_at_put(int which, Metadata* contents) { + Symbol* symbol_at(int which) const { + return (Symbol*)*long_at_addr(which); } + void symbol_at_put(int which, Symbol* contents) { *long_at_addr(which) = (jlong)contents; } #else - Metadata* metadata_at(int which) const { - return (Metadata*)*int_at_addr(which); } - void metadata_at_put(int which, Metadata* contents) { + Symbol* symbol_at(int which) const { + return (Symbol*)*int_at_addr(which); } + void symbol_at_put(int which, Symbol* contents) { *int_at_addr(which) = (int)contents; } #endif // _LP64