8140685: Fix backtrace building to not rely on constant pool merging

Store Symbol* for the name in the backtrace

Reviewed-by: gtriantafill, dholmes, kbarrett, lfoltan
This commit is contained in:
Coleen Phillimore 2017-02-01 17:56:22 -05:00
parent fe4d1bb602
commit 5a2d8cb819
5 changed files with 59 additions and 63 deletions

View File

@ -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 @@ class BacktraceBuilder: public StackObj {
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 @@ class BacktraceBuilder: public StackObj {
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 @@ class BacktraceBuilder: public StackObj {
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 @@ class BacktraceBuilder: public StackObj {
_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 @@ class BacktraceBuilder: public StackObj {
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 @@ class BacktraceBuilder: public StackObj {
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 @@ class BacktraceBuilder: public StackObj {
_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 @@ struct BacktraceElement : public StackObj {
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 @@ class BacktraceIterator : public StackObj {
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 @@ class BacktraceIterator : public StackObj {
_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 @@ class BacktraceIterator : public StackObj {
_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 @@ class BacktraceIterator : public StackObj {
// 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 @@ static void print_stack_element_to_stream(outputStream* st, Handle mirror, int m
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 @@ static void print_stack_element_to_stream(outputStream* st, Handle mirror, int m
}
}
// 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 @@ void java_lang_Throwable::print_stack_element(outputStream *st, const methodHand
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 @@ void java_lang_Throwable::print_stack_trace(Handle throwable, outputStream* st)
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 @@ void java_lang_Throwable::get_stack_trace_elements(Handle throwable,
method,
bte._version,
bte._bci,
bte._cpref, CHECK);
bte._name, CHECK);
}
}
@ -2159,15 +2161,14 @@ oop java_lang_StackTraceElement::create(const methodHandle& method, int bci, TRA
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 @@ void java_lang_StackTraceElement::fill_in(Handle element,
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 @@ void java_lang_StackTraceElement::fill_in(Handle element,
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 @@ void java_lang_StackFrameInfo::to_stack_trace_element(Handle stackFrame, Handle
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() {

View File

@ -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 @@ class java_lang_Throwable: AllStatic {
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 @@ class java_lang_StackTraceElement: AllStatic {
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;

View File

@ -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 @@ typeArrayOop oopFactory::new_typeArray(BasicType type, int length, TRAPS) {
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;
}

View File

@ -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 @@ class oopFactory: AllStatic {
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);

View File

@ -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 @@ class typeArrayOopDesc : public arrayOopDesc {
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