Fix multi-image TODOs in class linker, runtime, and oat file.

- Modified SanityCheckArtMethodPointerArray in class linker
- Put back warnings in OatFile::GetOatDexFile
- Reinstated ImageSpace VerifyImageAllocations in Runtime::Init

Change-Id: I3bdb8f87d885213795c82c41e5095fec6daf00c4

diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 0518911..d995f28 100644 (file)
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -736,7 +736,7 @@ void ClassLinker::RunRootClinits() {
 
 static void SanityCheckArtMethod(ArtMethod* m,
                                  mirror::Class* expected_class,
-                                 std::vector<gc::space::ImageSpace*> spaces)
+                                 std::vector<gc::space::ImageSpace*>& spaces)
     SHARED_REQUIRES(Locks::mutator_lock_) {
   if (m->IsRuntimeMethod()) {
     CHECK(m->GetDeclaringClass() == nullptr) << PrettyMethod(m);
@@ -760,7 +760,7 @@ static void SanityCheckArtMethod(ArtMethod* m,
 static void SanityCheckArtMethodPointerArray(mirror::PointerArray* arr,
                                              mirror::Class* expected_class,
                                              size_t pointer_size,
-                                             std::vector<gc::space::ImageSpace*> spaces)
+                                             std::vector<gc::space::ImageSpace*>& spaces)
     SHARED_REQUIRES(Locks::mutator_lock_) {
   CHECK(arr != nullptr);
   for (int32_t j = 0; j < arr->GetLength(); ++j) {
@@ -775,27 +775,32 @@ static void SanityCheckArtMethodPointerArray(mirror::PointerArray* arr,
   }
 }
 
-/* TODO: Modify check to support multiple image spaces and reenable. b/26317072
-static void SanityCheckArtMethodPointerArray(
-    ArtMethod** arr,
-    size_t size,
-    size_t pointer_size,
-    gc::space::ImageSpace* space) SHARED_REQUIRES(Locks::mutator_lock_) {
+static void SanityCheckArtMethodPointerArray(ArtMethod** arr,
+                                             size_t size,
+                                             size_t pointer_size,
+                                             std::vector<gc::space::ImageSpace*>& spaces)
+    SHARED_REQUIRES(Locks::mutator_lock_) {
   CHECK_EQ(arr != nullptr, size != 0u);
   if (arr != nullptr) {
-    auto offset = reinterpret_cast<uint8_t*>(arr) - space->Begin();
-    CHECK(space->GetImageHeader().GetImageSection(
-        ImageHeader::kSectionDexCacheArrays).Contains(offset));
+    bool contains = false;
+    for (auto space : spaces) {
+      auto offset = reinterpret_cast<uint8_t*>(arr) - space->Begin();
+      if (space->GetImageHeader().GetImageSection(
+          ImageHeader::kSectionDexCacheArrays).Contains(offset)) {
+        contains = true;
+        break;
+      }
+    }
+    CHECK(contains);
   }
   for (size_t j = 0; j < size; ++j) {
     ArtMethod* method = mirror::DexCache::GetElementPtrSize(arr, j, pointer_size);
     // expected_class == null means we are a dex cache.
     if (method != nullptr) {
-      SanityCheckArtMethod(method, nullptr, space);
+      SanityCheckArtMethod(method, nullptr, spaces);
     }
   }
 }
-*/
 
 static void SanityCheckObjectsCallback(mirror::Object* obj, void* arg ATTRIBUTE_UNUSED)
     SHARED_REQUIRES(Locks::mutator_lock_) {
@@ -1018,13 +1023,12 @@ bool ClassLinker::InitFromImage(std::string* error_msg) {
         return false;
       }
 
-      // TODO: Modify check to support multiple image spaces and reenable.
-//      if (kSanityCheckObjects) {
-//        SanityCheckArtMethodPointerArray(dex_cache->GetResolvedMethods(),
-//                                         dex_cache->NumResolvedMethods(),
-//                                         image_pointer_size_,
-//                                         spaces);
-//      }
+      if (kSanityCheckObjects) {
+        SanityCheckArtMethodPointerArray(dex_cache->GetResolvedMethods(),
+                                         dex_cache->NumResolvedMethods(),
+                                         image_pointer_size_,
+                                         spaces);
+      }
 
       if (dex_file->GetLocationChecksum() != oat_dex_file->GetDexFileLocationChecksum()) {
         *error_msg = StringPrintf("Checksums do not match for %s: %x vs %x",

diff --git a/runtime/oat_file.cc b/runtime/oat_file.cc
index e3de14b..83e594b 100644 (file)
--- a/runtime/oat_file.cc
+++ b/runtime/oat_file.cc
@@ -983,7 +983,6 @@ const OatFile::OatDexFile* OatFile::GetOatDexFile(const char* dex_location,
     LOG(WARNING) << "Failed to find OatDexFile for DexFile " << dex_location
                  << " ( canonical path " << dex_canonical_location << ")"
                  << " with checksum " << checksum << " in OatFile " << GetLocation();
-    /* TODO: Modify for multi-image support and reenable. b/26317072
     if (kIsDebugBuild) {
       for (const OatDexFile* odf : oat_dex_files_storage_) {
         LOG(WARNING) << "OatFile " << GetLocation()
@@ -992,7 +991,6 @@ const OatFile::OatDexFile* OatFile::GetOatDexFile(const char* dex_location,
                      << " with checksum 0x" << std::hex << odf->GetDexFileLocationChecksum();
       }
     }
-    */
   }
 
   return nullptr;

diff --git a/runtime/runtime.cc b/runtime/runtime.cc
index 5c72629..dfb2877 100644 (file)
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -1089,13 +1089,11 @@ bool Runtime::Init(RuntimeArgumentMap&& runtime_options_in) {
       LOG(ERROR) << "Could not initialize from image: " << error_msg;
       return false;
     }
-    /* TODO: Modify check to support multiple image spaces and reenable. b/26317072
     if (kIsDebugBuild) {
       for (auto image_space : GetHeap()->GetBootImageSpaces()) {
         image_space->VerifyImageAllocations();
       }
     }
-    */
     if (boot_class_path_string_.empty()) {
       // The bootclasspath is not explicitly specified: construct it from the loaded dex files.
       const std::vector<const DexFile*>& boot_class_path = GetClassLinker()->GetBootClassPath();