-
Bug
-
Resolution: Fixed
-
P3
-
11, 14
-
b23
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);
+ }
}
}
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);
+ }
}
}