Skip to content
Permalink
Browse files
8255208: CodeStrings passed to Disassembler::decode are ignored
Reviewed-by: kvn, iklam
  • Loading branch information
cl4es committed Oct 23, 2020
1 parent 8e5dff0 commit c1524c59adb8d150aaedd0552fae0444dd2e638f
Show file tree
Hide file tree
Showing 11 changed files with 118 additions and 204 deletions.
@@ -140,7 +140,7 @@ CodeBuffer::~CodeBuffer() {

// 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();
NOT_PRODUCT(free_strings();)

#ifdef ASSERT
// Save allocation type to execute assert in ~ResourceObj()
@@ -267,13 +267,14 @@ bool CodeBuffer::is_backward_branch(Label& L) {
return L.is_bound() && insts_end() <= locator_address(L.loc());
}

#ifndef PRODUCT
address CodeBuffer::decode_begin() {
address begin = _insts.start();
if (_decode_begin != NULL && _decode_begin > begin)
begin = _decode_begin;
return begin;
}

#endif // !PRODUCT

GrowableArray<int>* CodeBuffer::create_patch_overflow() {
if (_overflow_arena == NULL) {
@@ -752,7 +753,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(_code_strings);
NOT_PRODUCT(dest_blob->set_strings(_code_strings);)

// Done moving code bytes; were they the right size?
assert((int)align_up(dest.total_content_size(), oopSize) == dest_blob->content_size(), "sanity");
@@ -957,12 +958,11 @@ void CodeBuffer::expand(CodeSection* which_cs, csize_t amount) {
debug_only(Copy::fill_to_bytes(bxp->_total_start, bxp->_total_size,
badCodeHeapFreeVal));

_decode_begin = NULL; // sanity

// Make certain that the new sections are all snugly inside the new blob.
verify_section_allocation();

#ifndef PRODUCT
_decode_begin = NULL; // sanity
if (PrintNMethods && (WizardMode || Verbose)) {
tty->print("expanded CodeBuffer:");
this->print();
@@ -1032,10 +1032,6 @@ void CodeBuffer::log_section_sizes(const char* name) {

#ifndef PRODUCT

void CodeSection::decode() {
Disassembler::decode(start(), end());
}

void CodeBuffer::block_comment(intptr_t offset, const char * comment) {
if (_collect_comments) {
_code_strings.add_comment(offset, comment);
@@ -1054,8 +1050,12 @@ class CodeString: public CHeapObj<mtCode> {
CodeString* _prev;
intptr_t _offset;

static long allocated_code_strings;

~CodeString() {
assert(_next == NULL && _prev == NULL, "wrong interface for freeing list");
allocated_code_strings--;
log_trace(codestrings)("Freeing CodeString [%s] (%p)", _string, (void*)_string);
os::free((void*)_string);
}

@@ -1064,12 +1064,14 @@ class CodeString: public CHeapObj<mtCode> {
public:
CodeString(const char * string, intptr_t offset = -1)
: _next(NULL), _prev(NULL), _offset(offset) {
allocated_code_strings++;
_string = os::strdup(string, mtCode);
log_trace(codestrings)("Created CodeString [%s] (%p)", _string, (void*)_string);
}

const char * string() const { return _string; }
intptr_t offset() const { assert(_offset >= 0, "offset for non comment?"); return _offset; }
CodeString* next() const { return _next; }
CodeString* next() const { return _next; }

void set_next(CodeString* next) {
_next = next;
@@ -1094,6 +1096,10 @@ class CodeString: public CHeapObj<mtCode> {
}
};

// For tracing statistics. Will use raw increment/decrement, so it might not be
// exact
long CodeString::allocated_code_strings = 0;

CodeString* CodeStrings::find(intptr_t offset) const {
CodeString* a = _strings->first_comment();
while (a != NULL && a->offset() != offset) {
@@ -1116,7 +1122,7 @@ void CodeStrings::add_comment(intptr_t offset, const char * comment) {
CodeString* c = new CodeString(comment, offset);
CodeString* inspos = (_strings == NULL) ? NULL : find_last(offset);

if (inspos) {
if (inspos != NULL) {
// insert after already existing comments with same offset
c->set_next(inspos->next());
inspos->set_next(c);
@@ -1130,29 +1136,22 @@ void CodeStrings::add_comment(intptr_t offset, const char * comment) {
}
}

void CodeStrings::assign(CodeStrings& other) {
other.check_valid();
assert(is_null(), "Cannot assign onto non-empty CodeStrings");
_strings = other._strings;
_strings_last = other._strings_last;
#ifdef ASSERT
_defunct = false;
#endif
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) {
log_debug(codestrings)("Copying %d Codestring(s)", other.count());

other.check_valid();
check_valid();
assert(is_null(), "Cannot copy onto non-empty CodeStrings");
CodeString* n = other._strings;
CodeString** ps = &_strings;
CodeString* prev = NULL;
while (n != NULL) {
*ps = new CodeString(n->string(),n->offset());
if (n->is_comment()) {
*ps = new CodeString(n->string(), n->offset());
} else {
*ps = new CodeString(n->string());
}
(*ps)->_prev = prev;
prev = *ps;
ps = &((*ps)->_next);
@@ -1162,13 +1161,6 @@ void CodeStrings::copy(CodeStrings& other) {

const char* CodeStrings::_prefix = " ;; "; // default: can be changed via set_prefix

// Check if any block comments are pending for the given offset.
bool CodeStrings::has_block_comment(intptr_t offset) const {
if (_strings == NULL) return false;
CodeString* c = find(offset);
return c != NULL;
}

void CodeStrings::print_block_comment(outputStream* stream, intptr_t offset) const {
check_valid();
if (_strings != NULL) {
@@ -1184,8 +1176,19 @@ void CodeStrings::print_block_comment(outputStream* stream, intptr_t offset) con
}
}

// Also sets isNull()
int CodeStrings::count() const {
int i = 0;
CodeString* s = _strings;
while (s != NULL) {
i++;
s = s->_next;
}
return i;
}

// Also sets is_null()
void CodeStrings::free() {
log_debug(codestrings)("Freeing %d out of approx. %ld CodeString(s), ", count(), CodeString::allocated_code_strings);
CodeString* n = _strings;
while (n) {
// unlink the node from the list saving a pointer to the next
@@ -1215,7 +1218,7 @@ const char* CodeStrings::add_string(const char * string) {

void CodeBuffer::decode() {
ttyLocker ttyl;
Disassembler::decode(decode_begin(), insts_end(), tty);
Disassembler::decode(decode_begin(), insts_end(), tty NOT_PRODUCT(COMMA &strings()));
_decode_begin = insts_end();
}

@@ -1246,10 +1249,4 @@ void CodeBuffer::print() {
}
}

// Directly disassemble code buffer.
void CodeBuffer::decode(address start, address end) {
ttyLocker ttyl;
Disassembler::decode(this, start, end, tty);
}

#endif // PRODUCT
@@ -284,20 +284,18 @@ class CodeStrings {
bool _defunct; // Zero bit pattern is "valid", see memset call in decode_env::decode_env
#endif
static const char* _prefix; // defaults to " ;; "
#endif

CodeString* find(intptr_t offset) const;
CodeString* find_last(intptr_t offset) const;

void set_null_and_invalidate() {
#ifndef PRODUCT
_strings = NULL;
_strings_last = NULL;
#ifdef ASSERT
_defunct = true;
#endif
#endif
}
#endif

public:
CodeStrings() {
@@ -310,6 +308,7 @@ class CodeStrings {
#endif
}

#ifndef PRODUCT
bool is_null() {
#ifdef ASSERT
return _strings == NULL;
@@ -318,30 +317,25 @@ class CodeStrings {
#endif
}

const char* add_string(const char * string) PRODUCT_RETURN_(return NULL;);
const char* add_string(const char * string);

void add_comment(intptr_t offset, const char * comment) PRODUCT_RETURN;
bool has_block_comment(intptr_t offset) const;
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;
void add_comment(intptr_t offset, const char * comment);
void print_block_comment(outputStream* stream, intptr_t offset) const;
int count() const;
// COPY strings from other to this; leave other valid.
void copy(CodeStrings& other) PRODUCT_RETURN;
void copy(CodeStrings& other);
// FREE strings; invalidate this.
void free() PRODUCT_RETURN;
void free();

// Guarantee that _strings are used at most once; assign and free invalidate a buffer.
inline void check_valid() const {
#ifdef ASSERT
assert(!_defunct, "Use of invalid CodeStrings");
#endif
}

static void set_prefix(const char *prefix) {
#ifndef PRODUCT
_prefix = prefix;
#endif
}
#endif // !PRODUCT
};

// A CodeBuffer describes a memory space into which assembly
@@ -410,8 +404,7 @@ class CodeBuffer: public StackObj {
csize_t _total_size; // size in bytes of combined memory buffer

OopRecorder* _oop_recorder;
CodeStrings _code_strings;
bool _collect_comments; // Indicate if we need to collect block comments at all.

OopRecorder _default_oop_recorder; // override with initialize_oop_recorder
Arena* _overflow_arena;

@@ -421,8 +414,12 @@ class CodeBuffer: public StackObj {
bool _immutable_PIC;
#endif

address _decode_begin; // start address for decode
#ifndef PRODUCT
CodeStrings _code_strings;
bool _collect_comments; // Indicate if we need to collect block comments at all.
address _decode_begin; // start address for decode
address decode_begin();
#endif

void initialize_misc(const char * name) {
// all pointers other than code_start/end and those inside the sections
@@ -431,14 +428,15 @@ class CodeBuffer: public StackObj {
_before_expand = NULL;
_blob = NULL;
_oop_recorder = NULL;
_decode_begin = NULL;
_overflow_arena = NULL;
_code_strings = CodeStrings();
_last_insn = NULL;
#if INCLUDE_AOT
_immutable_PIC = false;
#endif

#ifndef PRODUCT
_decode_begin = NULL;
_code_strings = CodeStrings();
// Collect block comments, but restrict collection to cases where a disassembly is output.
_collect_comments = ( PrintAssembly
|| PrintStubCode
@@ -447,6 +445,7 @@ class CodeBuffer: public StackObj {
|| PrintSignatureHandlers
|| UnlockDiagnosticVMOptions
);
#endif
}

void initialize(address code_start, csize_t code_size) {
@@ -635,35 +634,27 @@ class CodeBuffer: public StackObj {
// Override default oop recorder.
void initialize_oop_recorder(OopRecorder* r);

OopRecorder* oop_recorder() const { return _oop_recorder; }
CodeStrings& strings() { return _code_strings; }
OopRecorder* oop_recorder() const { return _oop_recorder; }

address last_insn() const { return _last_insn; }
void set_last_insn(address a) { _last_insn = a; }
void clear_last_insn() { set_last_insn(NULL); }

#ifndef PRODUCT
CodeStrings& strings() { return _code_strings; }

void free_strings() {
if (!_code_strings.is_null()) {
_code_strings.free(); // sets _strings Null as a side-effect.
}
}

// Directly disassemble code buffer.
// Print the comment associated with offset on stream, if there is one.
virtual void print_block_comment(outputStream* stream, address block_begin) {
#ifndef PRODUCT
intptr_t offset = (intptr_t)(block_begin - _total_start); // I assume total_start is not correct for all code sections.
_code_strings.print_block_comment(stream, offset);
#endif
}
bool has_block_comment(address block_begin) {
#ifndef PRODUCT
intptr_t offset = (intptr_t)(block_begin - _total_start); // I assume total_start is not correct for all code sections.
return _code_strings.has_block_comment(offset);
#else
return false;
#endif
}

// Code generation
void relocate(address at, RelocationHolder const& rspec, int format = 0) {
@@ -87,8 +87,8 @@ CodeBlob::CodeBlob(const char* name, CompilerType type, const CodeBlobLayout& la
_relocation_end(layout.relocation_end()),
_oop_maps(oop_maps),
_caller_must_gc_arguments(caller_must_gc_arguments),
_strings(CodeStrings()),
_name(name)
NOT_PRODUCT(COMMA _strings(CodeStrings()))
{
assert(is_aligned(layout.size(), oopSize), "unaligned size");
assert(is_aligned(layout.header_size(), oopSize), "unaligned size");
@@ -115,8 +115,8 @@ CodeBlob::CodeBlob(const char* name, CompilerType type, const CodeBlobLayout& la
_relocation_begin(layout.relocation_begin()),
_relocation_end(layout.relocation_end()),
_caller_must_gc_arguments(caller_must_gc_arguments),
_strings(CodeStrings()),
_name(name)
NOT_PRODUCT(COMMA _strings(CodeStrings()))
{
assert(is_aligned(_size, oopSize), "unaligned size");
assert(is_aligned(_header_size, oopSize), "unaligned size");
@@ -158,7 +158,7 @@ RuntimeBlob::RuntimeBlob(
void CodeBlob::flush() {
FREE_C_HEAP_ARRAY(unsigned char, _oop_maps);
_oop_maps = NULL;
_strings.free();
NOT_PRODUCT(_strings.free();)
}

void CodeBlob::set_oop_maps(OopMapSet* p) {

1 comment on commit c1524c5

@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented on c1524c5 Oct 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.