Don't assume resolved classses are filled in for ResolvedFieldAccessTest
The assumption that the dex cache type for class of the field is
resolved is not correct since FindClass does not fill in the
resolved type array. This resulted in crashes if dex_access_to ==
nullptr.
Also implemented for ResolvedMethodAccessTest
Bug: 31277064
Test: Launch ZeroTouch with profile guided compilation.
Change-Id: I40cfb15f3ae4fbfe941fd01ea46211bd86b6e6f7
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;
+}
