8264207: CodeStrings does not honour fixed address assumption.

Reviewed-by: redestad, neliasso

Author: Patric Hedlin

src/hotspot/share/asm/codeBuffer.cpp

@@ -31,7 +31,6 @@
 #include "utilities/debug.hpp"
 #include "utilities/macros.hpp"
 
-class CodeStrings;
 class PhaseCFG;
 class Compile;
 class BufferBlob;
@@ -273,70 +272,75 @@ class CodeSection {
 #endif //PRODUCT
 };
 
-class CodeString;
-class CodeStrings {
-private:
+
 #ifndef PRODUCT
-  CodeString* _strings;
-  CodeString* _strings_last;
-#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
-  static const char* _prefix; // defaults to " ;; "
 
-  CodeString* find(intptr_t offset) const;
-  CodeString* find_last(intptr_t offset) const;
+class AsmRemarkCollection;
+class DbgStringCollection;
 
-  void set_null_and_invalidate() {
-    _strings = NULL;
-    _strings_last = NULL;
-#ifdef ASSERT
-    _defunct = true;
-#endif
-  }
-#endif
+// The assumption made here is that most code remarks (or comments) added to
+// the generated assembly code are unique, i.e. there is very little gain in
+// trying to share the strings between the different offsets tracked in a
+// buffer (or blob).
 
-public:
-  CodeStrings() {
-#ifndef PRODUCT
-    _strings = NULL;
-    _strings_last = NULL;
-#ifdef ASSERT
-    _defunct = false;
-#endif
-#endif
-  }
+class AsmRemarks {
+ public:
+  AsmRemarks();
+ ~AsmRemarks();
 
-#ifndef PRODUCT
-  bool is_null() {
-#ifdef ASSERT
-    return _strings == NULL;
-#else
-    return true;
-#endif
-  }
+  const char* insert(uint offset, const char* remstr);
 
-  const char* add_string(const char * string);
+  bool is_empty() const;
 
-  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);
-  // FREE strings; invalidate this.
-  void free();
+  void share(const AsmRemarks &src);
+  void clear();
+  uint print(uint offset, outputStream* strm = tty) const;
 
-  // Guarantee that _strings are used at most once; assign and free invalidate a buffer.
-  inline void check_valid() const {
-    assert(!_defunct, "Use of invalid CodeStrings");
-  }
+  // For testing purposes only.
+  const AsmRemarkCollection* ref() const { return _remarks; }
 
-  static void set_prefix(const char *prefix) {
-    _prefix = prefix;
+private:
+  AsmRemarkCollection* _remarks;
+};
+
+// The assumption made here is that the number of debug strings (with a fixed
+// address requirement) is a rather small set per compilation unit.
+
+class DbgStrings {
+ public:
+  DbgStrings();
+ ~DbgStrings();
+
+  const char* insert(const char* dbgstr);
+
+  bool is_empty() const;
+
+  void share(const DbgStrings &src);
+  void clear();
+
+  // For testing purposes only.
+  const DbgStringCollection* ref() const { return _strings; }
+
+private:
+  DbgStringCollection* _strings;
+};
+#endif // not PRODUCT
+
+
+#ifdef ASSERT
+#include "utilities/copy.hpp"
+
+class Scrubber {
+ public:
+  Scrubber(void* addr, size_t size) : _addr(addr), _size(size) {}
+ ~Scrubber() {
+    Copy::fill_to_bytes(_addr, _size, badResourceValue);
   }
-#endif // !PRODUCT
+ private:
+  void*  _addr;
+  size_t _size;
 };
+#endif // ASSERT
 
 // A CodeBuffer describes a memory space into which assembly
 // code is generated.  This memory space usually occupies the
@@ -362,7 +366,7 @@ class CodeStrings {
 // Instructions and data in one section can contain relocatable references to
 // addresses in a sibling section.
 
-class CodeBuffer: public StackObj {
+class CodeBuffer: public StackObj DEBUG_ONLY(COMMA private Scrubber) {
   friend class CodeSection;
   friend class StubCodeGenerator;
 
@@ -411,7 +415,8 @@ class CodeBuffer: public StackObj {
   address      _last_insn;      // used to merge consecutive memory barriers, loads or stores.
 
 #ifndef PRODUCT
-  CodeStrings  _code_strings;
+  AsmRemarks   _asm_remarks;
+  DbgStrings   _dbg_strings;
   bool         _collect_comments; // Indicate if we need to collect block comments at all.
   address      _decode_begin;     // start address for decode
   address      decode_begin();
@@ -429,7 +434,6 @@ class CodeBuffer: public StackObj {
 
 #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
@@ -484,7 +488,9 @@ class CodeBuffer: public StackObj {
 
  public:
   // (1) code buffer referring to pre-allocated instruction memory
-  CodeBuffer(address code_start, csize_t code_size) {
+  CodeBuffer(address code_start, csize_t code_size)
+    DEBUG_ONLY(: Scrubber(this, sizeof(*this)))
+  {
     assert(code_start != NULL, "sanity");
     initialize_misc("static buffer");
     initialize(code_start, code_size);
@@ -497,14 +503,18 @@ class CodeBuffer: public StackObj {
   // (3) code buffer allocating codeBlob memory for code & relocation
   // info but with lazy initialization.  The name must be something
   // informative.
-  CodeBuffer(const char* name) {
+  CodeBuffer(const char* name)
+    DEBUG_ONLY(: Scrubber(this, sizeof(*this)))
+  {
     initialize_misc(name);
   }
 
   // (4) code buffer allocating codeBlob memory for code & relocation
   // info.  The name must be something informative and code_size must
   // include both code and stubs sizes.
-  CodeBuffer(const char* name, csize_t code_size, csize_t locs_size) {
+  CodeBuffer(const char* name, csize_t code_size, csize_t locs_size)
+    DEBUG_ONLY(: Scrubber(this, sizeof(*this)))
+  {
     initialize_misc(name);
     initialize(code_size, locs_size);
   }
@@ -634,12 +644,12 @@ class CodeBuffer: public StackObj {
   void clear_last_insn() { set_last_insn(NULL); }
 
 #ifndef PRODUCT
-  CodeStrings& strings() { return _code_strings; }
+  AsmRemarks &asm_remarks() { return _asm_remarks; }
+  DbgStrings &dbg_strings() { return _dbg_strings; }
 
-  void free_strings() {
-    if (!_code_strings.is_null()) {
-      _code_strings.free(); // sets _strings Null as a side-effect.
-    }
+  void clear_strings() {
+    _asm_remarks.clear();
+    _dbg_strings.clear();
   }
 #endif
 
@@ -666,7 +676,7 @@ class CodeBuffer: public StackObj {
     }
   }
 
-  void block_comment(intptr_t offset, const char * comment) PRODUCT_RETURN;
+  void block_comment(ptrdiff_t offset, const char* comment) PRODUCT_RETURN;
   const char* code_string(const char* str) PRODUCT_RETURN_(return NULL;);
 
   // Log a little info about section usage in the CodeBuffer

src/hotspot/share/asm/codeBuffer.hpp

@@ -94,7 +94,6 @@ CodeBlob::CodeBlob(const char* name, CompilerType type, const CodeBlobLayout& la
   _oop_maps(oop_maps),
   _caller_must_gc_arguments(caller_must_gc_arguments),
   _name(name)
-  NOT_PRODUCT(COMMA _strings(CodeStrings()))
 {
   assert(is_aligned(layout.size(),            oopSize), "unaligned size");
   assert(is_aligned(layout.header_size(),     oopSize), "unaligned size");
@@ -107,7 +106,7 @@ CodeBlob::CodeBlob(const char* name, CompilerType type, const CodeBlobLayout& la
   S390_ONLY(_ctable_offset = 0;) // avoid uninitialized fields
 }
 
-CodeBlob::CodeBlob(const char* name, CompilerType type, const CodeBlobLayout& layout, CodeBuffer* cb, int frame_complete_offset, int frame_size, OopMapSet* oop_maps, bool caller_must_gc_arguments) :
+CodeBlob::CodeBlob(const char* name, CompilerType type, const CodeBlobLayout& layout, CodeBuffer* cb /*UNUSED*/, int frame_complete_offset, int frame_size, OopMapSet* oop_maps, bool caller_must_gc_arguments) :
   _type(type),
   _size(layout.size()),
   _header_size(layout.header_size()),
@@ -122,7 +121,6 @@ CodeBlob::CodeBlob(const char* name, CompilerType type, const CodeBlobLayout& la
   _relocation_end(layout.relocation_end()),
   _caller_must_gc_arguments(caller_must_gc_arguments),
   _name(name)
-  NOT_PRODUCT(COMMA _strings(CodeStrings()))
 {
   assert(is_aligned(_size,        oopSize), "unaligned size");
   assert(is_aligned(_header_size, oopSize), "unaligned size");
@@ -164,7 +162,8 @@ RuntimeBlob::RuntimeBlob(
 void CodeBlob::flush() {
   FREE_C_HEAP_ARRAY(unsigned char, _oop_maps);
   _oop_maps = NULL;
-  NOT_PRODUCT(_strings.free();)
+  NOT_PRODUCT(_asm_remarks.clear());
+  NOT_PRODUCT(_dbg_strings.clear());
 }
 
 void CodeBlob::set_oop_maps(OopMapSet* p) {
@@ -190,7 +189,8 @@ void RuntimeBlob::trace_new_stub(RuntimeBlob* stub, const char* name1, const cha
       ttyLocker ttyl;
       tty->print_cr("- - - [BEGIN] - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -");
       tty->print_cr("Decoding %s " INTPTR_FORMAT, stub_id, (intptr_t) stub);
-      Disassembler::decode(stub->code_begin(), stub->code_end(), tty);
+      Disassembler::decode(stub->code_begin(), stub->code_end(), tty
+                           NOT_PRODUCT(COMMA &stub->asm_remarks()));
       if ((stub->oop_maps() != NULL) && AbstractDisassembler::show_structs()) {
         tty->print_cr("- - - [OOP MAPS]- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -");
         stub->oop_maps()->print();

src/hotspot/share/code/codeBlob.cpp

@@ -112,15 +112,22 @@ class CodeBlob {
   const char*         _name;
   S390_ONLY(int       _ctable_offset;)
 
-  NOT_PRODUCT(CodeStrings _strings;)
+#ifndef PRODUCT
+  AsmRemarks _asm_remarks;
+  DbgStrings _dbg_strings;
+
+ ~CodeBlob() {
+    _asm_remarks.clear();
+    _dbg_strings.clear();
+  }
+#endif // not PRODUCT
 
   CodeBlob(const char* name, CompilerType type, const CodeBlobLayout& layout, int frame_complete_offset, int frame_size, ImmutableOopMapSet* oop_maps, bool caller_must_gc_arguments);
   CodeBlob(const char* name, CompilerType type, const CodeBlobLayout& layout, CodeBuffer* cb, int frame_complete_offset, int frame_size, OopMapSet* oop_maps, bool caller_must_gc_arguments);
 
 public:
   // Only used by unit test.
-  CodeBlob()
-    : _type(compiler_none) {}
+  CodeBlob() : _type(compiler_none) {}
 
   // Returns the space needed for CodeBlob
   static unsigned int allocation_size(CodeBuffer* cb, int header_size);
@@ -232,18 +239,21 @@ class CodeBlob {
   void dump_for_addr(address addr, outputStream* st, bool verbose) const;
   void print_code();
 
-  // Print the comment associated with offset on stream, if there is one
+  // Print to stream, any comments associated with offset.
   virtual void print_block_comment(outputStream* stream, address block_begin) const {
-  #ifndef PRODUCT
-    intptr_t offset = (intptr_t)(block_begin - code_begin());
-    _strings.print_block_comment(stream, offset);
-  #endif
+#ifndef PRODUCT
+    ptrdiff_t offset = block_begin - code_begin();
+    assert(offset >= 0, "Expecting non-negative offset!");
+    _asm_remarks.print(uint(offset), stream);
+#endif
   }
 
 #ifndef PRODUCT
-  void set_strings(CodeStrings& strings) {
-    _strings.copy(strings);
-  }
+  AsmRemarks &asm_remarks() { return _asm_remarks; }
+  DbgStrings &dbg_strings() { return _dbg_strings; }
+
+  void use_remarks(AsmRemarks &remarks) { _asm_remarks.share(remarks); }
+  void use_strings(DbgStrings &strings) { _dbg_strings.share(strings); }
 #endif
 };
 

src/hotspot/share/code/codeBlob.hpp

@@ -56,8 +56,7 @@ class ICStub: public Stub {
  protected:
   friend class ICStubInterface;
   // This will be called only by ICStubInterface
-  void    initialize(int size,
-                     CodeStrings strings)        { _size = size; _ic_site = NULL; }
+  void    initialize(int size)                   { _size = size; _ic_site = NULL; }
   void    finalize(); // called when a method is removed
 
   // General info

src/hotspot/share/code/icBuffer.hpp

@@ -109,8 +109,7 @@ Stub* StubQueue::stub_containing(address pc) const {
 
 Stub* StubQueue::request_committed(int code_size) {
   Stub* s = request(code_size);
-  CodeStrings strings;
-  if (s != NULL) commit(code_size, strings);
+  if (s != NULL) commit(code_size);
   return s;
 }
 
@@ -127,8 +126,7 @@ Stub* StubQueue::request(int requested_code_size) {
       assert(_buffer_limit == _buffer_size, "buffer must be fully usable");
       if (_queue_end + requested_size <= _buffer_size) {
         // code fits in at the end => nothing to do
-        CodeStrings strings;
-        stub_initialize(s, requested_size, strings);
+        stub_initialize(s, requested_size);
         return s;
       } else {
         // stub doesn't fit in at the queue end
@@ -145,8 +143,7 @@ Stub* StubQueue::request(int requested_code_size) {
     // Queue: |XXX|.......|XXXXXXX|.......|
     //        ^0  ^end    ^begin  ^limit  ^size
     s = current_stub();
-    CodeStrings strings;
-    stub_initialize(s, requested_size, strings);
+    stub_initialize(s, requested_size);
     return s;
   }
   // Not enough space left
@@ -155,12 +152,12 @@ Stub* StubQueue::request(int requested_code_size) {
 }
 
 
-void StubQueue::commit(int committed_code_size, CodeStrings& strings) {
+void StubQueue::commit(int committed_code_size) {
   assert(committed_code_size > 0, "committed_code_size must be > 0");
   int committed_size = align_up(stub_code_size_to_size(committed_code_size), CodeEntryAlignment);
   Stub* s = current_stub();
   assert(committed_size <= stub_size(s), "committed size must not exceed requested size");
-  stub_initialize(s, committed_size, strings);
+  stub_initialize(s, committed_size);
   _queue_end += committed_size;
   _number_of_stubs++;
   if (_mutex != NULL) _mutex->unlock();