diff options
author | Mathieu Chartier <mathieuc@google.com> | 2014-03-06 18:11:53 -0800 |
---|---|---|
committer | Mathieu Chartier <mathieuc@google.com> | 2014-03-07 12:09:04 -0800 |
commit | c645f1ddb7c40bea6a38eda4b3f83f6b6dec405b (patch) | |
tree | de6141864c1c011216c19dd99a2c1e2bc442dd6d | |
parent | a9d7be62735e3356cef7e8ed797c519134a17061 (diff) | |
download | art-c645f1ddb7c40bea6a38eda4b3f83f6b6dec405b.zip art-c645f1ddb7c40bea6a38eda4b3f83f6b6dec405b.tar.gz art-c645f1ddb7c40bea6a38eda4b3f83f6b6dec405b.tar.bz2 |
Add more VerifyObject calls.
Added verify object calls to SirtRef, IndirectReferenceTable,
ReferenceTable.
Removed un-needed verify object in ScopedObjectAccess / DecodeJObject
since object sources are handled.
Bug: 12934910
Change-Id: I55a46a8ea61fed2a77526eda27fd2cce97a9b125
34 files changed, 105 insertions, 50 deletions
diff --git a/compiler/dex/mir_field_info.cc b/compiler/dex/mir_field_info.cc index 96eda01..7c630e8 100644 --- a/compiler/dex/mir_field_info.cc +++ b/compiler/dex/mir_field_info.cc @@ -24,7 +24,7 @@ #include "mirror/class_loader.h" // Only to allow casts in SirtRef<ClassLoader>. #include "mirror/dex_cache.h" // Only to allow casts in SirtRef<DexCache>. #include "scoped_thread_state_change.h" -#include "sirt_ref.h" +#include "sirt_ref-inl.h" namespace art { diff --git a/compiler/driver/compiler_driver-inl.h b/compiler/driver/compiler_driver-inl.h index d401398..1499ae4 100644 --- a/compiler/driver/compiler_driver-inl.h +++ b/compiler/driver/compiler_driver-inl.h @@ -25,6 +25,7 @@ #include "mirror/dex_cache.h" #include "mirror/art_field-inl.h" #include "scoped_thread_state_change.h" +#include "sirt_ref-inl.h" namespace art { diff --git a/compiler/driver/compiler_driver.cc b/compiler/driver/compiler_driver.cc index fc22add..d3d58c9 100644 --- a/compiler/driver/compiler_driver.cc +++ b/compiler/driver/compiler_driver.cc @@ -49,6 +49,7 @@ #include "mirror/throwable.h" #include "scoped_thread_state_change.h" #include "ScopedLocalRef.h" +#include "sirt_ref-inl.h" #include "thread.h" #include "thread_pool.h" #include "trampolines/trampoline_compiler.h" diff --git a/compiler/driver/compiler_driver.h b/compiler/driver/compiler_driver.h index 80a6796..ac70e5a 100644 --- a/compiler/driver/compiler_driver.h +++ b/compiler/driver/compiler_driver.h @@ -52,6 +52,7 @@ struct InlineIGetIPutData; class OatWriter; class ParallelCompilationManager; class ScopedObjectAccess; +template<class T> class SirtRef; class TimingLogger; class VerificationResults; class VerifiedMethod; diff --git a/compiler/driver/compiler_driver_test.cc b/compiler/driver/compiler_driver_test.cc index 2b3af62..949fade 100644 --- a/compiler/driver/compiler_driver_test.cc +++ b/compiler/driver/compiler_driver_test.cc @@ -30,6 +30,7 @@ #include "mirror/dex_cache-inl.h" #include "mirror/object_array-inl.h" #include "mirror/object-inl.h" +#include "sirt_ref-inl.h" namespace art { diff --git a/compiler/image_writer.cc b/compiler/image_writer.cc index 74be604..f4b507a 100644 --- a/compiler/image_writer.cc +++ b/compiler/image_writer.cc @@ -50,7 +50,7 @@ #include "object_utils.h" #include "runtime.h" #include "scoped_thread_state_change.h" -#include "sirt_ref.h" +#include "sirt_ref-inl.h" #include "UniquePtr.h" #include "utils.h" diff --git a/compiler/oat_writer.cc b/compiler/oat_writer.cc index 5394bbc..ffd7b41 100644 --- a/compiler/oat_writer.cc +++ b/compiler/oat_writer.cc @@ -33,6 +33,7 @@ #include "output_stream.h" #include "safe_map.h" #include "scoped_thread_state_change.h" +#include "sirt_ref-inl.h" #include "verifier/method_verifier.h" namespace art { diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc index 67a9e06..7c81ffb 100644 --- a/dex2oat/dex2oat.cc +++ b/dex2oat/dex2oat.cc @@ -54,7 +54,6 @@ #include "runtime.h" #include "ScopedLocalRef.h" #include "scoped_thread_state_change.h" -#include "sirt_ref.h" #include "vector_output_stream.h" #include "well_known_classes.h" #include "zip_archive.h" diff --git a/runtime/class_linker-inl.h b/runtime/class_linker-inl.h index 754d1dd..6c53563 100644 --- a/runtime/class_linker-inl.h +++ b/runtime/class_linker-inl.h @@ -24,7 +24,7 @@ #include "mirror/iftable.h" #include "mirror/object_array.h" #include "object_utils.h" -#include "sirt_ref.h" +#include "sirt_ref-inl.h" namespace art { diff --git a/runtime/entrypoints/entrypoint_utils.h b/runtime/entrypoints/entrypoint_utils.h index 498ac2c..8b48b36 100644 --- a/runtime/entrypoints/entrypoint_utils.h +++ b/runtime/entrypoints/entrypoint_utils.h @@ -30,7 +30,7 @@ #include "mirror/object-inl.h" #include "mirror/throwable.h" #include "object_utils.h" -#include "sirt_ref.h" +#include "sirt_ref-inl.h" #include "thread.h" namespace art { diff --git a/runtime/entrypoints/quick/quick_jni_entrypoints.cc b/runtime/entrypoints/quick/quick_jni_entrypoints.cc index 737fa3e..116957d 100644 --- a/runtime/entrypoints/quick/quick_jni_entrypoints.cc +++ b/runtime/entrypoints/quick/quick_jni_entrypoints.cc @@ -24,6 +24,7 @@ #include "object_utils.h" #include "scoped_thread_state_change.h" #include "thread.h" +#include "verify_object-inl.h" namespace art { diff --git a/runtime/gc/heap-inl.h b/runtime/gc/heap-inl.h index 3d2f7ea..b80e72e 100644 --- a/runtime/gc/heap-inl.h +++ b/runtime/gc/heap-inl.h @@ -24,8 +24,8 @@ #include "gc/space/dlmalloc_space-inl.h" #include "gc/space/large_object_space.h" #include "gc/space/rosalloc_space-inl.h" -#include "object_utils.h" #include "runtime.h" +#include "sirt_ref-inl.h" #include "thread.h" #include "thread-inl.h" #include "verify_object-inl.h" @@ -37,7 +37,9 @@ template <bool kInstrumented, bool kCheckLargeObject, typename PreFenceVisitor> inline mirror::Object* Heap::AllocObjectWithAllocator(Thread* self, mirror::Class* klass, size_t byte_count, AllocatorType allocator, const PreFenceVisitor& pre_fence_visitor) { - DebugCheckPreconditionsForAllocObject(klass, byte_count); + if (kIsDebugBuild) { + CheckPreconditionsForAllocObject(klass, byte_count); + } // Since allocation can cause a GC which will need to SuspendAll, make sure all allocations are // done in the runnable state where suspension is expected. DCHECK_EQ(self->GetState(), kRunnable); @@ -226,13 +228,6 @@ inline mirror::Object* Heap::TryToAllocate(Thread* self, AllocatorType allocator return ret; } -inline void Heap::DebugCheckPreconditionsForAllocObject(mirror::Class* c, size_t byte_count) { - DCHECK(c == NULL || (c->IsClassClass() && byte_count >= sizeof(mirror::Class)) || - (c->IsVariableSize() || c->GetObjectSize() == byte_count) || - strlen(ClassHelper(c).GetDescriptor()) == 0); - DCHECK_GE(byte_count, sizeof(mirror::Object)); -} - inline Heap::AllocationTimer::AllocationTimer(Heap* heap, mirror::Object** allocated_obj_ptr) : heap_(heap), allocated_obj_ptr_(allocated_obj_ptr) { if (kMeasureAllocationTime) { diff --git a/runtime/gc/heap.cc b/runtime/gc/heap.cc index 2497e6a..87b4e60 100644 --- a/runtime/gc/heap.cc +++ b/runtime/gc/heap.cc @@ -2708,5 +2708,12 @@ void Heap::AddModUnionTable(accounting::ModUnionTable* mod_union_table) { mod_union_tables_.Put(mod_union_table->GetSpace(), mod_union_table); } +void Heap::CheckPreconditionsForAllocObject(mirror::Class* c, size_t byte_count) { + CHECK(c == NULL || (c->IsClassClass() && byte_count >= sizeof(mirror::Class)) || + (c->IsVariableSize() || c->GetObjectSize() == byte_count) || + strlen(ClassHelper(c).GetDescriptor()) == 0); + CHECK_GE(byte_count, sizeof(mirror::Object)); +} + } // namespace gc } // namespace art diff --git a/runtime/gc/heap.h b/runtime/gc/heap.h index 4c4e943..3a8739a 100644 --- a/runtime/gc/heap.h +++ b/runtime/gc/heap.h @@ -192,7 +192,7 @@ class Heap { void SwapSemiSpaces() EXCLUSIVE_LOCKS_REQUIRED(Locks::mutator_lock_); - void DebugCheckPreconditionsForAllocObject(mirror::Class* c, size_t byte_count) + void CheckPreconditionsForAllocObject(mirror::Class* c, size_t byte_count) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); void ThrowOutOfMemoryError(size_t byte_count, bool large_object_allocation); diff --git a/runtime/gc/space/malloc_space.cc b/runtime/gc/space/malloc_space.cc index 61d1071..dac043e 100644 --- a/runtime/gc/space/malloc_space.cc +++ b/runtime/gc/space/malloc_space.cc @@ -24,6 +24,7 @@ #include "mirror/class-inl.h" #include "mirror/object-inl.h" #include "runtime.h" +#include "sirt_ref-inl.h" #include "thread.h" #include "thread_list.h" #include "utils.h" diff --git a/runtime/indirect_reference_table.cc b/runtime/indirect_reference_table.cc index c8855e3..ed3fb5f 100644 --- a/runtime/indirect_reference_table.cc +++ b/runtime/indirect_reference_table.cc @@ -355,4 +355,13 @@ 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)];; + VerifyObject(obj); + return obj; +} + } // namespace art diff --git a/runtime/indirect_reference_table.h b/runtime/indirect_reference_table.h index 9a8e4f2..a2de726 100644 --- a/runtime/indirect_reference_table.h +++ b/runtime/indirect_reference_table.h @@ -267,12 +267,7 @@ class IndirectReferenceTable { * * Returns kInvalidIndirectRefObject if iref is invalid. */ - mirror::Object* Get(IndirectRef iref) const { - if (!GetChecked(iref)) { - return kInvalidIndirectRefObject; - } - return table_[ExtractIndex(iref)]; - } + 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; diff --git a/runtime/jni_internal.cc b/runtime/jni_internal.cc index 4fad5c9..f8865ea 100644 --- a/runtime/jni_internal.cc +++ b/runtime/jni_internal.cc @@ -2537,11 +2537,13 @@ class JNI { IndirectRef ref = reinterpret_cast<IndirectRef>(java_object); IndirectRefKind kind = GetIndirectRefKind(ref); switch (kind) { - case kLocal: + case kLocal: { + ScopedObjectAccess soa(env); if (static_cast<JNIEnvExt*>(env)->locals.Get(ref) != kInvalidIndirectRefObject) { return JNILocalRefType; } return JNIInvalidRefType; + } case kGlobal: return JNIGlobalRefType; case kWeakGlobal: @@ -3192,7 +3194,11 @@ mirror::Object* JavaVMExt::DecodeWeakGlobal(Thread* self, IndirectRef ref) { while (UNLIKELY(!allow_new_weak_globals_)) { weak_globals_add_condition_.WaitHoldingLocks(self); } - return const_cast<mirror::Object*>(weak_globals_.Get(ref)); + mirror::Object* obj = weak_globals_.Get(ref); + if (obj != kClearedJniWeakGlobal) { + VerifyObject(obj); + } + return obj; } void JavaVMExt::DumpReferenceTables(std::ostream& os) { diff --git a/runtime/jni_internal.h b/runtime/jni_internal.h index 606d5d1..7b49d33 100644 --- a/runtime/jni_internal.h +++ b/runtime/jni_internal.h @@ -25,7 +25,6 @@ #include "object_callbacks.h" #include "reference_table.h" #include "runtime.h" -#include "sirt_ref.h" #include <iosfwd> #include <string> @@ -48,6 +47,7 @@ union JValue; class Libraries; class ParsedOptions; class ScopedObjectAccess; +template<class T> class SirtRef; class Thread; void JniAbortF(const char* jni_function_name, const char* fmt, ...) @@ -101,7 +101,8 @@ class JavaVMExt : public JavaVM { void DeleteWeakGlobalRef(Thread* self, jweak obj) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); void SweepJniWeakGlobals(IsMarkedCallback* callback, void* arg); - mirror::Object* DecodeWeakGlobal(Thread* self, IndirectRef ref); + mirror::Object* DecodeWeakGlobal(Thread* self, IndirectRef ref) + SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); Runtime* runtime; diff --git a/runtime/mirror/class.h b/runtime/mirror/class.h index f9a5ea2..76ab94c 100644 --- a/runtime/mirror/class.h +++ b/runtime/mirror/class.h @@ -17,7 +17,6 @@ #ifndef ART_RUNTIME_MIRROR_CLASS_H_ #define ART_RUNTIME_MIRROR_CLASS_H_ -#include "gc/heap.h" #include "invoke_type.h" #include "modifiers.h" #include "object.h" diff --git a/runtime/mirror/stack_trace_element.cc b/runtime/mirror/stack_trace_element.cc index 02a396a..5217e5e 100644 --- a/runtime/mirror/stack_trace_element.cc +++ b/runtime/mirror/stack_trace_element.cc @@ -20,6 +20,7 @@ #include "class-inl.h" #include "gc/accounting/card_table-inl.h" #include "object-inl.h" +#include "sirt_ref-inl.h" #include "string.h" namespace art { diff --git a/runtime/mirror/stack_trace_element.h b/runtime/mirror/stack_trace_element.h index 779ec4b..9e023c7 100644 --- a/runtime/mirror/stack_trace_element.h +++ b/runtime/mirror/stack_trace_element.h @@ -18,10 +18,10 @@ #define ART_RUNTIME_MIRROR_STACK_TRACE_ELEMENT_H_ #include "object.h" -#include "sirt_ref.h" namespace art { +template<class T> class SirtRef; struct StackTraceElementOffsets; namespace mirror { diff --git a/runtime/monitor.h b/runtime/monitor.h index eb07196..55504b5 100644 --- a/runtime/monitor.h +++ b/runtime/monitor.h @@ -31,6 +31,8 @@ namespace art { +template<class T> class SirtRef; + namespace mirror { class ArtMethod; class Object; diff --git a/runtime/native/java_lang_Runtime.cc b/runtime/native/java_lang_Runtime.cc index 0629f4d..f6149ff 100644 --- a/runtime/native/java_lang_Runtime.cc +++ b/runtime/native/java_lang_Runtime.cc @@ -24,6 +24,7 @@ #include "runtime.h" #include "scoped_thread_state_change.h" #include "ScopedUtfChars.h" +#include "sirt_ref-inl.h" namespace art { diff --git a/runtime/native/scoped_fast_native_object_access.h b/runtime/native/scoped_fast_native_object_access.h index b5ee748..645d78c 100644 --- a/runtime/native/scoped_fast_native_object_access.h +++ b/runtime/native/scoped_fast_native_object_access.h @@ -80,8 +80,6 @@ class ScopedFastNativeObjectAccess { return NULL; } - VerifyObject(obj); - DCHECK_NE((reinterpret_cast<uintptr_t>(obj) & 0xffff0000), 0xebad0000); IndirectReferenceTable& locals = Env()->locals; diff --git a/runtime/object_utils.h b/runtime/object_utils.h index 4eac291..dd2bd4f 100644 --- a/runtime/object_utils.h +++ b/runtime/object_utils.h @@ -28,7 +28,7 @@ #include "mirror/string.h" #include "runtime.h" -#include "sirt_ref.h" +#include "sirt_ref-inl.h" #include <string> diff --git a/runtime/reference_table.cc b/runtime/reference_table.cc index f43a15b..a3119bb 100644 --- a/runtime/reference_table.cc +++ b/runtime/reference_table.cc @@ -40,6 +40,7 @@ ReferenceTable::~ReferenceTable() { void ReferenceTable::Add(mirror::Object* obj) { DCHECK(obj != NULL); + VerifyObject(obj); if (entries_.size() >= max_size_) { LOG(FATAL) << "ReferenceTable '" << name_ << "' " << "overflowed (" << max_size_ << " entries)"; diff --git a/runtime/scoped_thread_state_change.h b/runtime/scoped_thread_state_change.h index f0f5ed2..d9e7986 100644 --- a/runtime/scoped_thread_state_change.h +++ b/runtime/scoped_thread_state_change.h @@ -169,8 +169,6 @@ class ScopedObjectAccessUnchecked : public ScopedThreadStateChange { return NULL; } - VerifyObject(obj); - DCHECK_NE((reinterpret_cast<uintptr_t>(obj) & 0xffff0000), 0xebad0000); IndirectReferenceTable& locals = Env()->locals; diff --git a/runtime/sirt_ref-inl.h b/runtime/sirt_ref-inl.h new file mode 100644 index 0000000..7f2d847 --- /dev/null +++ b/runtime/sirt_ref-inl.h @@ -0,0 +1,45 @@ +/* + * 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_SIRT_REF_INL_H_ +#define ART_RUNTIME_SIRT_REF_INL_H_ + +#include "sirt_ref.h" + +#include "verify_object-inl.h" + +namespace art { + +template<class T> inline SirtRef<T>::SirtRef(Thread* self, T* object) : self_(self), sirt_(object) { + VerifyObject(object); + self_->PushSirt(&sirt_); +} + +template<class T> inline SirtRef<T>::~SirtRef() { + StackIndirectReferenceTable* top_sirt = self_->PopSirt(); + DCHECK_EQ(top_sirt, &sirt_); +} + +template<class T> inline T* SirtRef<T>::reset(T* object) { + VerifyObject(object); + T* old_ref = get(); + sirt_.SetReference(0, object); + return old_ref; +} + +} // namespace art + +#endif // ART_RUNTIME_SIRT_REF_INL_H_ diff --git a/runtime/sirt_ref.h b/runtime/sirt_ref.h index 38e652a..2226e17 100644 --- a/runtime/sirt_ref.h +++ b/runtime/sirt_ref.h @@ -28,13 +28,8 @@ namespace art { template<class T> class SirtRef { public: - SirtRef(Thread* self, T* object) : self_(self), sirt_(object) { - self_->PushSirt(&sirt_); - } - ~SirtRef() { - StackIndirectReferenceTable* top_sirt = self_->PopSirt(); - DCHECK_EQ(top_sirt, &sirt_); - } + SirtRef(Thread* self, T* object); + ~SirtRef(); T& operator*() const SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) { return *get(); @@ -47,11 +42,7 @@ class SirtRef { } // Returns the old reference. - T* reset(T* object = nullptr) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) { - T* old_ref = get(); - sirt_.SetReference(0, object); - return old_ref; - } + T* reset(T* object = nullptr) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); private: Thread* const self_; diff --git a/runtime/thread.cc b/runtime/thread.cc index a50fa00..3a70613 100644 --- a/runtime/thread.cc +++ b/runtime/thread.cc @@ -1203,9 +1203,11 @@ mirror::Object* Thread::DecodeJObject(jobject obj) const { if (LIKELY(SirtContains(obj))) { // Read from SIRT. result = reinterpret_cast<StackReference<mirror::Object>*>(obj)->AsMirrorPtr(); + VerifyObject(result); } else if (Runtime::Current()->GetJavaVM()->work_around_app_jni_bugs) { // Assume an invalid local reference is actually a direct pointer. result = reinterpret_cast<mirror::Object*>(obj); + VerifyObject(result); } else { result = kInvalidIndirectRefObject; } @@ -1225,10 +1227,6 @@ mirror::Object* Thread::DecodeJObject(jobject obj) const { if (UNLIKELY(result == nullptr)) { JniAbortF(nullptr, "use of deleted %s %p", ToStr<IndirectRefKind>(kind).c_str(), obj); - } else { - if (result != kInvalidIndirectRefObject) { - VerifyObject(result); - } } return result; } diff --git a/runtime/verifier/method_verifier-inl.h b/runtime/verifier/method_verifier-inl.h index 74c3e33..c554394 100644 --- a/runtime/verifier/method_verifier-inl.h +++ b/runtime/verifier/method_verifier-inl.h @@ -21,6 +21,7 @@ #include "method_verifier.h" #include "mirror/class_loader.h" #include "mirror/dex_cache.h" +#include "sirt_ref-inl.h" namespace art { namespace verifier { diff --git a/runtime/verifier/method_verifier.cc b/runtime/verifier/method_verifier.cc index 555714f..c4c3082 100644 --- a/runtime/verifier/method_verifier.cc +++ b/runtime/verifier/method_verifier.cc @@ -14,7 +14,7 @@ * limitations under the License. */ -#include "method_verifier.h" +#include "method_verifier-inl.h" #include <iostream> @@ -40,6 +40,7 @@ #include "register_line-inl.h" #include "runtime.h" #include "scoped_thread_state_change.h" +#include "sirt_ref-inl.h" #include "verifier/dex_gc_map.h" namespace art { diff --git a/runtime/verifier/method_verifier.h b/runtime/verifier/method_verifier.h index 031cfec..5f13191 100644 --- a/runtime/verifier/method_verifier.h +++ b/runtime/verifier/method_verifier.h @@ -33,12 +33,12 @@ #include "reg_type_cache-inl.h" #include "register_line.h" #include "safe_map.h" -#include "sirt_ref.h" #include "UniquePtr.h" namespace art { struct ReferenceMap2Visitor; +template<class T> class SirtRef; namespace verifier { |