From f698d6fbdbd0a1262cf1627927141c0e09420885 Mon Sep 17 00:00:00 2001 From: sspitsyn Date: Wed, 8 Apr 2015 14:20:09 -0700 Subject: [PATCH] 8067662: "java.lang.NullPointerException: Method name is null" from StackTraceElement. Summary: use method cpref and klass version to provide meaningful methods name in stacktraces Reviewed-by: coleenp, dcubed --- src/share/vm/classfile/javaClasses.cpp | 92 ++++++++++++++++---------- src/share/vm/classfile/javaClasses.hpp | 13 ++-- src/share/vm/oops/instanceKlass.cpp | 47 +++++++++++++ src/share/vm/oops/instanceKlass.hpp | 3 + 4 files changed, 113 insertions(+), 42 deletions(-) diff --git a/src/share/vm/classfile/javaClasses.cpp b/src/share/vm/classfile/javaClasses.cpp index d5869a757..8106fcaa8 100644 --- a/src/share/vm/classfile/javaClasses.cpp +++ b/src/share/vm/classfile/javaClasses.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2014, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 2015, 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 @@ -1278,7 +1278,8 @@ static inline int version_at(unsigned int merged) { } static inline bool version_matches(Method* method, int version) { - return (method->constants()->version() == version && version < MAX_VERSION); + assert(version < MAX_VERSION, "version is too big"); + return method != NULL && (method->constants()->version() == version); } static inline int get_line_number(Method* method, int bci) { @@ -1308,6 +1309,7 @@ class BacktraceBuilder: public StackObj { typeArrayOop _methods; typeArrayOop _bcis; objArrayOop _mirrors; + typeArrayOop _cprefs; // needed to insulate method name against redefinition int _index; No_Safepoint_Verifier _nsv; @@ -1315,8 +1317,9 @@ class BacktraceBuilder: public StackObj { enum { trace_methods_offset = java_lang_Throwable::trace_methods_offset, - trace_bcis_offset = java_lang_Throwable::trace_bcis_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_next_offset = java_lang_Throwable::trace_next_offset, trace_size = java_lang_Throwable::trace_size, trace_chunk_size = java_lang_Throwable::trace_chunk_size @@ -1338,9 +1341,14 @@ 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; + } // constructor for new backtrace - BacktraceBuilder(TRAPS): _methods(NULL), _bcis(NULL), _head(NULL), _mirrors(NULL) { + BacktraceBuilder(TRAPS): _methods(NULL), _bcis(NULL), _head(NULL), _mirrors(NULL), _cprefs(NULL) { expand(CHECK); _backtrace = _head; _index = 0; @@ -1350,6 +1358,7 @@ class BacktraceBuilder: public StackObj { _methods = get_methods(backtrace); _bcis = get_bcis(backtrace); _mirrors = get_mirrors(backtrace); + _cprefs = get_cprefs(backtrace); assert(_methods->length() == _bcis->length() && _methods->length() == _mirrors->length(), "method and source information arrays should match"); @@ -1375,17 +1384,22 @@ 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); + if (!old_head.is_null()) { old_head->obj_at_put(trace_next_offset, new_head()); } 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()); _head = new_head(); _methods = new_methods(); _bcis = new_bcis(); _mirrors = new_mirrors(); + _cprefs = new_cprefs(); _index = 0; } @@ -1405,8 +1419,9 @@ class BacktraceBuilder: public StackObj { method = mhandle(); } - _methods->short_at_put(_index, method->method_idnum()); + _methods->short_at_put(_index, method->orig_method_idnum()); _bcis->int_at_put(_index, merge_bci_and_version(bci, method->constants()->version())); + _cprefs->short_at_put(_index, method->name_index()); // We need to save the mirrors in the backtrace to keep the class // from being unloaded while we still have this stack trace. @@ -1419,27 +1434,26 @@ class BacktraceBuilder: public StackObj { // Print stack trace element to resource allocated buffer char* java_lang_Throwable::print_stack_element_to_buffer(Handle mirror, - int method_id, int version, int bci) { + int method_id, int version, int bci, int cpref) { // Get strings and string lengths InstanceKlass* holder = InstanceKlass::cast(java_lang_Class::as_Klass(mirror())); const char* klass_name = holder->external_name(); int buf_len = (int)strlen(klass_name); - // The method id may point to an obsolete method, can't get more stack information - Method* method = holder->method_with_idnum(method_id); - if (method == NULL) { - char* buf = NEW_RESOURCE_ARRAY(char, buf_len + 64); - // This is what the java code prints in this case - added Redefined - sprintf(buf, "\tat %s.null (Redefined)", klass_name); - return buf; - } + Method* method = holder->method_with_orig_idnum(method_id, version); - char* method_name = method->name()->as_C_string(); + // 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(); buf_len += (int)strlen(method_name); + // Use a specific ik version as a holder since the mirror might + // refer to a version that is now obsolete and no longer accessible + // via the previous versions list. + holder = holder->get_klass_version(version); char* source_file_name = NULL; - if (version_matches(method, version)) { + if (holder != NULL) { Symbol* source = holder->source_file_name(); if (source != NULL) { source_file_name = source->as_C_string(); @@ -1481,17 +1495,18 @@ char* java_lang_Throwable::print_stack_element_to_buffer(Handle mirror, } void java_lang_Throwable::print_stack_element(outputStream *st, Handle mirror, - int method_id, int version, int bci) { + int method_id, int version, int bci, int cpref) { ResourceMark rm; - char* buf = print_stack_element_to_buffer(mirror, method_id, version, bci); + char* buf = print_stack_element_to_buffer(mirror, method_id, version, bci, cpref); st->print_cr("%s", buf); } void java_lang_Throwable::print_stack_element(outputStream *st, methodHandle method, int bci) { Handle mirror = method->method_holder()->java_mirror(); - int method_id = method->method_idnum(); + int method_id = method->orig_method_idnum(); int version = method->constants()->version(); - print_stack_element(st, mirror, method_id, version, bci); + int cpref = method->name_index(); + print_stack_element(st, mirror, method_id, version, bci, cpref); } const char* java_lang_Throwable::no_stack_trace_message() { @@ -1516,6 +1531,7 @@ void java_lang_Throwable::print_stack_trace(oop throwable, outputStream* st) { typeArrayHandle methods (THREAD, BacktraceBuilder::get_methods(result)); typeArrayHandle bcis (THREAD, BacktraceBuilder::get_bcis(result)); objArrayHandle mirrors (THREAD, BacktraceBuilder::get_mirrors(result)); + typeArrayHandle cprefs (THREAD, BacktraceBuilder::get_cprefs(result)); int length = methods()->length(); for (int index = 0; index < length; index++) { @@ -1525,7 +1541,8 @@ void java_lang_Throwable::print_stack_trace(oop throwable, outputStream* st) { int method = methods->short_at(index); int version = version_at(bcis->int_at(index)); int bci = bci_at(bcis->int_at(index)); - print_stack_element(st, mirror, method, version, bci); + int cpref = cprefs->short_at(index); + print_stack_element(st, mirror, method, version, bci, cpref); } result = objArrayHandle(THREAD, objArrayOop(result->obj_at(trace_next_offset))); } @@ -1809,29 +1826,30 @@ oop java_lang_Throwable::get_stack_trace_element(oop throwable, int index, TRAPS if (chunk == NULL) { THROW_(vmSymbols::java_lang_IndexOutOfBoundsException(), NULL); } - // Get method id, bci, version and mirror from chunk + // Get method id, bci, version, mirror and cpref from chunk typeArrayOop methods = BacktraceBuilder::get_methods(chunk); typeArrayOop bcis = BacktraceBuilder::get_bcis(chunk); objArrayOop mirrors = BacktraceBuilder::get_mirrors(chunk); + typeArrayOop cprefs = BacktraceBuilder::get_cprefs(chunk); assert(methods != NULL && bcis != NULL && mirrors != NULL, "sanity check"); int method = methods->short_at(chunk_index); int version = version_at(bcis->int_at(chunk_index)); int bci = bci_at(bcis->int_at(chunk_index)); + int cpref = cprefs->short_at(chunk_index); Handle mirror(THREAD, mirrors->obj_at(chunk_index)); // Chunk can be partial full if (mirror.is_null()) { THROW_(vmSymbols::java_lang_IndexOutOfBoundsException(), NULL); } - - oop element = java_lang_StackTraceElement::create(mirror, method, version, bci, CHECK_0); + oop element = java_lang_StackTraceElement::create(mirror, method, version, bci, cpref, CHECK_0); return element; } oop java_lang_StackTraceElement::create(Handle mirror, int method_id, - int version, int bci, TRAPS) { + int version, int bci, int cpref, TRAPS) { // Allocate java.lang.StackTraceElement instance Klass* k = SystemDictionary::StackTraceElement_klass(); assert(k != NULL, "must be loaded in 1.4+"); @@ -1848,17 +1866,13 @@ oop java_lang_StackTraceElement::create(Handle mirror, int method_id, oop classname = StringTable::intern((char*) str, CHECK_0); java_lang_StackTraceElement::set_declaringClass(element(), classname); - Method* method = holder->method_with_idnum(method_id); - // Method on stack may be obsolete because it was redefined so cannot be - // found by idnum. - if (method == NULL) { - // leave name and fileName null - java_lang_StackTraceElement::set_lineNumber(element(), -1); - return element(); - } + 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); // Fill in method name - oop methodname = StringTable::intern(method->name(), CHECK_0); + oop methodname = StringTable::intern(sym, CHECK_0); java_lang_StackTraceElement::set_methodName(element(), methodname); if (!version_matches(method, version)) { @@ -1867,6 +1881,11 @@ oop java_lang_StackTraceElement::create(Handle mirror, int method_id, java_lang_StackTraceElement::set_lineNumber(element(), -1); } else { // Fill in source file name and line number. + // Use a specific ik version as a holder since the mirror might + // refer to a version that is now obsolete and no longer accessible + // via the previous versions list. + holder = holder->get_klass_version(version); + assert(holder != NULL, "sanity check"); Symbol* source = holder->source_file_name(); if (ShowHiddenFrames && source == NULL) source = vmSymbols::unknown_class_name(); @@ -1881,8 +1900,9 @@ oop java_lang_StackTraceElement::create(Handle mirror, int method_id, oop java_lang_StackTraceElement::create(methodHandle method, int bci, TRAPS) { Handle mirror (THREAD, method->method_holder()->java_mirror()); - int method_id = method->method_idnum(); - return create(mirror, method_id, method->constants()->version(), bci, THREAD); + int method_id = method->orig_method_idnum(); + int cpref = method->name_index(); + return create(mirror, method_id, method->constants()->version(), bci, cpref, THREAD); } void java_lang_reflect_AccessibleObject::compute_offsets() { diff --git a/src/share/vm/classfile/javaClasses.hpp b/src/share/vm/classfile/javaClasses.hpp index 7b041ee22..5d489c7c4 100644 --- a/src/share/vm/classfile/javaClasses.hpp +++ b/src/share/vm/classfile/javaClasses.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2014, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 2015, 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 @@ -484,8 +484,9 @@ class java_lang_Throwable: AllStatic { trace_methods_offset = 0, trace_bcis_offset = 1, trace_mirrors_offset = 2, - trace_next_offset = 3, - trace_size = 4, + trace_cprefs_offset = 3, + trace_next_offset = 4, + trace_size = 5, trace_chunk_size = 32 }; @@ -496,7 +497,7 @@ class java_lang_Throwable: AllStatic { static int static_unassigned_stacktrace_offset; // Printing - static char* print_stack_element_to_buffer(Handle mirror, int method, int version, int bci); + static char* print_stack_element_to_buffer(Handle mirror, int method, int version, int bci, int cpref); // StackTrace (programmatic access, new since 1.4) static void clear_stacktrace(oop throwable); // No stack trace available @@ -517,7 +518,7 @@ class java_lang_Throwable: AllStatic { static oop message(Handle throwable); static void set_message(oop throwable, oop value); static void print_stack_element(outputStream *st, Handle mirror, int method, - int version, int bci); + int version, int bci, int cpref); static void print_stack_element(outputStream *st, methodHandle method, int bci); static void print_stack_usage(Handle stream); @@ -1326,7 +1327,7 @@ class java_lang_StackTraceElement: AllStatic { static void set_lineNumber(oop element, int value); // Create an instance of StackTraceElement - static oop create(Handle mirror, int method, int version, int bci, TRAPS); + static oop create(Handle mirror, int method, int version, int bci, int cpref, TRAPS); static oop create(methodHandle method, int bci, TRAPS); // Debugging diff --git a/src/share/vm/oops/instanceKlass.cpp b/src/share/vm/oops/instanceKlass.cpp index e751304d7..16ed65584 100644 --- a/src/share/vm/oops/instanceKlass.cpp +++ b/src/share/vm/oops/instanceKlass.cpp @@ -3747,6 +3747,22 @@ bool InstanceKlass::has_previous_version() const { } // end has_previous_version() +InstanceKlass* InstanceKlass::get_klass_version(int version) { + if (constants()->version() == version) { + return this; + } + PreviousVersionWalker pvw(Thread::current(), (InstanceKlass*)this); + for (PreviousVersionNode * pv_node = pvw.next_previous_version(); + pv_node != NULL; pv_node = pvw.next_previous_version()) { + ConstantPool* prev_cp = pv_node->prev_constant_pool(); + if (prev_cp->version() == version) { + return prev_cp->pool_holder(); + } + } + return NULL; // None found +} + + Method* InstanceKlass::method_with_idnum(int idnum) { Method* m = NULL; if (idnum < methods()->length()) { @@ -3765,6 +3781,37 @@ Method* InstanceKlass::method_with_idnum(int idnum) { return m; } + +Method* InstanceKlass::method_with_orig_idnum(int idnum) { + if (idnum >= methods()->length()) { + return NULL; + } + Method* m = methods()->at(idnum); + if (m != NULL && m->orig_method_idnum() == idnum) { + return m; + } + // Obsolete method idnum does not match the original idnum + for (int index = 0; index < methods()->length(); ++index) { + m = methods()->at(index); + if (m->orig_method_idnum() == idnum) { + return m; + } + } + // None found, return null for the caller to handle. + return NULL; +} + + +Method* InstanceKlass::method_with_orig_idnum(int idnum, int version) { + InstanceKlass* holder = get_klass_version(version); + if (holder == NULL) { + return NULL; // The version of klass is gone, no method is found + } + Method* method = holder->method_with_orig_idnum(idnum); + return method; +} + + jint InstanceKlass::get_cached_class_file_len() { return VM_RedefineClasses::get_cached_class_file_len(_cached_class_file); } diff --git a/src/share/vm/oops/instanceKlass.hpp b/src/share/vm/oops/instanceKlass.hpp index b4ff42768..18b0ebec2 100644 --- a/src/share/vm/oops/instanceKlass.hpp +++ b/src/share/vm/oops/instanceKlass.hpp @@ -358,6 +358,8 @@ class InstanceKlass: public Klass { Array* methods() const { return _methods; } void set_methods(Array* a) { _methods = a; } Method* method_with_idnum(int idnum); + Method* method_with_orig_idnum(int idnum); + Method* method_with_orig_idnum(int idnum, int version); // method ordering Array* method_ordering() const { return _method_ordering; } @@ -658,6 +660,7 @@ class InstanceKlass: public Klass { return _previous_versions; } + InstanceKlass* get_klass_version(int version); static void purge_previous_versions(InstanceKlass* ik); // JVMTI: Support for caching a class file before it is modified by an agent that can do retransformation -- GitLab