diff options
author | Andreas Gampe <agampe@google.com> | 2015-04-08 10:26:16 -0700 |
---|---|---|
committer | Andreas Gampe <agampe@google.com> | 2015-04-09 09:09:11 -0700 |
commit | 3f5881fda3606b27e30bf903052c73b03910f90b (patch) | |
tree | 0c60e00a3923d47658e2d224997ab2742c113877 /runtime | |
parent | 1576be32be4a99a1cffdaaf209a3cd67e8b2f88a (diff) | |
download | android_art-3f5881fda3606b27e30bf903052c73b03910f90b.tar.gz android_art-3f5881fda3606b27e30bf903052c73b03910f90b.tar.bz2 android_art-3f5881fda3606b27e30bf903052c73b03910f90b.zip |
ART: IRT refactor
IRT creation might fail. Add a path that allows to bypass the aborts
and instead signal validity. Hide this path with a private constructor,
rewrite users to use a static Create method.
Bug: 20110201
Change-Id: I440499c3372cd7557eb970b70ce2c4543da520e4
Diffstat (limited to 'runtime')
-rw-r--r-- | runtime/indirect_reference_table.cc | 21 | ||||
-rw-r--r-- | runtime/indirect_reference_table.h | 7 | ||||
-rw-r--r-- | runtime/jni_env_ext.cc | 20 | ||||
-rw-r--r-- | runtime/jni_env_ext.h | 8 | ||||
-rw-r--r-- | runtime/thread.cc | 6 |
5 files changed, 54 insertions, 8 deletions
diff --git a/runtime/indirect_reference_table.cc b/runtime/indirect_reference_table.cc index cd59365d0c..5012965eab 100644 --- a/runtime/indirect_reference_table.cc +++ b/runtime/indirect_reference_table.cc @@ -64,7 +64,8 @@ void IndirectReferenceTable::AbortIfNoCheckJNI() { } IndirectReferenceTable::IndirectReferenceTable(size_t initialCount, - size_t maxCount, IndirectRefKind desiredKind) + size_t maxCount, IndirectRefKind desiredKind, + bool abort_on_error) : kind_(desiredKind), max_entries_(maxCount) { CHECK_GT(initialCount, 0U); @@ -75,16 +76,28 @@ IndirectReferenceTable::IndirectReferenceTable(size_t initialCount, const size_t table_bytes = maxCount * sizeof(IrtEntry); table_mem_map_.reset(MemMap::MapAnonymous("indirect ref table", nullptr, table_bytes, PROT_READ | PROT_WRITE, false, false, &error_str)); - CHECK(table_mem_map_.get() != nullptr) << error_str; - CHECK_EQ(table_mem_map_->Size(), table_bytes); + if (abort_on_error) { + CHECK(table_mem_map_.get() != nullptr) << error_str; + CHECK_EQ(table_mem_map_->Size(), table_bytes); + CHECK(table_mem_map_->Begin() != nullptr); + } else if (table_mem_map_.get() == nullptr || + table_mem_map_->Size() != table_bytes || + table_mem_map_->Begin() == nullptr) { + table_mem_map_.reset(); + LOG(ERROR) << error_str; + return; + } table_ = reinterpret_cast<IrtEntry*>(table_mem_map_->Begin()); - CHECK(table_ != nullptr); segment_state_.all = IRT_FIRST_SEGMENT; } IndirectReferenceTable::~IndirectReferenceTable() { } +bool IndirectReferenceTable::IsValid() const { + return table_mem_map_.get() != nullptr; +} + IndirectRef IndirectReferenceTable::Add(uint32_t cookie, mirror::Object* obj) { IRTSegmentState prevState; prevState.all = cookie; diff --git a/runtime/indirect_reference_table.h b/runtime/indirect_reference_table.h index 25b0281767..0072184f62 100644 --- a/runtime/indirect_reference_table.h +++ b/runtime/indirect_reference_table.h @@ -258,10 +258,15 @@ bool inline operator!=(const IrtIterator& lhs, const IrtIterator& rhs) { class IndirectReferenceTable { public: - IndirectReferenceTable(size_t initialCount, size_t maxCount, IndirectRefKind kind); + // WARNING: When using with abort_on_error = false, the object may be in a partially + // initialized state. Use IsValid() to check. + IndirectReferenceTable(size_t initialCount, size_t maxCount, IndirectRefKind kind, + bool abort_on_error = true); ~IndirectReferenceTable(); + bool IsValid() const; + /* * Add a new entry. "obj" must be a valid non-NULL object reference. * diff --git a/runtime/jni_env_ext.cc b/runtime/jni_env_ext.cc index b2d3835405..84fc404b46 100644 --- a/runtime/jni_env_ext.cc +++ b/runtime/jni_env_ext.cc @@ -28,11 +28,29 @@ static constexpr size_t kMonitorsMax = 4096; // Arbitrary sanity check. static constexpr size_t kLocalsInitial = 64; // Arbitrary. +// Checking "locals" requires the mutator lock, but at creation time we're really only interested +// in validity, which isn't changing. To avoid grabbing the mutator lock, factored out and tagged +// with NO_THREAD_SAFETY_ANALYSIS. +static bool CheckLocalsValid(JNIEnvExt* in) NO_THREAD_SAFETY_ANALYSIS { + if (in == nullptr) { + return false; + } + return in->locals.IsValid(); +} + +JNIEnvExt* JNIEnvExt::Create(Thread* self_in, JavaVMExt* vm_in) { + std::unique_ptr<JNIEnvExt> ret(new JNIEnvExt(self_in, vm_in)); + if (CheckLocalsValid(ret.get())) { + return ret.release(); + } + return nullptr; +} + JNIEnvExt::JNIEnvExt(Thread* self_in, JavaVMExt* vm_in) : self(self_in), vm(vm_in), local_ref_cookie(IRT_FIRST_SEGMENT), - locals(kLocalsInitial, kLocalsMax, kLocal), + locals(kLocalsInitial, kLocalsMax, kLocal, false), check_jni(false), critical(0), monitors("monitors", kMonitorsInitial, kMonitorsMax) { diff --git a/runtime/jni_env_ext.h b/runtime/jni_env_ext.h index af87cb4226..29d912cb01 100644 --- a/runtime/jni_env_ext.h +++ b/runtime/jni_env_ext.h @@ -34,7 +34,8 @@ class JavaVMExt; static constexpr size_t kLocalsMax = 512; struct JNIEnvExt : public JNIEnv { - JNIEnvExt(Thread* self, JavaVMExt* vm); + static JNIEnvExt* Create(Thread* self, JavaVMExt* vm); + ~JNIEnvExt(); void DumpReferenceTables(std::ostream& os) @@ -87,6 +88,11 @@ struct JNIEnvExt : public JNIEnv { // Used by -Xcheck:jni. const JNINativeInterface* unchecked_functions; + + private: + // The constructor should not be called directly. It may leave the object in an erronuous state, + // and the result needs to be checked. + JNIEnvExt(Thread* self, JavaVMExt* vm); }; // Used to save and restore the JNIEnvExt state when not going through code created by the JNI diff --git a/runtime/thread.cc b/runtime/thread.cc index d1b0464906..89fc00eb99 100644 --- a/runtime/thread.cc +++ b/runtime/thread.cc @@ -377,7 +377,11 @@ bool Thread::Init(ThreadList* thread_list, JavaVMExt* java_vm) { tls32_.thin_lock_thread_id = thread_list->AllocThreadId(this); - tlsPtr_.jni_env = new JNIEnvExt(this, java_vm); + tlsPtr_.jni_env = JNIEnvExt::Create(this, java_vm); + if (tlsPtr_.jni_env == nullptr) { + return false; + } + thread_list->Register(this); return true; } |