8366341: [BACKOUT] JDK-8365256: RelocIterator should use indexes instead of pointers

Reviewed-by: ayang

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(nullptr),
+  _mutable_data(header_begin() + size), // default value is blob_end()
   _size(size),
   _relocation_size(align_up(cb->total_relocation_size(), oopSize)),
   _content_offset(CodeBlob::align_code_offset(header_size)),
@@ -158,8 +158,10 @@ 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);
 }
@@ -168,7 +170,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(nullptr),
+  _mutable_data(header_begin() + size), // default value is blob_end()
   _size(size),
   _relocation_size(0),
   _content_offset(CodeBlob::align_code_offset(header_size)),
@@ -182,9 +184,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) {
@@ -195,7 +197,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 = nullptr;
+    _mutable_data = blob_end(); // default value
   }
   if (_relocation_size > 0) {
     assert(_mutable_data_size > 0, "relocation is part of mutable data section");
@@ -204,13 +206,17 @@ void CodeBlob::restore_mutable_data(address reloc_data) {
 }
 
 void CodeBlob::purge() {
-  os::free(_mutable_data);
-  _mutable_data = nullptr;
-  _mutable_data_size = 0;
-  delete _oop_maps;
-  _oop_maps = nullptr;
-  _relocation_size = 0;
-
+  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;
+  }
   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
-    _immutable_data          = nullptr;
+    // native wrapper does not have read-only data but we need unique not null address
+    _immutable_data          = blob_end();
     _immutable_data_size     = 0;
     _nul_chk_table_offset    = 0;
     _handler_table_offset    = 0;
@@ -1510,7 +1510,8 @@ nmethod::nmethod(
       assert(immutable_data != nullptr, "required");
       _immutable_data     = immutable_data;
     } else {
-      _immutable_data     = nullptr;
+      // We need unique not null address
+      _immutable_data     = blob_end();
     }
     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)));
@@ -2146,14 +2147,15 @@ void nmethod::purge(bool unregister_nmethod) {
     delete ec;
     ec = next;
   }
-
-  delete _pc_desc_container;
+  if (_pc_desc_container != nullptr) {
+    delete _pc_desc_container;
+  }
   delete[] _compiled_ic_data;
 
-  os::free(_immutable_data);
-  _immutable_data = nullptr;
-  _immutable_data_size = 0;
-
+  if (_immutable_data != blob_end()) {
+    os::free(_immutable_data);
+    _immutable_data = blob_end(); // Valid not null address
+  }
   if (unregister_nmethod) {
     Universe::heap()->unregister_nmethod(this);
   }

src/hotspot/share/code/relocInfo.cpp

@@ -116,6 +116,9 @@ 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();
 
@@ -127,9 +130,14 @@ void RelocIterator::initialize(nmethod* nm, address begin, address limit) {
   guarantee(nm != nullptr, "must be able to deduce nmethod from other arguments");
 
   _code    = nm;
-  _base = nm->relocation_begin();
-  _current = -1;
-  _len = nm->relocation_end() - _base;
+  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();
+  }
   _addr    = nm->content_begin();
 
   // Initialize code sections.
@@ -148,21 +156,11 @@ 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");
-  _base = cs->locs_start();
-  _len = cs->locs_end() - _base;
-  _current = -1;
+  _current = cs->locs_start() - 1;
+  _end     = cs->locs_end();
   _addr    = cs->start();
   _code    = nullptr; // Not cb->blob();
 
@@ -188,9 +186,8 @@ RelocIterator::RelocIterator(CodeBlob* cb) {
   } else {
     _code = nullptr;
   }
-  _base = cb->relocation_begin();
-  _len = cb->relocation_end() - _base;
-  _current = -1;
+  _current = cb->relocation_begin() - 1;
+  _end     = cb->relocation_end();
   _addr    = cb->content_begin();
 
   _section_start[CodeBuffer::SECT_CONSTS] = cb->content_begin();
@@ -219,7 +216,7 @@ void RelocIterator::set_limits(address begin, address limit) {
 
   // the limit affects this next stuff:
   if (begin != nullptr) {
-    int backup;
+    relocInfo* backup;
     address    backup_addr;
     while (true) {
       backup      = _current;
@@ -241,12 +238,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
@@ -353,9 +350,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(rtype, 0);
-  RelocIterator itr(ri);
-  itr.next();
+  relocInfo ri = relocInfo(rtype, 0);
+  RelocIterator itr;
+  itr.set_current(ri);
   itr.reloc();
   return itr._rh;
 }
@@ -842,7 +839,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) {
@@ -993,7 +990,7 @@ void RelocIterator::print_current_on(outputStream* st) {
 
 void RelocIterator::print_on(outputStream* st) {
   RelocIterator save_this = (*this);
-  relocInfo* scan = current_no_check();
+  relocInfo* scan = _current;
   if (!has_current())  scan += 1;  // nothing to scan here!
 
   bool skip_next = has_current();
@@ -1003,7 +1000,7 @@ void RelocIterator::print_on(outputStream* st) {
     skip_next = false;
 
     st->print("         @" INTPTR_FORMAT ": ", p2i(scan));
-    relocInfo* newscan = current_no_check()+1;
+    relocInfo* newscan = _current+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,13 +562,12 @@ class RelocIterator : public StackObj {
 
  private:
   address         _limit;   // stop producing relocations after this _addr
-  relocInfo*      _base;    // base pointer into relocInfo array
-  int             _current; // current index
-  int             _len;     // length
+  relocInfo*      _current; // the current relocation information
+  relocInfo*      _end;     // end marker; we're done iterating when _current == _end
   nmethod*        _code;    // compiled method containing _addr
   address         _addr;    // instruction to which the relocation applies
-  short*          _data;    // pointer to the relocation's data
   short           _databuf; // spare buffer for compressed data
+  short*          _data;    // pointer to the relocation's data
   short           _datalen; // number of halfwords in _data
 
   // Base addresses needed to compute targets of section_word_type relocs.
@@ -579,14 +578,15 @@ 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_no_check() const { return &_base[_current]; }
-  relocInfo* current() const {
-    assert(has_current(), "must have current");
-    return current_no_check();
-  }
+  relocInfo* current() const { assert(has_current(), "must have current");
+                               return _current; }
 
   void set_limits(address begin, address limit);
 
@@ -597,7 +597,6 @@ class RelocIterator : public StackObj {
   void initialize(nmethod* nm, address begin, address limit);
 
   RelocIterator() { initialize_misc(); }
-  RelocIterator(relocInfo& ri);
 
  public:
   // constructor
@@ -608,24 +607,25 @@ class RelocIterator : public StackObj {
   // get next reloc info, return !eos
   bool next() {
     _current++;
-    assert(_current <= _len, "must not overrun relocInfo");
-    if (_current == _len) {
+    assert(_current <= _end, "must not overrun relocInfo");
+    if (_current == _end) {
       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;
   }