Convert to UTF8 before doing dex file location comparison
Need to have strings in the same format or else the lengths
might not match due to UTF8 conversion.
Added regression test.
Bug: 26799552
(cherry picked from commit 3776db4e51ab61a6e535772417c2adf95920b569)
Change-Id: Ifbc8c38418c9da307a0ff4446d18cea4fb5e71bc
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 85dfab6..336315d 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -2926,19 +2926,19 @@
CHECK(dex_cache.Get() != nullptr) << dex_file.GetLocation();
// For app images, the dex cache location may be a suffix of the dex file location since the
// dex file location is an absolute path.
- const size_t dex_cache_length = dex_cache->GetLocation()->GetLength();
+ const std::string dex_cache_location = dex_cache->GetLocation()->ToModifiedUtf8();
+ const size_t dex_cache_length = dex_cache_location.length();
CHECK_GT(dex_cache_length, 0u) << dex_file.GetLocation();
std::string dex_file_location = dex_file.GetLocation();
CHECK_GE(dex_file_location.length(), dex_cache_length)
- << dex_cache->GetLocation()->ToModifiedUtf8() << " " << dex_file.GetLocation();
+ << dex_cache_location << " " << dex_file.GetLocation();
// Take suffix.
const std::string dex_file_suffix = dex_file_location.substr(
dex_file_location.length() - dex_cache_length,
dex_cache_length);
// Example dex_cache location is SettingsProvider.apk and
// dex file location is /system/priv-app/SettingsProvider/SettingsProvider.apk
- CHECK(dex_cache->GetLocation()->Equals(dex_file_suffix))
- << dex_cache->GetLocation()->ToModifiedUtf8() << " " << dex_file.GetLocation();
+ CHECK_EQ(dex_cache_location, dex_file_suffix);
// Clean up pass to remove null dex caches.
// Null dex caches can occur due to class unloading and we are lazily removing null entries.
JavaVMExt* const vm = self->GetJniEnv()->vm;
diff --git a/runtime/class_linker.h b/runtime/class_linker.h
index d503dd4..4975c29 100644
--- a/runtime/class_linker.h
+++ b/runtime/class_linker.h
@@ -1063,8 +1063,8 @@
friend class ImageWriter; // for GetClassRoots
friend class JniCompilerTest; // for GetRuntimeQuickGenericJniStub
friend class JniInternalTest; // for GetRuntimeQuickGenericJniStub
+ ART_FRIEND_TEST(ClassLinkerTest, RegisterDexFileName); // for DexLock, and RegisterDexFileLocked
ART_FRIEND_TEST(mirror::DexCacheTest, Open); // for AllocDexCache
-
DISALLOW_COPY_AND_ASSIGN(ClassLinker);
};
diff --git a/runtime/class_linker_test.cc b/runtime/class_linker_test.cc
index 471d7ca..40dfda9 100644
--- a/runtime/class_linker_test.cc
+++ b/runtime/class_linker_test.cc
@@ -1201,4 +1201,42 @@
EXPECT_FALSE(statics.Get()->IsBootStrapClassLoaded());
}
+// Regression test for b/26799552.
+TEST_F(ClassLinkerTest, RegisterDexFileName) {
+ ScopedObjectAccess soa(Thread::Current());
+ StackHandleScope<2> hs(soa.Self());
+ ClassLinker* class_linker = Runtime::Current()->GetClassLinker();
+ MutableHandle<mirror::DexCache> dex_cache(hs.NewHandle<mirror::DexCache>(nullptr));
+ {
+ ReaderMutexLock mu(soa.Self(), *class_linker->DexLock());
+ for (const ClassLinker::DexCacheData& data : class_linker->GetDexCachesData()) {
+ dex_cache.Assign(down_cast<mirror::DexCache*>(soa.Self()->DecodeJObject(data.weak_root)));
+ if (dex_cache.Get() != nullptr) {
+ break;
+ }
+ }
+ ASSERT_TRUE(dex_cache.Get() != nullptr);
+ }
+ // Make a copy of the dex cache and change the name.
+ dex_cache.Assign(dex_cache->Clone(soa.Self())->AsDexCache());
+ const uint16_t data[] = { 0x20AC, 0x20A1 };
+ Handle<mirror::String> location(hs.NewHandle(mirror::String::AllocFromUtf16(soa.Self(),
+ arraysize(data),
+ data)));
+ dex_cache->SetLocation(location.Get());
+ const DexFile* old_dex_file = dex_cache->GetDexFile();
+
+ DexFile* dex_file = new DexFile(old_dex_file->Begin(),
+ old_dex_file->Size(),
+ location->ToModifiedUtf8(),
+ 0u,
+ nullptr,
+ nullptr);
+ {
+ WriterMutexLock mu(soa.Self(), *class_linker->DexLock());
+ // Check that inserting with a UTF16 name works.
+ class_linker->RegisterDexFileLocked(*dex_file, dex_cache);
+ }
+}
+
} // namespace art
diff --git a/runtime/dex_file.h b/runtime/dex_file.h
index 968b37b..200121e 100644
--- a/runtime/dex_file.h
+++ b/runtime/dex_file.h
@@ -1260,6 +1260,7 @@
mutable std::unique_ptr<TypeLookupTable> lookup_table_;
friend class DexFileVerifierTest;
+ ART_FRIEND_TEST(ClassLinkerTest, RegisterDexFileName); // for constructor
};
struct DexFileReference {
diff --git a/runtime/mirror/dex_cache.cc b/runtime/mirror/dex_cache.cc
index 7b058d0..692c6cb 100644
--- a/runtime/mirror/dex_cache.cc
+++ b/runtime/mirror/dex_cache.cc
@@ -50,7 +50,7 @@
CHECK_EQ(num_resolved_fields != 0u, resolved_fields != nullptr);
SetDexFile(dex_file);
- SetFieldObject<false>(OFFSET_OF_OBJECT_MEMBER(DexCache, location_), location);
+ SetLocation(location);
SetStrings(strings);
SetResolvedTypes(resolved_types);
SetResolvedMethods(resolved_methods);
@@ -79,5 +79,9 @@
}
}
+void DexCache::SetLocation(mirror::String* location) {
+ SetFieldObject<false>(OFFSET_OF_OBJECT_MEMBER(DexCache, location_), location);
+}
+
} // namespace mirror
} // namespace art
diff --git a/runtime/mirror/dex_cache.h b/runtime/mirror/dex_cache.h
index 5ed061f..0002076 100644
--- a/runtime/mirror/dex_cache.h
+++ b/runtime/mirror/dex_cache.h
@@ -191,11 +191,12 @@
return GetFieldPtr<const DexFile*>(OFFSET_OF_OBJECT_MEMBER(DexCache, dex_file_));
}
- void SetDexFile(const DexFile* dex_file) SHARED_REQUIRES(Locks::mutator_lock_)
- ALWAYS_INLINE {
- return SetFieldPtr<false>(OFFSET_OF_OBJECT_MEMBER(DexCache, dex_file_), dex_file);
+ void SetDexFile(const DexFile* dex_file) SHARED_REQUIRES(Locks::mutator_lock_) {
+ SetFieldPtr<false>(OFFSET_OF_OBJECT_MEMBER(DexCache, dex_file_), dex_file);
}
+ void SetLocation(mirror::String* location) SHARED_REQUIRES(Locks::mutator_lock_);
+
// NOTE: Get/SetElementPtrSize() are intended for working with ArtMethod** and ArtField**
// provided by GetResolvedMethods/Fields() and ArtMethod::GetDexCacheResolvedMethods(),
// so they need to be public.
