8365256: RelocIterator should use indexes instead of pointers

Reviewed-by: kvn, dlong

Author: jdksjolen

src/hotspot/share/code/codeBlob.cpp

@@ -128,7 +128,7 @@ CodeBlob::CodeBlob(const char* name, CodeBlobKind kind, CodeBuffer* cb, int size
                    int mutable_data_size) :
   _oop_maps(nullptr), // will be set by set_oop_maps() call
   _name(name),
-  _mutable_data(header_begin() + size), // default value is blob_end()
+  _mutable_data(nullptr),
   _size(size),
   _relocation_size(align_up(cb->total_relocation_size(), oopSize)),
   _content_offset(CodeBlob::align_code_offset(header_size)),
@@ -158,10 +158,8 @@ CodeBlob::CodeBlob(const char* name, CodeBlobKind kind, CodeBuffer* cb, int size
     if (_mutable_data == nullptr) {
       vm_exit_out_of_memory(_mutable_data_size, OOM_MALLOC_ERROR, "codebuffer: no space for mutable data");
     }
-  } else {
-    // We need unique and valid not null address
-    assert(_mutable_data == blob_end(), "sanity");
   }
+  assert(_mutable_data != nullptr || _mutable_data_size == 0, "No mutable data => mutable data size is 0");
 
   set_oop_maps(oop_maps);
 }
@@ -170,7 +168,7 @@ CodeBlob::CodeBlob(const char* name, CodeBlobKind kind, CodeBuffer* cb, int size
 CodeBlob::CodeBlob(const char* name, CodeBlobKind kind, int size, uint16_t header_size) :
   _oop_maps(nullptr),
   _name(name),
-  _mutable_data(header_begin() + size), // default value is blob_end()
+  _mutable_data(nullptr),
   _size(size),
   _relocation_size(0),
   _content_offset(CodeBlob::align_code_offset(header_size)),
@@ -184,9 +182,9 @@ CodeBlob::CodeBlob(const char* name, CodeBlobKind kind, int size, uint16_t heade
   _kind(kind),
   _caller_must_gc_arguments(false)
 {
+  assert(_mutable_data == nullptr && _mutable_data_size == 0, "invariant");
   assert(is_aligned(size,            oopSize), "unaligned size");
   assert(is_aligned(header_size,     oopSize), "unaligned size");
-  assert(_mutable_data == blob_end(), "sanity");
 }
 
 void CodeBlob::restore_mutable_data(address reloc_data) {
@@ -197,7 +195,7 @@ void CodeBlob::restore_mutable_data(address reloc_data) {
       vm_exit_out_of_memory(_mutable_data_size, OOM_MALLOC_ERROR, "codebuffer: no space for mutable data");
     }
   } else {
-    _mutable_data = blob_end(); // default value
+    _mutable_data = nullptr;
   }
   if (_relocation_size > 0) {
     assert(_mutable_data_size > 0, "relocation is part of mutable data section");
@@ -206,17 +204,13 @@ void CodeBlob::restore_mutable_data(address reloc_data) {
 }
 
 void CodeBlob::purge() {
-  assert(_mutable_data != nullptr, "should never be null");
-  if (_mutable_data != blob_end()) {
-    os::free(_mutable_data);
-    _mutable_data = blob_end(); // Valid not null address
-    _mutable_data_size = 0;
-    _relocation_size = 0;
-  }
-  if (_oop_maps != nullptr) {
-    delete _oop_maps;
-    _oop_maps = nullptr;
-  }
+  os::free(_mutable_data);
+  _mutable_data = nullptr;
+  _mutable_data_size = 0;
+  delete _oop_maps;
+  _oop_maps = nullptr;
+  _relocation_size = 0;
+
   NOT_PRODUCT(_asm_remarks.clear());
   NOT_PRODUCT(_dbg_strings.clear());
 }

src/hotspot/share/code/nmethod.cpp

@@ -1327,8 +1327,8 @@ nmethod::nmethod(
            "wrong mutable data size: %d != %d + %d",
            _mutable_data_size, _relocation_size, metadata_size);
 
-    // native wrapper does not have read-only data but we need unique not null address
-    _immutable_data          = blob_end();
+    // native wrapper does not have read-only data
+    _immutable_data          = nullptr;
     _immutable_data_size     = 0;
     _nul_chk_table_offset    = 0;
     _handler_table_offset    = 0;
@@ -1510,8 +1510,7 @@ nmethod::nmethod(
       assert(immutable_data != nullptr, "required");
       _immutable_data     = immutable_data;
     } else {
-      // We need unique not null address
-      _immutable_data     = blob_end();
+      _immutable_data     = nullptr;
     }
     CHECKED_CAST(_nul_chk_table_offset, uint16_t, (align_up((int)dependencies->size_in_bytes(), oopSize)));
     CHECKED_CAST(_handler_table_offset, uint16_t, (_nul_chk_table_offset + align_up(nul_chk_table->size_in_bytes(), oopSize)));
@@ -2147,15 +2146,14 @@ void nmethod::purge(bool unregister_nmethod) {
     delete ec;
     ec = next;
   }
-  if (_pc_desc_container != nullptr) {
-    delete _pc_desc_container;
-  }
+
+  delete _pc_desc_container;
   delete[] _compiled_ic_data;
 
-  if (_immutable_data != blob_end()) {
-    os::free(_immutable_data);
-    _immutable_data = blob_end(); // Valid not null address
-  }
+  os::free(_immutable_data);
+  _immutable_data = nullptr;
+  _immutable_data_size = 0;
+
   if (unregister_nmethod) {
     Universe::heap()->unregister_nmethod(this);
   }

src/hotspot/share/code/relocInfo.cpp

@@ -116,9 +116,6 @@ void relocInfo::change_reloc_info_for_address(RelocIterator *itr, address pc, re
 // ----------------------------------------------------------------------------------------------------
 // Implementation of RelocIterator
 
-// A static dummy to serve as a safe pointer when there is no relocation info.
-static relocInfo dummy_relocInfo = relocInfo(relocInfo::none, 0);
-
 void RelocIterator::initialize(nmethod* nm, address begin, address limit) {
   initialize_misc();
 
@@ -130,14 +127,9 @@ void RelocIterator::initialize(nmethod* nm, address begin, address limit) {
   guarantee(nm != nullptr, "must be able to deduce nmethod from other arguments");
 
   _code    = nm;
-  if (nm->relocation_size() == 0) {
-    _current = &dummy_relocInfo - 1;
-    _end = &dummy_relocInfo;
-  } else {
-    assert(((nm->relocation_begin() != nullptr) && (nm->relocation_end() != nullptr)), "valid start and end pointer");
-    _current = nm->relocation_begin() - 1;
-    _end     = nm->relocation_end();
-  }
+  _base = nm->relocation_begin();
+  _current = -1;
+  _len = nm->relocation_end() - _base;
   _addr    = nm->content_begin();
 
   // Initialize code sections.
@@ -156,11 +148,21 @@ void RelocIterator::initialize(nmethod* nm, address begin, address limit) {
 }
 
 
+RelocIterator::RelocIterator(relocInfo& ri) {
+  initialize_misc();
+  _base = &ri;
+  _len = 1;
+  _current = -1;
+  _limit = nullptr;
+  _addr = 0;
+}
+
 RelocIterator::RelocIterator(CodeSection* cs, address begin, address limit) {
   initialize_misc();
   assert(((cs->locs_start() != nullptr) && (cs->locs_end() != nullptr)), "valid start and end pointer");
-  _current = cs->locs_start() - 1;
-  _end     = cs->locs_end();
+  _base = cs->locs_start();
+  _len = cs->locs_end() - _base;
+  _current = -1;
   _addr    = cs->start();
   _code    = nullptr; // Not cb->blob();
 
@@ -186,8 +188,9 @@ RelocIterator::RelocIterator(CodeBlob* cb) {
   } else {
     _code = nullptr;
   }
-  _current = cb->relocation_begin() - 1;
-  _end     = cb->relocation_end();
+  _base = cb->relocation_begin();
+  _len = cb->relocation_end() - _base;
+  _current = -1;
   _addr    = cb->content_begin();
 
   _section_start[CodeBuffer::SECT_CONSTS] = cb->content_begin();
@@ -216,7 +219,7 @@ void RelocIterator::set_limits(address begin, address limit) {
 
   // the limit affects this next stuff:
   if (begin != nullptr) {
-    relocInfo* backup;
+    int backup;
     address    backup_addr;
     while (true) {
       backup      = _current;
@@ -238,12 +241,12 @@ void RelocIterator::set_limits(address begin, address limit) {
 // very efficiently (a single extra halfword).  Larger chunks of
 // relocation data need a halfword header to hold their size.
 void RelocIterator::advance_over_prefix() {
-  if (_current->is_datalen()) {
-    _data    = (short*) _current->data();
-    _datalen =          _current->datalen();
+  if (current()->is_datalen()) {
+    _data    = (short*) current()->data();
+    _datalen =          current()->datalen();
     _current += _datalen + 1;   // skip the embedded data & header
   } else {
-    _databuf = _current->immediate();
+    _databuf = current()->immediate();
     _data = &_databuf;
     _datalen = 1;
     _current++;                 // skip the header
@@ -350,9 +353,9 @@ void Relocation::const_verify_data_value(address x) {
 
 RelocationHolder Relocation::spec_simple(relocInfo::relocType rtype) {
   if (rtype == relocInfo::none)  return RelocationHolder::none;
-  relocInfo ri = relocInfo(rtype, 0);
-  RelocIterator itr;
-  itr.set_current(ri);
+  relocInfo ri(rtype, 0);
+  RelocIterator itr(ri);
+  itr.next();
   itr.reloc();
   return itr._rh;
 }
@@ -839,7 +842,7 @@ void RelocIterator::print_current_on(outputStream* st) {
     return;
   }
   st->print("relocInfo@" INTPTR_FORMAT " [type=%d(%s) addr=" INTPTR_FORMAT " offset=%d",
-             p2i(_current), type(), relocInfo::type_name((relocInfo::relocType) type()), p2i(_addr), _current->addr_offset());
+            p2i(current()), type(), relocInfo::type_name((relocInfo::relocType) type()), p2i(_addr), current()->addr_offset());
   if (current()->format() != 0)
     st->print(" format=%d", current()->format());
   if (datalen() == 1) {
@@ -990,7 +993,7 @@ void RelocIterator::print_current_on(outputStream* st) {
 
 void RelocIterator::print_on(outputStream* st) {
   RelocIterator save_this = (*this);
-  relocInfo* scan = _current;
+  relocInfo* scan = current_no_check();
   if (!has_current())  scan += 1;  // nothing to scan here!
 
   bool skip_next = has_current();
@@ -1000,7 +1003,7 @@ void RelocIterator::print_on(outputStream* st) {
     skip_next = false;
 
     st->print("         @" INTPTR_FORMAT ": ", p2i(scan));
-    relocInfo* newscan = _current+1;
+    relocInfo* newscan = current_no_check()+1;
     if (!has_current())  newscan -= 1;  // nothing to scan here!
     while (scan < newscan) {
       st->print("%04x", *(short*)scan & 0xFFFF);

src/hotspot/share/code/relocInfo.hpp

@@ -562,12 +562,13 @@ class RelocIterator : public StackObj {
 
  private:
   address         _limit;   // stop producing relocations after this _addr
-  relocInfo*      _current; // the current relocation information
-  relocInfo*      _end;     // end marker; we're done iterating when _current == _end
+  relocInfo*      _base;    // base pointer into relocInfo array
+  int             _current; // current index
+  int             _len;     // length
   nmethod*        _code;    // compiled method containing _addr
   address         _addr;    // instruction to which the relocation applies
-  short           _databuf; // spare buffer for compressed data
   short*          _data;    // pointer to the relocation's data
+  short           _databuf; // spare buffer for compressed data
   short           _datalen; // number of halfwords in _data
 
   // Base addresses needed to compute targets of section_word_type relocs.
@@ -578,15 +579,14 @@ class RelocIterator : public StackObj {
     _datalen = !b ? -1 : 0;
     DEBUG_ONLY(_data = nullptr);
   }
-  void set_current(relocInfo& ri) {
-    _current = &ri;
-    set_has_current(true);
-  }
 
   RelocationHolder _rh; // where the current relocation is allocated
 
-  relocInfo* current() const { assert(has_current(), "must have current");
-                               return _current; }
+  relocInfo* current_no_check() const { return &_base[_current]; }
+  relocInfo* current() const {
+    assert(has_current(), "must have current");
+    return current_no_check();
+  }
 
   void set_limits(address begin, address limit);
 
@@ -597,6 +597,7 @@ class RelocIterator : public StackObj {
   void initialize(nmethod* nm, address begin, address limit);
 
   RelocIterator() { initialize_misc(); }
+  RelocIterator(relocInfo& ri);
 
  public:
   // constructor
@@ -607,25 +608,24 @@ class RelocIterator : public StackObj {
   // get next reloc info, return !eos
   bool next() {
     _current++;
-    assert(_current <= _end, "must not overrun relocInfo");
-    if (_current == _end) {
+    assert(_current <= _len, "must not overrun relocInfo");
+    if (_current == _len) {
       set_has_current(false);
       return false;
     }
     set_has_current(true);
 
-    if (_current->is_prefix()) {
+    if (current()->is_prefix()) {
       advance_over_prefix();
       assert(!current()->is_prefix(), "only one prefix at a time");
     }
 
-    _addr += _current->addr_offset();
+    _addr += current()->addr_offset();
 
     if (_limit != nullptr && _addr >= _limit) {
       set_has_current(false);
       return false;
     }
-
     return true;
   }