8225716: G1 GC: Undefined behaviour in G1BlockOffsetTablePart::block_at_or_preceding
authoraph
Tue, 18 Jun 2019 16:15:15 +0100
changeset 55429 d7da94e6c169
parent 55428 e9da3a44a7ed
child 55430 82deab2dd59e
8225716: G1 GC: Undefined behaviour in G1BlockOffsetTablePart::block_at_or_preceding Reviewed-by: kbarrett, tschatzl
src/hotspot/share/gc/g1/g1BlockOffsetTable.cpp
src/hotspot/share/gc/g1/g1BlockOffsetTable.hpp
src/hotspot/share/gc/g1/g1BlockOffsetTable.inline.hpp
--- a/src/hotspot/share/gc/g1/g1BlockOffsetTable.cpp	Wed Jun 19 08:52:15 2019 -0400
+++ b/src/hotspot/share/gc/g1/g1BlockOffsetTable.cpp	Tue Jun 18 16:15:15 2019 +0100
@@ -391,8 +391,6 @@
 #endif // !PRODUCT
 
 HeapWord* G1BlockOffsetTablePart::initialize_threshold_raw() {
-  assert(!G1CollectedHeap::heap()->is_in_reserved(_bot->_offset_array),
-         "just checking");
   _next_offset_index = _bot->index_for_raw(_space->bottom());
   _next_offset_index++;
   _next_offset_threshold =
@@ -401,8 +399,6 @@
 }
 
 void G1BlockOffsetTablePart::zero_bottom_entry_raw() {
-  assert(!G1CollectedHeap::heap()->is_in_reserved(_bot->_offset_array),
-         "just checking");
   size_t bottom_index = _bot->index_for_raw(_space->bottom());
   assert(_bot->address_for_index_raw(bottom_index) == _space->bottom(),
          "Precondition of call");
@@ -410,8 +406,6 @@
 }
 
 HeapWord* G1BlockOffsetTablePart::initialize_threshold() {
-  assert(!G1CollectedHeap::heap()->is_in_reserved(_bot->_offset_array),
-         "just checking");
   _next_offset_index = _bot->index_for(_space->bottom());
   _next_offset_index++;
   _next_offset_threshold =
--- a/src/hotspot/share/gc/g1/g1BlockOffsetTable.hpp	Wed Jun 19 08:52:15 2019 -0400
+++ b/src/hotspot/share/gc/g1/g1BlockOffsetTable.hpp	Tue Jun 18 16:15:15 2019 +0100
@@ -52,7 +52,7 @@
 
   // Array for keeping offsets for retrieving object start fast given an
   // address.
-  u_char* _offset_array;          // byte array keeping backwards offsets
+  volatile u_char* _offset_array;  // byte array keeping backwards offsets
 
   void check_offset(size_t offset, const char* msg) const {
     assert(offset <= BOTConstants::N_words,
@@ -64,10 +64,7 @@
   // For performance these have to devolve to array accesses in product builds.
   inline u_char offset_array(size_t index) const;
 
-  void set_offset_array_raw(size_t index, u_char offset) {
-    _offset_array[index] = offset;
-  }
-
+  inline void set_offset_array_raw(size_t index, u_char offset);
   inline void set_offset_array(size_t index, u_char offset);
 
   inline void set_offset_array(size_t index, HeapWord* high, HeapWord* low);
--- a/src/hotspot/share/gc/g1/g1BlockOffsetTable.inline.hpp	Wed Jun 19 08:52:15 2019 -0400
+++ b/src/hotspot/share/gc/g1/g1BlockOffsetTable.inline.hpp	Tue Jun 18 16:15:15 2019 +0100
@@ -29,6 +29,7 @@
 #include "gc/g1/heapRegion.hpp"
 #include "gc/shared/memset_with_concurrent_readers.hpp"
 #include "gc/shared/space.hpp"
+#include "runtime/atomic.hpp"
 
 inline HeapWord* G1BlockOffsetTablePart::block_start(const void* addr) {
   if (addr >= _space->bottom() && addr < _space->end()) {
@@ -51,7 +52,11 @@
 
 u_char G1BlockOffsetTable::offset_array(size_t index) const {
   check_index(index, "index out of range");
-  return _offset_array[index];
+  return Atomic::load(&_offset_array[index]);
+}
+
+void G1BlockOffsetTable::set_offset_array_raw(size_t index, u_char offset) {
+  Atomic::store(offset, &_offset_array[index]);
 }
 
 void G1BlockOffsetTable::set_offset_array(size_t index, u_char offset) {
@@ -71,7 +76,8 @@
   check_index(right, "right index out of range");
   assert(left <= right, "indexes out of order");
   size_t num_cards = right - left + 1;
-  memset_with_concurrent_readers(&_offset_array[left], offset, num_cards);
+  memset_with_concurrent_readers
+    (const_cast<u_char*> (&_offset_array[left]), offset, num_cards);
 }
 
 // Variant of index_for that does not check the index for validity.