Address read barrier issue with cl/106467
And tidy/add a check.
Bug: 12687968
Change-Id: If63dc0d9d0a0ce5f2eeb81734ff8f4307865f67d
diff --git a/runtime/indirect_reference_table-inl.h b/runtime/indirect_reference_table-inl.h
index 9bf3ea2..9ee6d89 100644
--- a/runtime/indirect_reference_table-inl.h
+++ b/runtime/indirect_reference_table-inl.h
@@ -27,15 +27,6 @@
class Object;
} // namespace mirror
-inline void IrtIterator::SkipNullsAndTombstones() {
- // We skip NULLs and tombstones. Clients don't want to see implementation details.
- while (i_ < capacity_ &&
- (table_[i_].IsNull() ||
- Runtime::Current()->IsClearedJniWeakGlobal(table_[i_].Read<kWithoutReadBarrier>()))) {
- ++i_;
- }
-}
-
// Verifies that the indirect table lookup is valid.
// Returns "false" if something looks bad.
inline bool IndirectReferenceTable::GetChecked(IndirectRef iref) const {
diff --git a/runtime/indirect_reference_table.cc b/runtime/indirect_reference_table.cc
index 49bffa4..2278408 100644
--- a/runtime/indirect_reference_table.cc
+++ b/runtime/indirect_reference_table.cc
@@ -161,10 +161,12 @@
}
void IndirectReferenceTable::AssertEmpty() {
- if (UNLIKELY(begin() != end())) {
- ScopedObjectAccess soa(Thread::Current());
- LOG(FATAL) << "Internal Error: non-empty local reference table\n"
- << MutatorLockedDumpable<IndirectReferenceTable>(*this);
+ for (size_t i = 0; i < Capacity(); ++i) {
+ if (!table_[i].IsNull()) {
+ ScopedObjectAccess soa(Thread::Current());
+ LOG(FATAL) << "Internal Error: non-empty local reference table\n"
+ << MutatorLockedDumpable<IndirectReferenceTable>(*this);
+ }
}
}
@@ -264,6 +266,11 @@
void IndirectReferenceTable::VisitRoots(RootCallback* callback, void* arg, uint32_t tid,
RootType root_type) {
for (auto ref : *this) {
+ if (*ref == nullptr) {
+ // Need to skip null entries to make it possible to do the
+ // non-null check after the call back.
+ continue;
+ }
callback(ref, arg, tid, root_type);
DCHECK(*ref != nullptr);
}
diff --git a/runtime/indirect_reference_table.h b/runtime/indirect_reference_table.h
index 562ba1e..5291e50 100644
--- a/runtime/indirect_reference_table.h
+++ b/runtime/indirect_reference_table.h
@@ -206,12 +206,10 @@
explicit IrtIterator(GcRoot<mirror::Object>* table, size_t i, size_t capacity)
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_)
: table_(table), i_(i), capacity_(capacity) {
- SkipNullsAndTombstones();
}
IrtIterator& operator++() SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
++i_;
- SkipNullsAndTombstones();
return *this;
}
@@ -225,8 +223,6 @@
}
private:
- void SkipNullsAndTombstones() SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
-
GcRoot<mirror::Object>* const table_;
size_t i_;
const size_t capacity_;
diff --git a/runtime/java_vm_ext.cc b/runtime/java_vm_ext.cc
index 424addb..0ac5b88 100644
--- a/runtime/java_vm_ext.cc
+++ b/runtime/java_vm_ext.cc
@@ -760,6 +760,11 @@
for (mirror::Object** entry : weak_globals_) {
// Since this is called by the GC, we don't need a read barrier.
mirror::Object* obj = *entry;
+ if (obj == nullptr) {
+ // Need to skip null here to distinguish between null entries
+ // and cleared weak ref entries.
+ continue;
+ }
mirror::Object* new_obj = callback(obj, arg);
if (new_obj == nullptr) {
new_obj = Runtime::Current()->GetClearedJniWeakGlobal();
diff --git a/runtime/runtime-inl.h b/runtime/runtime-inl.h
index 8b632b2..fe05073 100644
--- a/runtime/runtime-inl.h
+++ b/runtime/runtime-inl.h
@@ -29,9 +29,7 @@
inline mirror::Object* Runtime::GetClearedJniWeakGlobal() {
mirror::Object* obj = sentinel_.Read();
- if (obj == nullptr) {
- LOG(ERROR) << "Failed to return cleared JNI weak global sentinel";
- }
+ DCHECK(obj != nullptr);
return obj;
}
diff --git a/runtime/runtime.cc b/runtime/runtime.cc
index 89ad505..f258a30 100644
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -809,6 +809,7 @@
// Initialize the special sentinel_ value early.
sentinel_ = GcRoot<mirror::Object>(class_linker_->AllocObject(self));
+ CHECK(sentinel_.Read() != nullptr);
verifier::MethodVerifier::Init();