[8u] RFR (XS) 8259271: gc/parallel/TestDynShrinkHeap.java still fails "assert(covered_region.contains(new_memregion)) failed: new region is not in covered_region"

[Hohensee, Paul]

Lgtm. I've created an openjdk8u backport issue https://bugs.openjdk.java.net/browse/JDK-8262790 (new'ish process), which you can tag.

Thanks,
Paul

-----Original Message-----
From: jdk8u-dev <jdk8u-dev-retn at openjdk.java.net> on behalf of Aleksey Shipilev <shade at redhat.com>
Date: Monday, March 1, 2021 at 10:10 AM
To: "jdk8u-dev at openjdk.java.net" <jdk8u-dev at openjdk.java.net>
Subject: [8u] RFR (XS) 8259271: gc/parallel/TestDynShrinkHeap.java still fails "assert(covered_region.contains(new_memregion)) failed: new region is not in covered_region"

This is second of three patches that dance around the same issue.

Original change:
   https://bugs.openjdk.java.net/browse/JDK-8259271
   https://github.com/openjdk/jdk16/commit/685c03dc

This applies on top of JDK-8257999.

Unfortunately, 8u code does not have OrderAccess cleanups, so we have to use the older forms of
load_acquire. 8u variant is:

diff -r a9fd914bc78f src/share/vm/gc_implementation/shared/mutableSpace.cpp
--- a/src/share/vm/gc_implementation/shared/mutableSpace.cpp    Thu Dec 17 14:18:00 2020 +0000
+++ b/src/share/vm/gc_implementation/shared/mutableSpace.cpp    Mon Mar 01 19:07:25 2021 +0100
@@ -26,10 +26,11 @@
  #include "utilities/macros.hpp"
  #if INCLUDE_ALL_GCS
  #include "gc_implementation/shared/mutableSpace.hpp"
  #include "gc_implementation/shared/spaceDecorator.hpp"
  #include "oops/oop.inline.hpp"
+#include "runtime/orderAccess.hpp"
  #include "runtime/safepoint.hpp"
  #include "runtime/thread.hpp"
  #endif // INCLUDE_ALL_GCS

  PRAGMA_FORMAT_MUTE_WARNINGS_FOR_GCC
@@ -190,11 +191,15 @@
  }

  // This version is lock-free.
  HeapWord* MutableSpace::cas_allocate(size_t size) {
    do {
-    HeapWord* obj = top();
+    // Read top before end, else the range check may pass when it shouldn't.
+    // If end is read first, other threads may advance end and top such that
+    // current top > old end and current top + size > current end.  Then
+    // pointer_delta underflows, allowing installation of top > current end.
+    HeapWord* obj = (HeapWord*)OrderAccess::load_ptr_acquire(top_addr());
      if (pointer_delta(end(), obj) >= size) {
        HeapWord* new_top = obj + size;
        HeapWord* result = (HeapWord*)Atomic::cmpxchg_ptr(new_top, top_addr(), obj);
        // result can be one of two:
        //  the old top value: the exchange succeeded

Testing: tier1

--
Thanks,
-Aleksey