Ensure the GC visits Obsolete Methods
We were previously not visiting obsolete methods during GCs. This
could lead to the use of stale pointers.
Bug: 36335999
Test: ./test/testrunner/testrunner.py --host --interp-ac --gcstress -j40
Change-Id: I2b5c7c75b29f9037204a860501fcdb78104b5e7a
diff --git a/runtime/mirror/class-inl.h b/runtime/mirror/class-inl.h
index 2cff47e..6812026 100644
--- a/runtime/mirror/class-inl.h
+++ b/runtime/mirror/class-inl.h
@@ -29,6 +29,7 @@
#include "dex_file.h"
#include "gc/heap-inl.h"
#include "iftable.h"
+#include "class_ext-inl.h"
#include "object_array-inl.h"
#include "read_barrier-inl.h"
#include "reference-inl.h"
@@ -83,6 +84,12 @@
}
template<VerifyObjectFlags kVerifyFlags, ReadBarrierOption kReadBarrierOption>
+inline ClassExt* Class::GetExtData() {
+ return GetFieldObject<ClassExt, kVerifyFlags, kReadBarrierOption>(
+ OFFSET_OF_OBJECT_MEMBER(Class, ext_data_));
+}
+
+template<VerifyObjectFlags kVerifyFlags, ReadBarrierOption kReadBarrierOption>
inline DexCache* Class::GetDexCache() {
return GetFieldObject<DexCache, kVerifyFlags, kReadBarrierOption>(
OFFSET_OF_OBJECT_MEMBER(Class, dex_cache_));
@@ -951,6 +958,10 @@
for (ArtMethod& method : GetMethods(pointer_size)) {
method.VisitRoots<kReadBarrierOption>(visitor, pointer_size);
}
+ ObjPtr<ClassExt> ext(GetExtData<kDefaultVerifyFlags, kReadBarrierOption>());
+ if (!ext.IsNull()) {
+ ext->VisitNativeRoots<kReadBarrierOption, Visitor>(visitor, pointer_size);
+ }
}
inline IterationRange<StrideIterator<ArtMethod>> Class::GetDirectMethods(PointerSize pointer_size) {
diff --git a/runtime/mirror/class.cc b/runtime/mirror/class.cc
index eb2ec9b..c8ca7b6 100644
--- a/runtime/mirror/class.cc
+++ b/runtime/mirror/class.cc
@@ -64,10 +64,6 @@
java_lang_Class_.VisitRootIfNonNull(visitor, RootInfo(kRootStickyClass));
}
-ClassExt* Class::GetExtData() {
- return GetFieldObject<ClassExt>(OFFSET_OF_OBJECT_MEMBER(Class, ext_data_));
-}
-
ClassExt* Class::EnsureExtDataPresent(Thread* self) {
ObjPtr<ClassExt> existing(GetExtData());
if (!existing.IsNull()) {
diff --git a/runtime/mirror/class.h b/runtime/mirror/class.h
index c52b66a..c294aa1 100644
--- a/runtime/mirror/class.h
+++ b/runtime/mirror/class.h
@@ -1162,6 +1162,8 @@
void SetClinitThreadId(pid_t new_clinit_thread_id) REQUIRES_SHARED(Locks::mutator_lock_);
+ template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags,
+ ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
ClassExt* GetExtData() REQUIRES_SHARED(Locks::mutator_lock_);
// Returns the ExtData for this class, allocating one if necessary. This should be the only way
diff --git a/runtime/mirror/class_ext-inl.h b/runtime/mirror/class_ext-inl.h
new file mode 100644
index 0000000..feaac85
--- /dev/null
+++ b/runtime/mirror/class_ext-inl.h
@@ -0,0 +1,47 @@
+/*
+ * Copyright (C) 2017 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_MIRROR_CLASS_EXT_INL_H_
+#define ART_RUNTIME_MIRROR_CLASS_EXT_INL_H_
+
+#include "class_ext.h"
+
+#include "art_method-inl.h"
+
+namespace art {
+namespace mirror {
+
+template<ReadBarrierOption kReadBarrierOption, class Visitor>
+void ClassExt::VisitNativeRoots(Visitor& visitor, PointerSize pointer_size) {
+ ObjPtr<PointerArray> arr(GetObsoleteMethods<kDefaultVerifyFlags, kReadBarrierOption>());
+ if (arr.IsNull()) {
+ return;
+ }
+ int32_t len = arr->GetLength();
+ for (int32_t i = 0; i < len; i++) {
+ ArtMethod* method = arr->GetElementPtrSize<ArtMethod*,
+ kDefaultVerifyFlags,
+ kReadBarrierOption>(i, pointer_size);
+ if (method != nullptr) {
+ method->VisitRoots<kReadBarrierOption>(visitor, pointer_size);
+ }
+ }
+}
+
+} // namespace mirror
+} // namespace art
+
+#endif // ART_RUNTIME_MIRROR_CLASS_EXT_INL_H_
diff --git a/runtime/mirror/class_ext.cc b/runtime/mirror/class_ext.cc
index 7270079..5dc3aca 100644
--- a/runtime/mirror/class_ext.cc
+++ b/runtime/mirror/class_ext.cc
@@ -14,7 +14,7 @@
* limitations under the License.
*/
-#include "class_ext.h"
+#include "class_ext-inl.h"
#include "art_method-inl.h"
#include "base/casts.h"
@@ -24,7 +24,6 @@
#include "gc/accounting/card_table-inl.h"
#include "object-inl.h"
#include "object_array.h"
-#include "object_array-inl.h"
#include "stack_trace_element.h"
#include "utils.h"
#include "well_known_classes.h"
@@ -34,6 +33,11 @@
GcRoot<Class> ClassExt::dalvik_system_ClassExt_;
+uint32_t ClassExt::ClassSize(PointerSize pointer_size) {
+ uint32_t vtable_entries = Object::kVTableLength;
+ return Class::ComputeClassSize(true, vtable_entries, 0, 0, 0, 0, 0, pointer_size);
+}
+
void ClassExt::SetObsoleteArrays(ObjPtr<PointerArray> methods,
ObjPtr<ObjectArray<DexCache>> dex_caches) {
DCHECK_EQ(GetLockOwnerThreadId(), Thread::Current()->GetThreadId())
diff --git a/runtime/mirror/class_ext.h b/runtime/mirror/class_ext.h
index ad8a61b..fac955a 100644
--- a/runtime/mirror/class_ext.h
+++ b/runtime/mirror/class_ext.h
@@ -17,9 +17,8 @@
#ifndef ART_RUNTIME_MIRROR_CLASS_EXT_H_
#define ART_RUNTIME_MIRROR_CLASS_EXT_H_
-#include "class-inl.h"
-
#include "array.h"
+#include "class.h"
#include "dex_cache.h"
#include "gc_root.h"
#include "object.h"
@@ -36,10 +35,7 @@
// C++ mirror of dalvik.system.ClassExt
class MANAGED ClassExt : public Object {
public:
- static uint32_t ClassSize(PointerSize pointer_size) {
- uint32_t vtable_entries = Object::kVTableLength;
- return Class::ComputeClassSize(true, vtable_entries, 0, 0, 0, 0, 0, pointer_size);
- }
+ static uint32_t ClassSize(PointerSize pointer_size);
// Size of an instance of dalvik.system.ClassExt.
static constexpr uint32_t InstanceSize() {
@@ -57,8 +53,11 @@
OFFSET_OF_OBJECT_MEMBER(ClassExt, obsolete_dex_caches_));
}
- PointerArray* GetObsoleteMethods() REQUIRES_SHARED(Locks::mutator_lock_) {
- return GetFieldObject<PointerArray>(OFFSET_OF_OBJECT_MEMBER(ClassExt, obsolete_methods_));
+ template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags,
+ ReadBarrierOption kReadBarrierOption = kWithReadBarrier>
+ inline PointerArray* GetObsoleteMethods() REQUIRES_SHARED(Locks::mutator_lock_) {
+ return GetFieldObject<PointerArray, kVerifyFlags, kReadBarrierOption>(
+ OFFSET_OF_OBJECT_MEMBER(ClassExt, obsolete_methods_));
}
ByteArray* GetOriginalDexFileBytes() REQUIRES_SHARED(Locks::mutator_lock_) {
@@ -78,6 +77,10 @@
static void ResetClass();
static void VisitRoots(RootVisitor* visitor) REQUIRES_SHARED(Locks::mutator_lock_);
+ template<ReadBarrierOption kReadBarrierOption = kWithReadBarrier, class Visitor>
+ inline void VisitNativeRoots(Visitor& visitor, PointerSize pointer_size)
+ REQUIRES_SHARED(Locks::mutator_lock_);
+
static ClassExt* Alloc(Thread* self) REQUIRES_SHARED(Locks::mutator_lock_);
private:
diff --git a/test/Android.run-test.mk b/test/Android.run-test.mk
index 051ee58..5dae9cf 100644
--- a/test/Android.run-test.mk
+++ b/test/Android.run-test.mk
@@ -374,7 +374,6 @@
# * 961-default-iface-resolution-gen and 964-default-iface-init-genare very long tests that often
# will take more than the timeout to run when gcstress is enabled. This is because gcstress
# slows down allocations significantly which these tests do a lot.
-# * 946-obsolete-throw: b/36335999.
TEST_ART_BROKEN_GCSTRESS_RUN_TESTS := \
137-cfi \
152-dead-large-object \
@@ -383,7 +382,6 @@
913-heaps \
961-default-iface-resolution-gen \
964-default-iface-init-gen \
- 946-obsolete-throw
ifneq (,$(filter gcstress,$(GC_TYPES)))
ART_TEST_KNOWN_BROKEN += $(call all-run-test-names,$(TARGET_TYPES),$(RUN_TYPES),$(PREBUILD_TYPES), \
diff --git a/test/knownfailures.json b/test/knownfailures.json
index 5923ebd..7a7d0e2 100644
--- a/test/knownfailures.json
+++ b/test/knownfailures.json
@@ -343,11 +343,6 @@
"variant": "interpreter | optimizing | regalloc_gc | jit"
},
{
- "tests": "946-obsolete-throw",
- "bug": "http://b/36335999",
- "variant": "interp-ac & gcstress"
- },
- {
"tests": ["912-classes",
"616-cha",
"616-cha-abstract"],