Fix vtable corruption issue
am: b47a1cc17f
Change-Id: I0f94ef0789b8a15525e38765e45c4a3383af643e
diff --git a/build/Android.gtest.mk b/build/Android.gtest.mk
index c911a17..011c487 100644
--- a/build/Android.gtest.mk
+++ b/build/Android.gtest.mk
@@ -37,6 +37,7 @@
MyClassNatives \
Nested \
NonStaticLeafMethods \
+ Packages \
ProtoCompare \
ProtoCompare2 \
ProfileTestMultiDex \
@@ -69,7 +70,7 @@
ART_GTEST_class_linker_test_DEX_DEPS := Interfaces MultiDex MyClass Nested Statics StaticsFromCode
ART_GTEST_compiler_driver_test_DEX_DEPS := AbstractMethod StaticLeafMethods ProfileTestMultiDex
-ART_GTEST_dex_cache_test_DEX_DEPS := Main
+ART_GTEST_dex_cache_test_DEX_DEPS := Main Packages
ART_GTEST_dex_file_test_DEX_DEPS := GetMethodSignature Main Nested
ART_GTEST_dex2oat_test_DEX_DEPS := $(ART_GTEST_dex2oat_environment_tests_DEX_DEPS)
ART_GTEST_exception_test_DEX_DEPS := ExceptionHandle
diff --git a/runtime/mirror/class-inl.h b/runtime/mirror/class-inl.h
index 0d95bb1..934a73b 100644
--- a/runtime/mirror/class-inl.h
+++ b/runtime/mirror/class-inl.h
@@ -368,8 +368,21 @@
// class rather than the declaring class itself.
DexCache* referrer_dex_cache = use_referrers_cache ? this->GetDexCache() : dex_cache;
uint32_t class_idx = referrer_dex_cache->GetDexFile()->GetFieldId(field_idx).class_idx_;
- // The referenced class has already been resolved with the field, get it from the dex cache.
- Class* dex_access_to = referrer_dex_cache->GetResolvedType(class_idx);
+ // The referenced class has already been resolved with the field, but may not be in the dex
+ // cache. Using ResolveType here without handles in the caller should be safe since there
+ // should be no thread suspension due to the class being resolved.
+ // TODO: Clean this up to use handles in the caller.
+ Class* dex_access_to;
+ {
+ StackHandleScope<2> hs(Thread::Current());
+ Handle<mirror::DexCache> h_dex_cache(hs.NewHandle(referrer_dex_cache));
+ Handle<mirror::ClassLoader> h_class_loader(hs.NewHandle(access_to->GetClassLoader()));
+ dex_access_to = Runtime::Current()->GetClassLinker()->ResolveType(
+ *referrer_dex_cache->GetDexFile(),
+ class_idx,
+ h_dex_cache,
+ h_class_loader);
+ }
DCHECK(dex_access_to != nullptr);
if (UNLIKELY(!this->CanAccess(dex_access_to))) {
if (throw_on_failure) {
@@ -398,8 +411,21 @@
// class rather than the declaring class itself.
DexCache* referrer_dex_cache = use_referrers_cache ? this->GetDexCache() : dex_cache;
uint32_t class_idx = referrer_dex_cache->GetDexFile()->GetMethodId(method_idx).class_idx_;
- // The referenced class has already been resolved with the method, get it from the dex cache.
- Class* dex_access_to = referrer_dex_cache->GetResolvedType(class_idx);
+ // The referenced class has already been resolved with the method, but may not be in the dex
+ // cache. Using ResolveType here without handles in the caller should be safe since there
+ // should be no thread suspension due to the class being resolved.
+ // TODO: Clean this up to use handles in the caller.
+ Class* dex_access_to;
+ {
+ StackHandleScope<2> hs(Thread::Current());
+ Handle<mirror::DexCache> h_dex_cache(hs.NewHandle(referrer_dex_cache));
+ Handle<mirror::ClassLoader> h_class_loader(hs.NewHandle(access_to->GetClassLoader()));
+ dex_access_to = Runtime::Current()->GetClassLinker()->ResolveType(
+ *referrer_dex_cache->GetDexFile(),
+ class_idx,
+ h_dex_cache,
+ h_class_loader);
+ }
DCHECK(dex_access_to != nullptr);
if (UNLIKELY(!this->CanAccess(dex_access_to))) {
if (throw_on_failure) {
diff --git a/runtime/mirror/class.h b/runtime/mirror/class.h
index c1504de..26dae7c 100644
--- a/runtime/mirror/class.h
+++ b/runtime/mirror/class.h
@@ -1476,6 +1476,7 @@
// java.lang.Class
static GcRoot<Class> java_lang_Class_;
+ ART_FRIEND_TEST(DexCacheTest, TestResolvedFieldAccess); // For ResolvedFieldAccessTest
friend struct art::ClassOffsets; // for verifying offset information
friend class Object; // For VisitReferences
DISALLOW_IMPLICIT_CONSTRUCTORS(Class);
diff --git a/runtime/mirror/dex_cache_test.cc b/runtime/mirror/dex_cache_test.cc
index 48f2ca5..0c8a782 100644
--- a/runtime/mirror/dex_cache_test.cc
+++ b/runtime/mirror/dex_cache_test.cc
@@ -62,5 +62,33 @@
EXPECT_TRUE(linear_alloc->Contains(klass->GetDexCache()->GetResolvedMethods()));
}
+TEST_F(DexCacheTest, TestResolvedFieldAccess) {
+ ScopedObjectAccess soa(Thread::Current());
+ jobject jclass_loader(LoadDex("Packages"));
+ ASSERT_TRUE(jclass_loader != nullptr);
+ Runtime* const runtime = Runtime::Current();
+ ClassLinker* const class_linker = runtime->GetClassLinker();
+ StackHandleScope<3> hs(soa.Self());
+ Handle<mirror::ClassLoader> class_loader(hs.NewHandle(
+ soa.Decode<mirror::ClassLoader*>(jclass_loader)));
+ Handle<mirror::Class> klass1 =
+ hs.NewHandle(class_linker->FindClass(soa.Self(), "Lpackage1/Package1;", class_loader));
+ ASSERT_TRUE(klass1.Get() != nullptr);
+ Handle<mirror::Class> klass2 =
+ hs.NewHandle(class_linker->FindClass(soa.Self(), "Lpackage2/Package2;", class_loader));
+ ASSERT_TRUE(klass2.Get() != nullptr);
+ EXPECT_EQ(klass1->GetDexCache(), klass2->GetDexCache());
+
+ EXPECT_NE(klass1->NumStaticFields(), 0u);
+ for (ArtField& field : klass2->GetSFields()) {
+ EXPECT_FALSE((
+ klass1->ResolvedFieldAccessTest</*throw_on_failure*/ false,
+ /*use_referrers_cache*/ false>(klass2.Get(),
+ &field,
+ field.GetDexFieldIndex(),
+ klass1->GetDexCache())));
+ }
+}
+
} // namespace mirror
} // namespace art
diff --git a/test/Packages/Package1.java b/test/Packages/Package1.java
new file mode 100644
index 0000000..6d58246
--- /dev/null
+++ b/test/Packages/Package1.java
@@ -0,0 +1,20 @@
+/*
+ * Copyright (C) 2016 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.
+ */
+
+package package1;
+class Package1 {
+ static int someField;
+}
diff --git a/test/Packages/Package2.java b/test/Packages/Package2.java
new file mode 100644
index 0000000..9ae370a
--- /dev/null
+++ b/test/Packages/Package2.java
@@ -0,0 +1,20 @@
+/*
+ * Copyright (C) 2016 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.
+ */
+
+package package2;
+class Package2 {
+ static int someField;
+}