Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8233506

ZGC: the load for Reference.get() can be converted to a load for strong refs

    Details

    • Subcomponent:
    • Resolved In Build:
      b23

      Description

      We've found a crash where the WeakReference.get in WeakHashMap$HashIterator::hasNext is loaded with ZBarrierSetRuntime::load_barrier_on_oop_field_preloaded instead of ZBarrierSetRuntime::load_barrier_on_weak_oop_field_preloaded.

      The code for the Reference.get intrinsic returns a LoadNode with _barrier == ZLoadBarrierWeak, as expected. However, a later load is transformed and replaced with this LoadNode, and the _barrier value is incorrectly replaced with ZLoadBarrierStrong.

      The code where this happens looks like this:
        nextKey = e.get(); // hold on to key in strong ref
          if (nextKey == null)

      The load node for the read of nextKey is replaced by the load node for e.get().

      This causes issues like 'assert(ZAddress::is_marked(addr)) failed: Should be marked', and most likely others as well.

      Not the proposed fix for this, but the reproducer stops reproducing this patch:
      diff --git a/src/hotspot/share/gc/z/c2/zBarrierSetC2.cpp b/src/hotspot/share/gc/z/c2/zBarrierSetC2.cpp
      --- a/src/hotspot/share/gc/z/c2/zBarrierSetC2.cpp
      +++ b/src/hotspot/share/gc/z/c2/zBarrierSetC2.cpp
      @@ -182,10 +182,13 @@
       Node* ZBarrierSetC2::load_at_resolved(C2Access& access, const Type* val_type) const {
         Node* result = BarrierSetC2::load_at_resolved(access, val_type);
         if (barrier_needed(access) && access.raw_access()->is_Mem()) {
      - if ((access.decorators() & ON_WEAK_OOP_REF) != 0) {
      - access.raw_access()->as_Load()->set_barrier_data(ZLoadBarrierWeak);
      - } else {
      - access.raw_access()->as_Load()->set_barrier_data(ZLoadBarrierStrong);
      + LoadNode* load_node = access.raw_access()->as_Load();
      + if (load_node->barrier_data() == 0) {
      + if ((access.decorators() & ON_WEAK_OOP_REF) != 0) {
      + load_node->set_barrier_data(ZLoadBarrierWeak);
      + } else {
      + load_node->set_barrier_data(ZLoadBarrierStrong);
      + }
           }
         }
       

        Attachments

          Activity

            People

            • Assignee:
              eosterlund Erik Ă–sterlund
              Reporter:
              stefank Stefan Karlsson
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: