Clean up Object size related read barriers.
Test: m test-art-host-gtest
Test: testrunner.py --host --interpreter
Test: testrunner.py --host --interpreter --gcstress
Bug: 119486698
Change-Id: I831838f230ebdd9e540462b2de56271895a01fad
diff --git a/runtime/gc/collector/concurrent_copying.cc b/runtime/gc/collector/concurrent_copying.cc
index 854defe..30da1ae 100644
--- a/runtime/gc/collector/concurrent_copying.cc
+++ b/runtime/gc/collector/concurrent_copying.cc
@@ -3220,7 +3220,7 @@
if (ReadBarrier::kEnableToSpaceInvariantChecks) {
AssertToSpaceInvariant(nullptr, MemberOffset(0), int_array_class);
}
- size_t component_size = int_array_class->GetComponentSize<kWithoutReadBarrier>();
+ size_t component_size = int_array_class->GetComponentSize();
CHECK_EQ(component_size, sizeof(int32_t));
size_t data_offset = mirror::Array::DataOffset(component_size).SizeValue();
if (data_offset > byte_size) {
diff --git a/runtime/jit/jit_code_cache.cc b/runtime/jit/jit_code_cache.cc
index 73b9b69..d3e4582 100644
--- a/runtime/jit/jit_code_cache.cc
+++ b/runtime/jit/jit_code_cache.cc
@@ -728,7 +728,7 @@
mirror::Object* object = roots[i].Read<kWithoutReadBarrier>();
if (object == nullptr || object == weak_sentinel) {
// entry got deleted in a previous sweep.
- } else if (object->IsString<kDefaultVerifyFlags, kWithoutReadBarrier>()) {
+ } else if (object->IsString<kDefaultVerifyFlags>()) {
mirror::Object* new_object = visitor->IsMarked(object);
// We know the string is marked because it's a strongly-interned string that
// is always alive. The IsMarked implementation of the CMS collector returns
diff --git a/runtime/mirror/array-inl.h b/runtime/mirror/array-inl.h
index 19d35a8..34925f5 100644
--- a/runtime/mirror/array-inl.h
+++ b/runtime/mirror/array-inl.h
@@ -35,14 +35,16 @@
return Class::ComputeClassSize(true, vtable_entries, 0, 0, 0, 0, 0, pointer_size);
}
-template<VerifyObjectFlags kVerifyFlags, ReadBarrierOption kReadBarrierOption>
+template<VerifyObjectFlags kVerifyFlags>
inline size_t Array::SizeOf() {
- // This is safe from overflow because the array was already allocated, so we know it's sane.
- size_t component_size_shift = GetClass<kVerifyFlags, kReadBarrierOption>()->
- template GetComponentSizeShift<kReadBarrierOption>();
+ // No read barrier is needed for reading a constant primitive field through
+ // constant reference field chain. See ReadBarrierOption.
+ size_t component_size_shift =
+ GetClass<kVerifyFlags, kWithoutReadBarrier>()->GetComponentSizeShift();
// Don't need to check this since we already check this in GetClass.
int32_t component_count =
GetLength<static_cast<VerifyObjectFlags>(kVerifyFlags & ~kVerifyThis)>();
+ // This is safe from overflow because the array was already allocated, so we know it's sane.
size_t header_size = DataOffset(1U << component_size_shift).SizeValue();
size_t data_size = component_count << component_size_shift;
return header_size + data_size;
diff --git a/runtime/mirror/array.cc b/runtime/mirror/array.cc
index 05e397d..d42f5a0 100644
--- a/runtime/mirror/array.cc
+++ b/runtime/mirror/array.cc
@@ -139,7 +139,8 @@
}
ObjPtr<Array> Array::CopyOf(Thread* self, int32_t new_length) {
- CHECK(GetClass()->GetComponentType()->IsPrimitive()) << "Will miss write barriers";
+ ObjPtr<Class> klass = GetClass();
+ CHECK(klass->IsPrimitiveArray()) << "Will miss write barriers";
DCHECK_GE(new_length, 0);
// We may get copied by a compacting GC.
StackHandleScope<1> hs(self);
@@ -147,16 +148,16 @@
auto* heap = Runtime::Current()->GetHeap();
gc::AllocatorType allocator_type = heap->IsMovableObject(this) ? heap->GetCurrentAllocator() :
heap->GetCurrentNonMovingAllocator();
- const auto component_size = GetClass()->GetComponentSize();
- const auto component_shift = GetClass()->GetComponentSizeShift();
+ const auto component_size = klass->GetComponentSize();
+ const auto component_shift = klass->GetComponentSizeShift();
ObjPtr<Array> new_array =
- Alloc<true>(self, GetClass(), new_length, component_shift, allocator_type);
+ Alloc<true>(self, klass, new_length, component_shift, allocator_type); // Invalidates klass.
if (LIKELY(new_array != nullptr)) {
memcpy(new_array->GetRawData(component_size, 0),
h_this->GetRawData(component_size, 0),
std::min(h_this->GetLength(), new_length) << component_shift);
}
- return new_array.Ptr();
+ return new_array;
}
// Explicitly instantiate all the primitive array types.
diff --git a/runtime/mirror/array.h b/runtime/mirror/array.h
index 593c0a8..1ee4e50 100644
--- a/runtime/mirror/array.h
+++ b/runtime/mirror/array.h
@@ -55,8 +55,7 @@
REQUIRES_SHARED(Locks::mutator_lock_)
REQUIRES(!Roles::uninterruptible_);
- template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags,
- ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
+ template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags>
size_t SizeOf() REQUIRES_SHARED(Locks::mutator_lock_);
template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags>
ALWAYS_INLINE int32_t GetLength() REQUIRES_SHARED(Locks::mutator_lock_) {
diff --git a/runtime/mirror/class-inl.h b/runtime/mirror/class-inl.h
index 2e8ced5..3ee8bfe 100644
--- a/runtime/mirror/class-inl.h
+++ b/runtime/mirror/class-inl.h
@@ -1007,14 +1007,14 @@
SetFieldObject<false, false>(ComponentTypeOffset(), new_component_type);
}
-template<ReadBarrierOption kReadBarrierOption>
inline size_t Class::GetComponentSize() {
- return 1U << GetComponentSizeShift<kReadBarrierOption>();
+ return 1U << GetComponentSizeShift();
}
-template<ReadBarrierOption kReadBarrierOption>
inline size_t Class::GetComponentSizeShift() {
- return GetComponentType<kDefaultVerifyFlags, kReadBarrierOption>()->GetPrimitiveTypeSizeShift();
+ // No read barrier is needed for reading a constant primitive field through
+ // constant reference field. See ReadBarrierOption.
+ return GetComponentType<kDefaultVerifyFlags, kWithoutReadBarrier>()->GetPrimitiveTypeSizeShift();
}
inline bool Class::IsObjectClass() {
diff --git a/runtime/mirror/class.cc b/runtime/mirror/class.cc
index 3b4ae0f..5025c02 100644
--- a/runtime/mirror/class.cc
+++ b/runtime/mirror/class.cc
@@ -1003,7 +1003,10 @@
ObjPtr<mirror::Class> klass = this;
while (klass->IsArrayClass()) {
++dim;
- klass = klass->GetComponentType();
+ // No read barrier needed, we're reading a chain of constant references for comparison
+ // with null. Then we follow up below with reading constant references to read constant
+ // primitive data in both proxy and non-proxy paths. See ReadBarrierOption.
+ klass = klass->GetComponentType<kDefaultVerifyFlags, kWithoutReadBarrier>();
}
if (klass->IsProxyClass()) {
// No read barrier needed, the `name` field is constant for proxy classes and
diff --git a/runtime/mirror/class.h b/runtime/mirror/class.h
index 687b510..ac5d52d 100644
--- a/runtime/mirror/class.h
+++ b/runtime/mirror/class.h
@@ -446,10 +446,8 @@
void SetComponentType(ObjPtr<Class> new_component_type) REQUIRES_SHARED(Locks::mutator_lock_);
- template<ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
size_t GetComponentSize() REQUIRES_SHARED(Locks::mutator_lock_);
- template<ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
size_t GetComponentSizeShift() REQUIRES_SHARED(Locks::mutator_lock_);
bool IsObjectClass() REQUIRES_SHARED(Locks::mutator_lock_);
@@ -478,8 +476,7 @@
template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags>
ALWAYS_INLINE bool IsVariableSize() REQUIRES_SHARED(Locks::mutator_lock_);
- template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags,
- ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
+ template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags>
uint32_t SizeOf() REQUIRES_SHARED(Locks::mutator_lock_) {
return GetField32<kVerifyFlags>(OFFSET_OF_OBJECT_MEMBER(Class, class_size_));
}
diff --git a/runtime/mirror/object-inl.h b/runtime/mirror/object-inl.h
index 56e1807..2476b13 100644
--- a/runtime/mirror/object-inl.h
+++ b/runtime/mirror/object-inl.h
@@ -300,14 +300,16 @@
return ObjPtr<DoubleArray>::DownCast(this);
}
-template<VerifyObjectFlags kVerifyFlags, ReadBarrierOption kReadBarrierOption>
+template<VerifyObjectFlags kVerifyFlags>
inline bool Object::IsString() {
- return GetClass<kVerifyFlags, kReadBarrierOption>()->IsStringClass();
+ // No read barrier is needed for reading a constant primitive field through
+ // constant reference field. See ReadBarrierOption.
+ return GetClass<kVerifyFlags, kWithoutReadBarrier>()->IsStringClass();
}
-template<VerifyObjectFlags kVerifyFlags, ReadBarrierOption kReadBarrierOption>
+template<VerifyObjectFlags kVerifyFlags>
inline ObjPtr<String> Object::AsString() {
- DCHECK((IsString<kVerifyFlags, kReadBarrierOption>()));
+ DCHECK((IsString<kVerifyFlags>()));
return ObjPtr<String>::DownCast(this);
}
@@ -347,19 +349,24 @@
inline size_t Object::SizeOf() {
// Read barrier is never required for SizeOf since objects sizes are constant. Reading from-space
// values is OK because of that.
- static constexpr ReadBarrierOption kRBO = kWithoutReadBarrier;
size_t result;
constexpr VerifyObjectFlags kNewFlags = RemoveThisFlags(kVerifyFlags);
if (IsArrayInstance<kVerifyFlags>()) {
- result = AsArray<kNewFlags>()->template SizeOf<kNewFlags, kRBO>();
+ result = AsArray<kNewFlags>()->template SizeOf<kNewFlags>();
} else if (IsClass<kNewFlags>()) {
- result = AsClass<kNewFlags>()->template SizeOf<kNewFlags, kRBO>();
- } else if (GetClass<kNewFlags, kRBO>()->IsStringClass()) {
- result = AsString<kNewFlags, kRBO>()->template SizeOf<kNewFlags>();
+ result = AsClass<kNewFlags>()->template SizeOf<kNewFlags>();
+ } else if (IsString<kNewFlags>()) {
+ result = AsString<kNewFlags>()->template SizeOf<kNewFlags>();
} else {
- result = GetClass<kNewFlags, kRBO>()->template GetObjectSize<kNewFlags>();
+ result = GetClass<kNewFlags, kWithoutReadBarrier>()->template GetObjectSize<kNewFlags>();
}
- DCHECK_GE(result, sizeof(Object)) << " class=" << Class::PrettyClass(GetClass<kNewFlags, kRBO>());
+ DCHECK_GE(result, sizeof(Object)) << " class="
+ // Note: Class::PrettyClass() is reading constant reference fields to get to constant
+ // primitive fields and safely avoids read barriers, so it is safe to call on a Class
+ // reference read without read barrier from a constant reference field.
+ // See ReadBarrierOption. And, for correctness, we actually have to avoid the read
+ // barrier here if Object::SizeOf() is called on a from-space reference.
+ << GetClass<kNewFlags, kWithoutReadBarrier>()->PrettyClass();
return result;
}
diff --git a/runtime/mirror/object.h b/runtime/mirror/object.h
index e681835..e6e9160 100644
--- a/runtime/mirror/object.h
+++ b/runtime/mirror/object.h
@@ -242,12 +242,10 @@
template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags>
ObjPtr<DoubleArray> AsDoubleArray() REQUIRES_SHARED(Locks::mutator_lock_);
- template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags,
- ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
+ template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags>
bool IsString() REQUIRES_SHARED(Locks::mutator_lock_);
- template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags,
- ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
+ template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags>
ObjPtr<String> AsString() REQUIRES_SHARED(Locks::mutator_lock_);
template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags>