diff --git a/src/share/vm/asm/codeBuffer.cpp b/src/share/vm/asm/codeBuffer.cpp index 24a3bbdfa60d169b2cbc35369f365a6eee1fe2d2..37180da35a53ef187dd2bbde6d92f59b9f7ef6b4 100644 --- a/src/share/vm/asm/codeBuffer.cpp +++ b/src/share/vm/asm/codeBuffer.cpp @@ -133,6 +133,10 @@ CodeBuffer::~CodeBuffer() { // free any overflow storage delete _overflow_arena; + // Claim is that stack allocation ensures resources are cleaned up. + // This is resource clean up, let's hope that all were properly copied out. + free_strings(); + #ifdef ASSERT // Save allocation type to execute assert in ~ResourceObj() // which is called after this destructor. @@ -704,7 +708,7 @@ void CodeBuffer::copy_code_to(CodeBlob* dest_blob) { relocate_code_to(&dest); // transfer strings and comments from buffer to blob - dest_blob->set_strings(_strings); + dest_blob->set_strings(_code_strings); // Done moving code bytes; were they the right size? assert(round_to(dest.total_content_size(), oopSize) == dest_blob->content_size(), "sanity"); @@ -1003,11 +1007,11 @@ void CodeSection::decode() { void CodeBuffer::block_comment(intptr_t offset, const char * comment) { - _strings.add_comment(offset, comment); + _code_strings.add_comment(offset, comment); } const char* CodeBuffer::code_string(const char* str) { - return _strings.add_string(str); + return _code_strings.add_string(str); } class CodeString: public CHeapObj { @@ -1073,6 +1077,7 @@ CodeString* CodeStrings::find_last(intptr_t offset) const { } void CodeStrings::add_comment(intptr_t offset, const char * comment) { + check_valid(); CodeString* c = new CodeString(comment, offset); CodeString* inspos = (_strings == NULL) ? NULL : find_last(offset); @@ -1088,11 +1093,32 @@ void CodeStrings::add_comment(intptr_t offset, const char * comment) { } void CodeStrings::assign(CodeStrings& other) { + other.check_valid(); + // Cannot do following because CodeStrings constructor is not alway run! + assert(is_null(), "Cannot assign onto non-empty CodeStrings"); _strings = other._strings; + other.set_null_and_invalidate(); +} + +// Deep copy of CodeStrings for consistent memory management. +// Only used for actual disassembly so this is cheaper than reference counting +// for the "normal" fastdebug case. +void CodeStrings::copy(CodeStrings& other) { + other.check_valid(); + check_valid(); + assert(is_null(), "Cannot copy onto non-empty CodeStrings"); + CodeString* n = other._strings; + CodeString** ps = &_strings; + while (n != NULL) { + *ps = new CodeString(n->string(),n->offset()); + ps = &((*ps)->_next); + n = n->next(); + } } void CodeStrings::print_block_comment(outputStream* stream, intptr_t offset) const { - if (_strings != NULL) { + check_valid(); + if (_strings != NULL) { CodeString* c = find(offset); while (c && c->offset() == offset) { stream->bol(); @@ -1103,7 +1129,7 @@ void CodeStrings::print_block_comment(outputStream* stream, intptr_t offset) con } } - +// Also sets isNull() void CodeStrings::free() { CodeString* n = _strings; while (n) { @@ -1113,10 +1139,11 @@ void CodeStrings::free() { delete n; n = p; } - _strings = NULL; + set_null_and_invalidate(); } const char* CodeStrings::add_string(const char * string) { + check_valid(); CodeString* s = new CodeString(string); s->set_next(_strings); _strings = s; diff --git a/src/share/vm/asm/codeBuffer.hpp b/src/share/vm/asm/codeBuffer.hpp index 1c3761baa4155b1ad719e68a3462893b1f17bbda..dcdb6903c786c452496702ee7869afb840b79ac5 100644 --- a/src/share/vm/asm/codeBuffer.hpp +++ b/src/share/vm/asm/codeBuffer.hpp @@ -27,6 +27,7 @@ #include "code/oopRecorder.hpp" #include "code/relocInfo.hpp" +#include "utilities/debug.hpp" class CodeStrings; class PhaseCFG; @@ -245,15 +246,39 @@ class CodeStrings VALUE_OBJ_CLASS_SPEC { private: #ifndef PRODUCT CodeString* _strings; +#ifdef ASSERT + // Becomes true after copy-out, forbids further use. + bool _defunct; // Zero bit pattern is "valid", see memset call in decode_env::decode_env +#endif #endif CodeString* find(intptr_t offset) const; CodeString* find_last(intptr_t offset) const; + void set_null_and_invalidate() { +#ifndef PRODUCT + _strings = NULL; +#ifdef ASSERT + _defunct = true; +#endif +#endif + } + public: CodeStrings() { #ifndef PRODUCT _strings = NULL; +#ifdef ASSERT + _defunct = false; +#endif +#endif + } + + bool is_null() { +#ifdef ASSERT + return _strings == NULL; +#else + return true; #endif } @@ -261,8 +286,17 @@ public: void add_comment(intptr_t offset, const char * comment) PRODUCT_RETURN; void print_block_comment(outputStream* stream, intptr_t offset) const PRODUCT_RETURN; + // MOVE strings from other to this; invalidate other. void assign(CodeStrings& other) PRODUCT_RETURN; + // COPY strings from other to this; leave other valid. + void copy(CodeStrings& other) PRODUCT_RETURN; void free() PRODUCT_RETURN; + // Guarantee that _strings are used at most once; assign invalidates a buffer. + inline void check_valid() const { +#ifdef ASSERT + assert(!_defunct, "Use of invalid CodeStrings"); +#endif + } }; // A CodeBuffer describes a memory space into which assembly @@ -330,7 +364,7 @@ class CodeBuffer: public StackObj { csize_t _total_size; // size in bytes of combined memory buffer OopRecorder* _oop_recorder; - CodeStrings _strings; + CodeStrings _code_strings; OopRecorder _default_oop_recorder; // override with initialize_oop_recorder Arena* _overflow_arena; @@ -531,7 +565,13 @@ class CodeBuffer: public StackObj { void initialize_oop_recorder(OopRecorder* r); OopRecorder* oop_recorder() const { return _oop_recorder; } - CodeStrings& strings() { return _strings; } + CodeStrings& strings() { return _code_strings; } + + void free_strings() { + if (!_code_strings.is_null()) { + _code_strings.free(); // sets _strings Null as a side-effect. + } + } // Code generation void relocate(address at, RelocationHolder const& rspec, int format = 0) { diff --git a/src/share/vm/code/codeBlob.cpp b/src/share/vm/code/codeBlob.cpp index f9e78bfb9e2d869409b2d316b917c8e01eec21dc..ee0d17de2ca1524617845b5f4a73b495d1ff2cb8 100644 --- a/src/share/vm/code/codeBlob.cpp +++ b/src/share/vm/code/codeBlob.cpp @@ -253,6 +253,7 @@ void* BufferBlob::operator new(size_t s, unsigned size, bool is_critical) throw( void BufferBlob::free( BufferBlob *blob ) { ThreadInVMfromUnknown __tiv; // get to VM state in case we block on CodeCache_lock + blob->flush(); { MutexLockerEx mu(CodeCache_lock, Mutex::_no_safepoint_check_flag); CodeCache::free((CodeBlob*)blob); diff --git a/src/share/vm/compiler/disassembler.cpp b/src/share/vm/compiler/disassembler.cpp index 0e9c14c1549792bf5bac3b0ce00bd31d085ec54a..3291360638f30f91030c3b57fe4987e21637efcd 100644 --- a/src/share/vm/compiler/disassembler.cpp +++ b/src/share/vm/compiler/disassembler.cpp @@ -245,12 +245,12 @@ class decode_env { }; decode_env::decode_env(CodeBlob* code, outputStream* output, CodeStrings c) { - memset(this, 0, sizeof(*this)); + memset(this, 0, sizeof(*this)); // Beware, this zeroes bits of fields. _output = output ? output : tty; _code = code; if (code != NULL && code->is_nmethod()) _nm = (nmethod*) code; - _strings.assign(c); + _strings.copy(c); // by default, output pc but not bytes: _print_pc = true; diff --git a/src/share/vm/interpreter/interpreter.hpp b/src/share/vm/interpreter/interpreter.hpp index cc26b378d73f7a870864265b3991abfde6029908..d3792f8477dcc964123e65c83dfa450e72f153ed 100644 --- a/src/share/vm/interpreter/interpreter.hpp +++ b/src/share/vm/interpreter/interpreter.hpp @@ -53,7 +53,9 @@ class InterpreterCodelet: public Stub { public: // Initialization/finalization void initialize(int size, - CodeStrings& strings) { _size = size; DEBUG_ONLY(_strings.assign(strings);) } + CodeStrings& strings) { _size = size; + DEBUG_ONLY(::new(&_strings) CodeStrings();) + DEBUG_ONLY(_strings.assign(strings);) } void finalize() { ShouldNotCallThis(); } // General info/converters