Revert "Revert "Load app images""

This reverts commit 1bc977cf2f8199311a97f2ba9431a184540e3e9c.

Bug: 22858531

Change-Id: Ide00bf3a73a02cba3bb364177204ad1b13f70295
diff --git a/compiler/driver/compiler_driver.cc b/compiler/driver/compiler_driver.cc
index d021525..818d50a 100644
--- a/compiler/driver/compiler_driver.cc
+++ b/compiler/driver/compiler_driver.cc
@@ -730,8 +730,8 @@
 
 bool CompilerDriver::IsImageClass(const char* descriptor) const {
   if (!IsBootImage()) {
-    // NOTE: Currently unreachable, all callers check IsImage().
-    return false;
+    // NOTE: Currently only reachable from InitImageMethodVisitor for the app image case.
+    return true;
   } else {
     return image_classes_->find(descriptor) != image_classes_->end();
   }
diff --git a/compiler/image_test.cc b/compiler/image_test.cc
index 12132c0..b841675 100644
--- a/compiler/image_test.cc
+++ b/compiler/image_test.cc
@@ -155,7 +155,11 @@
   {
     std::vector<const char*> dup_oat_filename(1, dup_oat->GetPath().c_str());
     std::vector<const char*> dup_image_filename(1, image_file.GetFilename().c_str());
-    bool success_image = writer->Write(kInvalidImageFd, dup_image_filename, dup_oat_filename);
+    bool success_image = writer->Write(kInvalidFd,
+                                       dup_image_filename,
+                                       kInvalidFd,
+                                       dup_oat_filename,
+                                       dup_oat_filename[0]);
     ASSERT_TRUE(success_image);
     bool success_fixup = ElfWriter::Fixup(dup_oat.get(),
                                           writer->GetOatDataBegin(dup_oat_filename[0]));
@@ -292,11 +296,17 @@
                              oat_data_begin,
                              oat_data_end,
                              oat_file_end,
+                             /*boot_image_begin*/0U,
+                             /*boot_image_size*/0U,
+                             /*boot_oat_begin*/0U,
+                             /*boot_oat_size_*/0U,
                              sizeof(void*),
                              /*compile_pic*/false,
+                             /*is_pic*/false,
                              ImageHeader::kDefaultStorageMode,
                              /*data_size*/0u);
     ASSERT_TRUE(image_header.IsValid());
+    ASSERT_TRUE(!image_header.IsAppImage());
 
     char* magic = const_cast<char*>(image_header.GetMagic());
     strcpy(magic, "");  // bad magic
diff --git a/compiler/image_writer.cc b/compiler/image_writer.cc
index d0bb201..72c615e 100644
--- a/compiler/image_writer.cc
+++ b/compiler/image_writer.cc
@@ -159,17 +159,45 @@
 
 bool ImageWriter::Write(int image_fd,
                         const std::vector<const char*>& image_filenames,
-                        const std::vector<const char*>& oat_filenames) {
+                        int oat_fd,
+                        const std::vector<const char*>& oat_filenames,
+                        const std::string& oat_location) {
+  // If image_fd or oat_fd are not kInvalidFd then we may have empty strings in image_filenames or
+  // oat_filenames.
   CHECK(!image_filenames.empty());
+  if (image_fd != kInvalidFd) {
+    CHECK_EQ(image_filenames.size(), 1u);
+  }
   CHECK(!oat_filenames.empty());
+  if (oat_fd != kInvalidFd) {
+    CHECK_EQ(oat_filenames.size(), 1u);
+  }
   CHECK_EQ(image_filenames.size(), oat_filenames.size());
 
   size_t oat_file_offset = 0;
 
   for (size_t i = 0; i < oat_filenames.size(); ++i) {
     const char* oat_filename = oat_filenames[i];
-    std::unique_ptr<File> oat_file(OS::OpenFileReadWrite(oat_filename));
-    if (oat_file.get() == nullptr) {
+    std::unique_ptr<File> oat_file;
+
+    if (oat_fd != -1) {
+      if (strlen(oat_filename) == 0u) {
+        oat_file.reset(new File(oat_fd, false));
+      } else {
+        oat_file.reset(new File(oat_fd, oat_filename, false));
+      }
+      int length = oat_file->GetLength();
+      if (length < 0) {
+        PLOG(ERROR) << "Oat file has negative length " << length;
+        return false;
+      } else {
+        // Leave the fd open since dex2oat still needs to write out the oat file with the fd.
+        oat_file->DisableAutoClose();
+      }
+    } else {
+      oat_file.reset(OS::OpenFileReadWrite(oat_filename));
+    }
+    if (oat_file == nullptr) {
       PLOG(ERROR) << "Failed to open oat file " << oat_filename;
       return false;
     }
@@ -181,7 +209,7 @@
       return false;
     }
     Runtime::Current()->GetOatFileManager().RegisterOatFile(
-      std::unique_ptr<const OatFile>(oat_file_));
+        std::unique_ptr<const OatFile>(oat_file_));
 
     const OatHeader& oat_header = oat_file_->GetOatHeader();
     ImageInfo& image_info = GetImageInfo(oat_filename);
@@ -220,8 +248,15 @@
 
     SetOatChecksumFromElfFile(oat_file.get());
 
-    if (oat_file->FlushCloseOrErase() != 0) {
-      LOG(ERROR) << "Failed to flush and close oat file " << oat_filename;
+    if (oat_fd != -1) {
+      // Leave fd open for caller.
+      if (oat_file->Flush() != 0) {
+        LOG(ERROR) << "Failed to flush oat file " << oat_filename << " for " << oat_location;
+        return false;
+      }
+    } else if (oat_file->FlushCloseOrErase() != 0) {
+      LOG(ERROR) << "Failed to flush and close oat file " << oat_filename
+                 << " for " << oat_location;
       return false;
     }
   }
@@ -238,16 +273,22 @@
     const char* oat_filename = oat_filenames[i];
     ImageInfo& image_info = GetImageInfo(oat_filename);
     std::unique_ptr<File> image_file;
-    if (image_fd != kInvalidImageFd) {
-      image_file.reset(new File(image_fd, image_filename, unix_file::kCheckSafeUsage));
+    if (image_fd != kInvalidFd) {
+      if (strlen(image_filename) == 0u) {
+        image_file.reset(new File(image_fd, unix_file::kCheckSafeUsage));
+      } else {
+        LOG(ERROR) << "image fd " << image_fd << " name " << image_filename;
+      }
     } else {
       image_file.reset(OS::CreateEmptyFile(image_filename));
     }
+
     if (image_file == nullptr) {
       LOG(ERROR) << "Failed to open image file " << image_filename;
       return false;
     }
-    if (fchmod(image_file->Fd(), 0644) != 0) {
+
+    if (!compile_app_image_ && fchmod(image_file->Fd(), 0644) != 0) {
       PLOG(ERROR) << "Failed to make image file world readable: " << image_filename;
       image_file->Erase();
       return EXIT_FAILURE;
@@ -701,6 +742,7 @@
     std::unordered_set<mirror::Class*>* visited) {
   DCHECK(early_exit != nullptr);
   DCHECK(visited != nullptr);
+  DCHECK(compile_app_image_);
   if (klass == nullptr) {
     return false;
   }
@@ -717,6 +759,13 @@
   visited->emplace(klass);
   bool result = IsBootClassLoaderNonImageClass(klass);
   bool my_early_exit = false;  // Only for ourselves, ignore caller.
+  // Remove classes that failed to verify since we don't want to have java.lang.VerifyError in the
+  // app image.
+  if (klass->GetStatus() == mirror::Class::kStatusError) {
+    result = true;
+  } else {
+    CHECK(klass->GetVerifyError() == nullptr) << PrettyClass(klass);
+  }
   if (!result) {
     // Check interfaces since these wont be visited through VisitReferences.)
     mirror::IfTable* if_table = klass->GetIfTable();
@@ -727,6 +776,12 @@
           visited);
     }
   }
+  if (klass->IsObjectArrayClass()) {
+    result = result || ContainsBootClassLoaderNonImageClassInternal(
+        klass->GetComponentType(),
+        &my_early_exit,
+        visited);
+  }
   // Check static fields and their classes.
   size_t num_static_fields = klass->NumReferenceStaticFields();
   if (num_static_fields != 0 && klass->IsResolved()) {
@@ -780,7 +835,9 @@
   if (compile_app_image_) {
     // For app images, we need to prune boot loader classes that are not in the boot image since
     // these may have already been loaded when the app image is loaded.
-    return !ContainsBootClassLoaderNonImageClass(klass);
+    // Keep classes in the boot image space since we don't want to re-resolve these.
+    return Runtime::Current()->GetHeap()->ObjectIsInBootImageSpace(klass) ||
+        !ContainsBootClassLoaderNonImageClass(klass);
   }
   std::string temp;
   return compiler_driver_.IsImageClass(klass->GetDescriptor(&temp));
@@ -843,25 +900,25 @@
     for (size_t i = 0, num = dex_cache->NumResolvedMethods(); i != num; ++i) {
       ArtMethod* method =
           mirror::DexCache::GetElementPtrSize(resolved_methods, i, target_ptr_size_);
-      if (method != nullptr) {
-        auto* declaring_class = method->GetDeclaringClass();
-        // Miranda methods may be held live by a class which was not an image class but have a
-        // declaring class which is an image class. Set it to the resolution method to be safe and
-        // prevent dangling pointers.
-        if (method->IsMiranda() || !KeepClass(declaring_class)) {
-          mirror::DexCache::SetElementPtrSize(resolved_methods,
-                                              i,
-                                              resolution_method,
-                                              target_ptr_size_);
-        } else {
-          // Check that the class is still in the classes table.
-          DCHECK(class_linker->ClassInClassTable(declaring_class)) << "Class "
-              << PrettyClass(declaring_class) << " not in class linker table";
-        }
+      DCHECK(method != nullptr) << "Expected resolution method instead of null method";
+      mirror::Class* declaring_class = method->GetDeclaringClass();
+      // Miranda methods may be held live by a class which was not an image class but have a
+      // declaring class which is an image class. Set it to the resolution method to be safe and
+      // prevent dangling pointers.
+      if (method->IsMiranda() || !KeepClass(declaring_class)) {
+        mirror::DexCache::SetElementPtrSize(resolved_methods,
+                                            i,
+                                            resolution_method,
+                                            target_ptr_size_);
+      } else {
+        // Check that the class is still in the classes table.
+        DCHECK(class_linker->ClassInClassTable(declaring_class)) << "Class "
+            << PrettyClass(declaring_class) << " not in class linker table";
       }
     }
+    ArtField** resolved_fields = dex_cache->GetResolvedFields();
     for (size_t i = 0; i < dex_cache->NumResolvedFields(); i++) {
-      ArtField* field = dex_cache->GetResolvedField(i, target_ptr_size_);
+      ArtField* field = mirror::DexCache::GetElementPtrSize(resolved_fields, i, target_ptr_size_);
       if (field != nullptr && !KeepClass(field->GetDeclaringClass())) {
         dex_cache->SetResolvedField(i, nullptr, target_ptr_size_);
       }
@@ -906,6 +963,32 @@
   }
 }
 
+mirror::String* ImageWriter::FindInternedString(mirror::String* string) {
+  Thread* const self = Thread::Current();
+  for (auto& pair : image_info_map_) {
+    const ImageInfo& image_info = pair.second;
+    mirror::String* const found = image_info.intern_table_->LookupStrong(self, string);
+    DCHECK(image_info.intern_table_->LookupWeak(self, string) == nullptr)
+        << string->ToModifiedUtf8();
+    if (found != nullptr) {
+      return found;
+    }
+  }
+  if (compile_app_image_) {
+    Runtime* const runtime = Runtime::Current();
+    mirror::String* found = runtime->GetInternTable()->LookupStrong(self, string);
+    // If we found it in the runtime intern table it could either be in the boot image or interned
+    // during app image compilation. If it was in the boot image return that, otherwise return null
+    // since it belongs to another image space.
+    if (found != nullptr && runtime->GetHeap()->ObjectIsInBootImageSpace(found)) {
+      return found;
+    }
+    DCHECK(runtime->GetInternTable()->LookupWeak(self, string) == nullptr)
+        << string->ToModifiedUtf8();
+  }
+  return nullptr;
+}
+
 void ImageWriter::CalculateObjectBinSlots(Object* obj) {
   DCHECK(obj != nullptr);
   // if it is a string, we want to intern it if its not interned.
@@ -915,13 +998,16 @@
 
     // we must be an interned string that was forward referenced and already assigned
     if (IsImageBinSlotAssigned(obj)) {
-      DCHECK_EQ(obj, image_info.intern_table_->InternStrongImageString(obj->AsString()));
+      DCHECK_EQ(obj, FindInternedString(obj->AsString()));
       return;
     }
-    // InternImageString allows us to intern while holding the heap bitmap lock. This is safe since
-    // we are guaranteed to not have GC during image writing.
-    mirror::String* const interned = image_info.intern_table_->InternStrongImageString(
-        obj->AsString());
+    // Need to check if the string is already interned in another image info so that we don't have
+    // the intern tables of two different images contain the same string.
+    mirror::String* interned = FindInternedString(obj->AsString());
+    if (interned == nullptr) {
+      // Not in another image space, insert to our table.
+      interned = image_info.intern_table_->InternStrongImageString(obj->AsString());
+    }
     if (obj != interned) {
       if (!IsImageBinSlotAssigned(interned)) {
         // interned obj is after us, allocate its location early
@@ -1066,6 +1152,11 @@
       // Visit and assign offsets for fields and field arrays.
       auto* as_klass = h_obj->AsClass();
       mirror::DexCache* dex_cache = as_klass->GetDexCache();
+      DCHECK_NE(klass->GetStatus(), mirror::Class::kStatusError);
+      if (compile_app_image_) {
+        // Extra sanity, no boot loader classes should be left!
+        CHECK(!IsBootClassLoaderClass(as_klass)) << PrettyClass(as_klass);
+      }
       LengthPrefixedArray<ArtField>* fields[] = {
           as_klass->GetSFieldsPtr(), as_klass->GetIFieldsPtr(),
       };
@@ -1405,6 +1496,13 @@
               << " Oat data end=" << reinterpret_cast<uintptr_t>(oat_data_end)
               << " Oat file end=" << reinterpret_cast<uintptr_t>(oat_file_end);
   }
+  // Store boot image info for app image so that we can relocate.
+  uint32_t boot_image_begin = 0;
+  uint32_t boot_image_end = 0;
+  uint32_t boot_oat_begin = 0;
+  uint32_t boot_oat_end = 0;
+  gc::Heap* const heap = Runtime::Current()->GetHeap();
+  heap->GetBootImagesSize(&boot_image_begin, &boot_image_end, &boot_oat_begin, &boot_oat_end);
 
   // Create the header, leave 0 for data size since we will fill this in as we are writing the
   // image.
@@ -1417,8 +1515,13 @@
                                                PointerToLowMemUInt32(image_info.oat_data_begin_),
                                                PointerToLowMemUInt32(oat_data_end),
                                                PointerToLowMemUInt32(oat_file_end),
+                                               boot_image_begin,
+                                               boot_image_end - boot_image_begin,
+                                               boot_oat_begin,
+                                               boot_oat_end - boot_oat_begin,
                                                target_ptr_size_,
                                                compile_pic_,
+                                               /*is_pic*/compile_app_image_,
                                                image_storage_mode_,
                                                /*data_size*/0u);
 }
@@ -1805,13 +1908,14 @@
       if (klass == class_linker->GetClassRoot(ClassLinker::kJavaLangDexCache)) {
         FixupDexCache(down_cast<mirror::DexCache*>(orig), down_cast<mirror::DexCache*>(copy));
       } else if (klass->IsClassLoaderClass()) {
+        mirror::ClassLoader* copy_loader = down_cast<mirror::ClassLoader*>(copy);
         // If src is a ClassLoader, set the class table to null so that it gets recreated by the
         // ClassLoader.
-        down_cast<mirror::ClassLoader*>(copy)->SetClassTable(nullptr);
+        copy_loader->SetClassTable(nullptr);
         // Also set allocator to null to be safe. The allocator is created when we create the class
         // table. We also never expect to unload things in the image since they are held live as
         // roots.
-        down_cast<mirror::ClassLoader*>(copy)->SetAllocator(nullptr);
+        copy_loader->SetAllocator(nullptr);
       }
     }
     FixupVisitor visitor(this, copy);
@@ -1896,7 +2000,7 @@
   // If we are compiling an app image, we need to use the stubs of the boot image.
   if (compile_app_image_) {
     // Use the current image pointers.
-    std::vector<gc::space::ImageSpace*> image_spaces =
+    const std::vector<gc::space::ImageSpace*>& image_spaces =
         Runtime::Current()->GetHeap()->GetBootImageSpaces();
     DCHECK(!image_spaces.empty());
     const OatFile* oat_file = image_spaces[0]->GetOatFile();
diff --git a/compiler/image_writer.h b/compiler/image_writer.h
index ad69038..622eb19 100644
--- a/compiler/image_writer.h
+++ b/compiler/image_writer.h
@@ -49,7 +49,7 @@
 
 class ClassTable;
 
-static constexpr int kInvalidImageFd = -1;
+static constexpr int kInvalidFd = -1;
 
 // Write a Space built during compilation for use during execution.
 class ImageWriter FINAL {
@@ -103,11 +103,15 @@
 
   uint8_t* GetOatFileBegin(const char* oat_filename) const;
 
-  // If image_fd is not kInvalidImageFd, then we use that for the file. Otherwise we open
+  // If image_fd is not kInvalidFd, then we use that for the image file. Otherwise we open
   // the names in image_filenames.
+  // If oat_fd is not kInvalidFd, then we use that for the oat file. Otherwise we open
+  // the names in oat_filenames.
   bool Write(int image_fd,
              const std::vector<const char*>& image_filenames,
-             const std::vector<const char*>& oat_filenames)
+             int oat_fd,
+             const std::vector<const char*>& oat_filenames,
+             const std::string& oat_location)
       REQUIRES(!Locks::mutator_lock_);
 
   uintptr_t GetOatDataBegin(const char* oat_filename) {
@@ -447,6 +451,10 @@
   const ImageInfo& GetConstImageInfo(const char* oat_filename) const;
   const ImageInfo& GetImageInfo(size_t index) const;
 
+  // Find an already strong interned string in the other images or in the boot image. Used to
+  // remove duplicates in the multi image and app image case.
+  mirror::String* FindInternedString(mirror::String* string) SHARED_REQUIRES(Locks::mutator_lock_);
+
   const CompilerDriver& compiler_driver_;
 
   // Beginning target image address for the first image.
diff --git a/compiler/oat_writer.cc b/compiler/oat_writer.cc
index c74c41f..a542162 100644
--- a/compiler/oat_writer.cc
+++ b/compiler/oat_writer.cc
@@ -725,8 +725,18 @@
 
       // Deduplicate code arrays if we are not producing debuggable code.
       bool deduped = false;
+      MethodReference method_ref(dex_file_, it.GetMemberIndex());
+      auto method_lb = writer_->method_offset_map_.map.lower_bound(method_ref);
       if (debuggable_) {
-        quick_code_offset = NewQuickCodeOffset(compiled_method, it, thumb_offset);
+        if (method_lb != writer_->method_offset_map_.map.end() &&
+            !writer_->method_offset_map_.map.key_comp()(method_ref, method_lb->first)) {
+          // Duplicate methods, we want the same code for both of them so that the oat writer puts
+          // the same code in both ArtMethods so that we do not get different oat code at runtime.
+          quick_code_offset = method_lb->second;
+          deduped = true;
+        } else {
+          quick_code_offset = NewQuickCodeOffset(compiled_method, it, thumb_offset);
+        }
       } else {
         auto lb = dedupe_map_.lower_bound(compiled_method);
         if (lb != dedupe_map_.end() && !dedupe_map_.key_comp()(compiled_method, lb->first)) {
@@ -739,14 +749,12 @@
       }
 
       if (code_size != 0) {
-        MethodReference method_ref(dex_file_, it.GetMemberIndex());
-        auto method_lb = writer_->method_offset_map_.map.lower_bound(method_ref);
         if (method_lb != writer_->method_offset_map_.map.end() &&
             !writer_->method_offset_map_.map.key_comp()(method_ref, method_lb->first)) {
           // TODO: Should this be a hard failure?
           LOG(WARNING) << "Multiple definitions of "
               << PrettyMethod(method_ref.dex_method_index, *method_ref.dex_file)
-              << ((method_lb->second != quick_code_offset) ? "; OFFSET MISMATCH" : "");
+              << " offsets " << method_lb->second << " " << quick_code_offset;
         } else {
           writer_->method_offset_map_.map.PutBefore(method_lb, method_ref, quick_code_offset);
         }
@@ -958,30 +966,40 @@
     }
 
     ClassLinker* linker = Runtime::Current()->GetClassLinker();
-    InvokeType invoke_type = it.GetMethodInvokeType(dex_file_->GetClassDef(class_def_index_));
     // Unchecked as we hold mutator_lock_ on entry.
     ScopedObjectAccessUnchecked soa(Thread::Current());
     StackHandleScope<1> hs(soa.Self());
     Handle<mirror::DexCache> dex_cache(hs.NewHandle(linker->FindDexCache(
         Thread::Current(), *dex_file_)));
-    ArtMethod* method = linker->ResolveMethod<ClassLinker::kNoICCECheckForCache>(
-        *dex_file_,
-        it.GetMemberIndex(),
-        dex_cache,
-        ScopedNullHandle<mirror::ClassLoader>(),
-        nullptr,
-        invoke_type);
-    if (method == nullptr) {
-      LOG(INTERNAL_FATAL) << "Unexpected failure to resolve a method: "
-                          << PrettyMethod(it.GetMemberIndex(), *dex_file_, true);
-      soa.Self()->AssertPendingException();
-      mirror::Throwable* exc = soa.Self()->GetException();
-      std::string dump = exc->Dump();
-      LOG(FATAL) << dump;
-      UNREACHABLE();
+    ArtMethod* method;
+    if (writer_->HasBootImage()) {
+      const InvokeType invoke_type = it.GetMethodInvokeType(
+          dex_file_->GetClassDef(class_def_index_));
+      method = linker->ResolveMethod<ClassLinker::kNoICCECheckForCache>(
+          *dex_file_,
+          it.GetMemberIndex(),
+          dex_cache,
+          ScopedNullHandle<mirror::ClassLoader>(),
+          nullptr,
+          invoke_type);
+      if (method == nullptr) {
+        LOG(INTERNAL_FATAL) << "Unexpected failure to resolve a method: "
+            << PrettyMethod(it.GetMemberIndex(), *dex_file_, true);
+        soa.Self()->AssertPendingException();
+        mirror::Throwable* exc = soa.Self()->GetException();
+        std::string dump = exc->Dump();
+        LOG(FATAL) << dump;
+        UNREACHABLE();
+      }
+    } else {
+      // Should already have been resolved by the compiler, just peek into the dex cache.
+      // It may not be resolved if the class failed to verify, in this case, don't set the
+      // entrypoint. This is not fatal since the dex cache will contain a resolution method.
+      method = dex_cache->GetResolvedMethod(it.GetMemberIndex(), linker->GetImagePointerSize());
     }
-
-    if (compiled_method != nullptr && compiled_method->GetQuickCode().size() != 0) {
+    if (method != nullptr &&
+        compiled_method != nullptr &&
+        compiled_method->GetQuickCode().size() != 0) {
       method->SetEntryPointFromQuickCompiledCodePtrSize(
           reinterpret_cast<void*>(offsets.code_offset_), pointer_size_);
     }
@@ -1467,7 +1485,7 @@
     } while (false)
 
   VISIT(InitCodeMethodVisitor);
-  if (compiler_driver_->IsBootImage()) {
+  if (HasImage()) {
     VISIT(InitImageMethodVisitor);
   }