diff options
-rw-r--r-- | runtime/indirect_reference_table-inl.h | 87 | ||||
-rw-r--r-- | runtime/indirect_reference_table.cc | 119 | ||||
-rw-r--r-- | runtime/indirect_reference_table.h | 30 | ||||
-rw-r--r-- | runtime/indirect_reference_table_test.cc | 2 | ||||
-rw-r--r-- | runtime/jni_internal.cc | 1 | ||||
-rw-r--r-- | runtime/jni_internal.h | 3 | ||||
-rw-r--r-- | runtime/jni_internal_test.cc | 3 | ||||
-rw-r--r-- | runtime/thread.cc | 7 |
8 files changed, 132 insertions, 120 deletions
diff --git a/runtime/indirect_reference_table-inl.h b/runtime/indirect_reference_table-inl.h new file mode 100644 index 0000000..1a28347 --- /dev/null +++ b/runtime/indirect_reference_table-inl.h @@ -0,0 +1,87 @@ +/* + * Copyright (C) 2014 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef ART_RUNTIME_INDIRECT_REFERENCE_TABLE_INL_H_ +#define ART_RUNTIME_INDIRECT_REFERENCE_TABLE_INL_H_ + +#include "indirect_reference_table.h" + +#include "verify_object-inl.h" + +namespace art { +namespace mirror { +class Object; +} // namespace mirror + +// Verifies that the indirect table lookup is valid. +// Returns "false" if something looks bad. +inline bool IndirectReferenceTable::GetChecked(IndirectRef iref) const { + if (UNLIKELY(iref == nullptr)) { + LOG(WARNING) << "Attempt to look up NULL " << kind_; + return false; + } + if (UNLIKELY(GetIndirectRefKind(iref) == kSirtOrInvalid)) { + LOG(ERROR) << "JNI ERROR (app bug): invalid " << kind_ << " " << iref; + AbortIfNoCheckJNI(); + return false; + } + const int topIndex = segment_state_.parts.topIndex; + int idx = ExtractIndex(iref); + if (UNLIKELY(idx >= topIndex)) { + LOG(ERROR) << "JNI ERROR (app bug): accessed stale " << kind_ << " " + << iref << " (index " << idx << " in a table of size " << topIndex << ")"; + AbortIfNoCheckJNI(); + return false; + } + if (UNLIKELY(table_[idx] == nullptr)) { + LOG(ERROR) << "JNI ERROR (app bug): accessed deleted " << kind_ << " " << iref; + AbortIfNoCheckJNI(); + return false; + } + if (UNLIKELY(!CheckEntry("use", iref, idx))) { + return false; + } + return true; +} + +// Make sure that the entry at "idx" is correctly paired with "iref". +inline bool IndirectReferenceTable::CheckEntry(const char* what, IndirectRef iref, int idx) const { + const mirror::Object* obj = table_[idx]; + IndirectRef checkRef = ToIndirectRef(obj, idx); + if (UNLIKELY(checkRef != iref)) { + LOG(ERROR) << "JNI ERROR (app bug): attempt to " << what + << " stale " << kind_ << " " << iref + << " (should be " << checkRef << ")"; + AbortIfNoCheckJNI(); + return false; + } + return true; +} + +inline mirror::Object* IndirectReferenceTable::Get(IndirectRef iref) const { + if (!GetChecked(iref)) { + return kInvalidIndirectRefObject; + } + mirror::Object* obj = table_[ExtractIndex(iref)]; + if (LIKELY(obj != kClearedJniWeakGlobal)) { + VerifyObject(obj); + } + return obj; +} + +} // namespace art + +#endif // ART_RUNTIME_INDIRECT_REFERENCE_TABLE_INL_H_ diff --git a/runtime/indirect_reference_table.cc b/runtime/indirect_reference_table.cc index 987df91..b81e43a 100644 --- a/runtime/indirect_reference_table.cc +++ b/runtime/indirect_reference_table.cc @@ -14,7 +14,8 @@ * limitations under the License. */ -#include "indirect_reference_table.h" +#include "indirect_reference_table-inl.h" + #include "jni_internal.h" #include "reference_table.h" #include "runtime.h" @@ -53,7 +54,7 @@ std::ostream& operator<<(std::ostream& os, const MutatorLockedDumpable<T>& rhs) return os; } -static void AbortMaybe() { +void IndirectReferenceTable::AbortIfNoCheckJNI() { // If -Xcheck:jni is on, it'll give a more detailed error before aborting. if (!Runtime::Current()->GetJavaVM()->check_jni) { // Otherwise, we want to abort rather than hand back a bad reference. @@ -67,12 +68,23 @@ IndirectReferenceTable::IndirectReferenceTable(size_t initialCount, CHECK_LE(initialCount, maxCount); CHECK_NE(desiredKind, kSirtOrInvalid); - table_ = reinterpret_cast<mirror::Object**>(malloc(initialCount * sizeof(const mirror::Object*))); - CHECK(table_ != NULL); - memset(table_, 0xd1, initialCount * sizeof(const mirror::Object*)); + std::string error_str; + const size_t initial_bytes = initialCount * sizeof(const mirror::Object*); + const size_t table_bytes = maxCount * sizeof(const mirror::Object*); + table_mem_map_.reset(MemMap::MapAnonymous("indirect ref table", nullptr, table_bytes, + PROT_READ | PROT_WRITE, false, &error_str)); + CHECK(table_mem_map_.get() != nullptr) << error_str; + + table_ = reinterpret_cast<mirror::Object**>(table_mem_map_->Begin()); + CHECK(table_ != nullptr); + memset(table_, 0xd1, initial_bytes); - slot_data_ = reinterpret_cast<IndirectRefSlot*>(calloc(initialCount, sizeof(IndirectRefSlot))); - CHECK(slot_data_ != NULL); + const size_t slot_bytes = maxCount * sizeof(IndirectRefSlot); + slot_mem_map_.reset(MemMap::MapAnonymous("indirect ref table slots", nullptr, slot_bytes, + PROT_READ | PROT_WRITE, false, &error_str)); + CHECK(slot_mem_map_.get() != nullptr) << error_str; + slot_data_ = reinterpret_cast<IndirectRefSlot*>(slot_mem_map_->Begin()); + CHECK(slot_data_ != nullptr); segment_state_.all = IRT_FIRST_SEGMENT; alloc_entries_ = initialCount; @@ -81,25 +93,6 @@ IndirectReferenceTable::IndirectReferenceTable(size_t initialCount, } IndirectReferenceTable::~IndirectReferenceTable() { - free(table_); - free(slot_data_); - table_ = NULL; - slot_data_ = NULL; - alloc_entries_ = max_entries_ = -1; -} - -// Make sure that the entry at "idx" is correctly paired with "iref". -bool IndirectReferenceTable::CheckEntry(const char* what, IndirectRef iref, int idx) const { - const mirror::Object* obj = table_[idx]; - IndirectRef checkRef = ToIndirectRef(obj, idx); - if (UNLIKELY(checkRef != iref)) { - LOG(ERROR) << "JNI ERROR (app bug): attempt to " << what - << " stale " << kind_ << " " << iref - << " (should be " << checkRef << ")"; - AbortMaybe(); - return false; - } - return true; } IndirectRef IndirectReferenceTable::Add(uint32_t cookie, mirror::Object* obj) { @@ -127,20 +120,6 @@ IndirectRef IndirectReferenceTable::Add(uint32_t cookie, mirror::Object* obj) { } DCHECK_GT(newSize, alloc_entries_); - table_ = reinterpret_cast<mirror::Object**>(realloc(table_, newSize * sizeof(mirror::Object*))); - slot_data_ = reinterpret_cast<IndirectRefSlot*>(realloc(slot_data_, - newSize * sizeof(IndirectRefSlot))); - if (table_ == NULL || slot_data_ == NULL) { - LOG(FATAL) << "JNI ERROR (app bug): unable to expand " - << kind_ << " table (from " - << alloc_entries_ << " to " << newSize - << ", max=" << max_entries_ << ")\n" - << MutatorLockedDumpable<IndirectReferenceTable>(*this); - } - - // Clear the newly-allocated slot_data_ elements. - memset(slot_data_ + alloc_entries_, 0, (newSize - alloc_entries_) * sizeof(IndirectRefSlot)); - alloc_entries_ = newSize; } @@ -185,55 +164,6 @@ void IndirectReferenceTable::AssertEmpty() { } } -// Verifies that the indirect table lookup is valid. -// Returns "false" if something looks bad. -bool IndirectReferenceTable::GetChecked(IndirectRef iref) const { - if (UNLIKELY(iref == NULL)) { - LOG(WARNING) << "Attempt to look up NULL " << kind_; - return false; - } - if (UNLIKELY(GetIndirectRefKind(iref) == kSirtOrInvalid)) { - LOG(ERROR) << "JNI ERROR (app bug): invalid " << kind_ << " " << iref; - AbortMaybe(); - return false; - } - - int topIndex = segment_state_.parts.topIndex; - int idx = ExtractIndex(iref); - if (UNLIKELY(idx >= topIndex)) { - LOG(ERROR) << "JNI ERROR (app bug): accessed stale " << kind_ << " " - << iref << " (index " << idx << " in a table of size " << topIndex << ")"; - AbortMaybe(); - return false; - } - - if (UNLIKELY(table_[idx] == NULL)) { - LOG(ERROR) << "JNI ERROR (app bug): accessed deleted " << kind_ << " " << iref; - AbortMaybe(); - return false; - } - - if (UNLIKELY(!CheckEntry("use", iref, idx))) { - return false; - } - - return true; -} - -static int Find(mirror::Object* direct_pointer, int bottomIndex, int topIndex, - mirror::Object** table) { - for (int i = bottomIndex; i < topIndex; ++i) { - if (table[i] == direct_pointer) { - return i; - } - } - return -1; -} - -bool IndirectReferenceTable::ContainsDirectPointer(mirror::Object* direct_pointer) const { - return Find(direct_pointer, 0, segment_state_.parts.topIndex, table_) != -1; -} - // Removes an object. We extract the table offset bits from "iref" // and zap the corresponding entry, leaving a hole if it's not at the top. // If the entry is not between the current top index and the bottom index @@ -346,15 +276,4 @@ void IndirectReferenceTable::Dump(std::ostream& os) const { ReferenceTable::Dump(os, entries); } -mirror::Object* IndirectReferenceTable::Get(IndirectRef iref) const { - if (!GetChecked(iref)) { - return kInvalidIndirectRefObject; - } - mirror::Object* obj = table_[ExtractIndex(iref)];; - if (obj != kClearedJniWeakGlobal) { - VerifyObject(obj); - } - return obj; -} - } // namespace art diff --git a/runtime/indirect_reference_table.h b/runtime/indirect_reference_table.h index a2de726..f365acc 100644 --- a/runtime/indirect_reference_table.h +++ b/runtime/indirect_reference_table.h @@ -24,6 +24,7 @@ #include "base/logging.h" #include "base/mutex.h" +#include "mem_map.h" #include "object_callbacks.h" #include "offsets.h" @@ -72,7 +73,7 @@ class Object; * To make everything fit nicely in 32-bit integers, the maximum size of * the table is capped at 64K. * - * None of the table functions are synchronized. + * Only SynchronizedGet is synchronized. */ /* @@ -191,11 +192,6 @@ static const uint32_t IRT_FIRST_SEGMENT = 0; * and local refs to improve performance. A large circular buffer might * reduce the amortized cost of adding global references. * - * TODO: if we can guarantee that the underlying storage doesn't move, - * e.g. by using oversized mmap regions to handle expanding tables, we may - * be able to avoid having to synchronize lookups. Might make sense to - * add a "synchronized lookup" call that takes the mutex as an argument, - * and either locks or doesn't lock based on internal details. */ union IRTSegmentState { uint32_t all; @@ -234,7 +230,7 @@ class IrtIterator { } } - mirror::Object** table_; + mirror::Object** const table_; size_t i_; size_t capacity_; }; @@ -267,10 +263,15 @@ class IndirectReferenceTable { * * Returns kInvalidIndirectRefObject if iref is invalid. */ - mirror::Object* Get(IndirectRef iref) const SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); - - // TODO: remove when we remove work_around_app_jni_bugs support. - bool ContainsDirectPointer(mirror::Object* direct_pointer) const; + mirror::Object* Get(IndirectRef iref) const SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) + ALWAYS_INLINE; + + // Synchronized get which reads a reference, acquiring a lock if necessary. + mirror::Object* SynchronizedGet(Thread* /*self*/, ReaderWriterMutex* /*mutex*/, + IndirectRef iref) const + SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) { + return Get(iref); + } /* * Remove an existing entry. @@ -351,6 +352,9 @@ class IndirectReferenceTable { } } + // Abort if check_jni is not enabled. + static void AbortIfNoCheckJNI(); + /* extra debugging checks */ bool GetChecked(IndirectRef) const; bool CheckEntry(const char*, IndirectRef, int) const; @@ -358,6 +362,10 @@ class IndirectReferenceTable { /* semi-public - read/write by jni down calls */ IRTSegmentState segment_state_; + // Mem map where we store the indirect refs. + UniquePtr<MemMap> table_mem_map_; + // Mem map where we store the extended debugging info. + UniquePtr<MemMap> slot_mem_map_; /* bottom of the stack */ mirror::Object** table_; /* bit mask, ORed into all irefs */ diff --git a/runtime/indirect_reference_table_test.cc b/runtime/indirect_reference_table_test.cc index 9b42e59..449817a 100644 --- a/runtime/indirect_reference_table_test.cc +++ b/runtime/indirect_reference_table_test.cc @@ -14,7 +14,7 @@ * limitations under the License. */ -#include "indirect_reference_table.h" +#include "indirect_reference_table-inl.h" #include "common_runtime_test.h" #include "mirror/object-inl.h" diff --git a/runtime/jni_internal.cc b/runtime/jni_internal.cc index e6a35d0..915f2c9 100644 --- a/runtime/jni_internal.cc +++ b/runtime/jni_internal.cc @@ -29,6 +29,7 @@ #include "class_linker-inl.h" #include "dex_file-inl.h" #include "gc/accounting/card_table-inl.h" +#include "indirect_reference_table-inl.h" #include "interpreter/interpreter.h" #include "jni.h" #include "mirror/art_field-inl.h" diff --git a/runtime/jni_internal.h b/runtime/jni_internal.h index ec911b2..cdf3c47 100644 --- a/runtime/jni_internal.h +++ b/runtime/jni_internal.h @@ -116,7 +116,8 @@ class JavaVMExt : public JavaVM { // JNI global references. ReaderWriterMutex globals_lock DEFAULT_MUTEX_ACQUIRED_AFTER; - IndirectReferenceTable globals GUARDED_BY(globals_lock); + // Not guarded by globals_lock since we sometimes use SynchronizedGet in Thread::DecodeJObject. + IndirectReferenceTable globals; Mutex libraries_lock DEFAULT_MUTEX_ACQUIRED_AFTER; Libraries* libraries GUARDED_BY(libraries_lock); diff --git a/runtime/jni_internal_test.cc b/runtime/jni_internal_test.cc index 14fc25c..778b9e5 100644 --- a/runtime/jni_internal_test.cc +++ b/runtime/jni_internal_test.cc @@ -987,9 +987,6 @@ TEST_F(JniInternalTest, PushLocalFrame_PopLocalFrame) { // Our local reference for the survivor is invalid because the survivor // gets a new local reference... EXPECT_EQ(JNIInvalidRefType, env_->GetObjectRefType(inner2)); - // ...but the survivor should be in the local reference table. - JNIEnvExt* env = reinterpret_cast<JNIEnvExt*>(env_); - EXPECT_TRUE(env->locals.ContainsDirectPointer(inner2_direct_pointer)); env_->PopLocalFrame(NULL); } diff --git a/runtime/thread.cc b/runtime/thread.cc index 3a62cd5..00a66d7 100644 --- a/runtime/thread.cc +++ b/runtime/thread.cc @@ -44,6 +44,7 @@ #include "gc/accounting/card_table-inl.h" #include "gc/heap.h" #include "gc/space/space.h" +#include "indirect_reference_table-inl.h" #include "jni_internal.h" #include "mirror/art_field-inl.h" #include "mirror/art_method-inl.h" @@ -1265,10 +1266,8 @@ mirror::Object* Thread::DecodeJObject(jobject obj) const { result = kInvalidIndirectRefObject; } } else if (kind == kGlobal) { - JavaVMExt* vm = Runtime::Current()->GetJavaVM(); - IndirectReferenceTable& globals = vm->globals; - ReaderMutexLock mu(const_cast<Thread*>(this), vm->globals_lock); - result = const_cast<mirror::Object*>(globals.Get(ref)); + JavaVMExt* const vm = Runtime::Current()->GetJavaVM(); + result = vm->globals.SynchronizedGet(const_cast<Thread*>(this), &vm->globals_lock, ref); } else { DCHECK_EQ(kind, kWeakGlobal); result = Runtime::Current()->GetJavaVM()->DecodeWeakGlobal(const_cast<Thread*>(this), ref); |