Refactor loading boot image.

Load art files first, then load oat files.

Test: m test-art-host-gtest
Test: testrunner.py --host --optimizing
Change-Id: I05871a8ababaab5a59919acd7bcfd07c69cc563d

3 files changed, 124 insertions, 183 deletions

diff --git a/runtime/gc/collector/immune_spaces_test.cc b/runtime/gc/collector/immune_spaces_test.cc
index fad3f0bee2..9f98f6c7f0 100644
--- a/runtime/gc/collector/immune_spaces_test.cc
+++ b/runtime/gc/collector/immune_spaces_test.cc
@@ -113,7 +113,7 @@ class ImmuneSpacesTest : public CommonRuntimeTest {
     ImageSection sections[ImageHeader::kSectionCount];
     new (image_map.Begin()) ImageHeader(
         /*image_begin=*/ PointerToLowMemUInt32(image_map.Begin()),
-        /*image_size=*/ image_map.Size(),
+        /*image_size=*/ image_size,
         sections,
         /*image_roots=*/ PointerToLowMemUInt32(image_map.Begin()) + 1,
         /*oat_checksum=*/ 0u,
diff --git a/runtime/gc/space/image_space.cc b/runtime/gc/space/image_space.cc
index 51b5ace027..bfb37463e5 100644
--- a/runtime/gc/space/image_space.cc
+++ b/runtime/gc/space/image_space.cc
@@ -383,20 +383,16 @@ class ImageSpace::Loader {
  public:
   static std::unique_ptr<ImageSpace> InitAppImage(const char* image_filename,
                                                   const char* image_location,
-                                                  bool validate_oat_file,
                                                   const OatFile* oat_file,
                                                   /*inout*/MemMap* image_reservation,
-                                                  /*inout*/MemMap* oat_reservation,
                                                   /*out*/std::string* error_msg)
       REQUIRES_SHARED(Locks::mutator_lock_) {
     TimingLogger logger(__PRETTY_FUNCTION__, /*precise=*/ true, VLOG_IS_ON(image));
     std::unique_ptr<ImageSpace> space = Init(image_filename,
                                              image_location,
-                                             validate_oat_file,
                                              oat_file,
                                              &logger,
                                              image_reservation,
-                                             oat_reservation,
                                              error_msg);
     if (space != nullptr) {
       TimingLogger::ScopedTiming timing("RelocateImage", &logger);
@@ -438,11 +434,9 @@ class ImageSpace::Loader {
 
   static std::unique_ptr<ImageSpace> Init(const char* image_filename,
                                           const char* image_location,
-                                          bool validate_oat_file,
                                           const OatFile* oat_file,
                                           TimingLogger* logger,
                                           /*inout*/MemMap* image_reservation,
-                                          /*inout*/MemMap* oat_reservation,
                                           /*out*/std::string* error_msg)
       REQUIRES_SHARED(Locks::mutator_lock_) {
     CHECK(image_filename != nullptr);
@@ -479,8 +473,8 @@ class ImageSpace::Loader {
     }
 
     if (oat_file != nullptr) {
-      // If we have an oat file, check the oat file checksum. The oat file is only non-null for the
-      // app image case. Otherwise, we open the oat file after the image and check the checksum there.
+      // If we have an oat file (i.e. for app image), check the oat file checksum.
+      // Otherwise, we open the oat file after the image and check the checksum there.
       const uint32_t oat_checksum = oat_file->GetOatHeader().GetChecksum();
       const uint32_t image_oat_checksum = image_header->GetOatChecksum();
       if (oat_checksum != image_oat_checksum) {
@@ -517,15 +511,13 @@ class ImageSpace::Loader {
       return nullptr;
     }
 
-    MemMap map;
-
     // GetImageBegin is the preferred address to map the image. If we manage to map the
     // image at the image begin, the amount of fixup work required is minimized.
     // If it is pic we will retry with error_msg for the failure case. Pass a null error_msg to
     // avoid reading proc maps for a mapping failure and slowing everything down.
     // For the boot image, we have already reserved the memory and we load the image
     // into the `image_reservation`.
-    map = LoadImageFile(
+    MemMap map = LoadImageFile(
         image_filename,
         image_location,
         *image_header,
@@ -583,33 +575,7 @@ class ImageSpace::Loader {
                                                      std::move(map),
                                                      std::move(bitmap),
                                                      image_end));
-
-    // VerifyImageAllocations() will be called later in Runtime::Init()
-    // as some class roots like ArtMethod::java_lang_reflect_ArtMethod_
-    // and ArtField::java_lang_reflect_ArtField_, which are used from
-    // Object::SizeOf() which VerifyImageAllocations() calls, are not
-    // set yet at this point.
-    if (oat_file == nullptr) {
-      TimingLogger::ScopedTiming timing("OpenOatFile", logger);
-      space->oat_file_ = OpenOatFile(*space, image_filename, oat_reservation, error_msg);
-      if (space->oat_file_ == nullptr) {
-        DCHECK(!error_msg->empty());
-        return nullptr;
-      }
-      space->oat_file_non_owned_ = space->oat_file_.get();
-    } else {
-      space->oat_file_non_owned_ = oat_file;
-    }
-
-    if (validate_oat_file) {
-      TimingLogger::ScopedTiming timing("ValidateOatFile", logger);
-      CHECK(space->oat_file_ != nullptr);
-      if (!ImageSpace::ValidateOatFile(*space->oat_file_, error_msg)) {
-        DCHECK(!error_msg->empty());
-        return nullptr;
-      }
-    }
-
+    space->oat_file_non_owned_ = oat_file;
     return space;
   }
 
@@ -1230,49 +1196,6 @@ class ImageSpace::Loader {
     }
     return true;
   }
-
-  static std::unique_ptr<OatFile> OpenOatFile(const ImageSpace& image,
-                                              const char* image_path,
-                                              /*inout*/MemMap* oat_reservation,
-                                              std::string* error_msg) {
-    const ImageHeader& image_header = image.GetImageHeader();
-    std::string oat_filename = ImageHeader::GetOatLocationFromImageLocation(image_path);
-
-    CHECK(image_header.GetOatDataBegin() != nullptr);
-
-    uint8_t* oat_data_begin = image_header.GetOatDataBegin();
-    if (oat_reservation != nullptr) {
-      oat_data_begin += oat_reservation->Begin() - image_header.GetOatFileBegin();
-    }
-    std::unique_ptr<OatFile> oat_file(OatFile::Open(/*zip_fd=*/ -1,
-                                                    oat_filename,
-                                                    oat_filename,
-                                                    !Runtime::Current()->IsAotCompiler(),
-                                                    /*low_4gb=*/ false,
-                                                    /*abs_dex_location=*/ nullptr,
-                                                    oat_reservation,
-                                                    error_msg));
-    if (oat_file == nullptr) {
-      *error_msg = StringPrintf("Failed to open oat file '%s' referenced from image %s: %s",
-                                oat_filename.c_str(),
-                                image.GetName(),
-                                error_msg->c_str());
-      return nullptr;
-    }
-    CHECK(oat_data_begin == oat_file->Begin());
-    uint32_t oat_checksum = oat_file->GetOatHeader().GetChecksum();
-    uint32_t image_oat_checksum = image_header.GetOatChecksum();
-    if (oat_checksum != image_oat_checksum) {
-      *error_msg = StringPrintf("Failed to match oat file checksum 0x%x to expected oat checksum 0x%x"
-                                " in image %s",
-                                oat_checksum,
-                                image_oat_checksum,
-                                image.GetName());
-      return nullptr;
-    }
-
-    return oat_file;
-  }
 };
 
 class ImageSpace::BootImageLoader {
@@ -1332,26 +1255,22 @@ class ImageSpace::BootImageLoader {
     }
     uint32_t image_start;
     uint32_t image_end;
-    uint32_t oat_end;
-    if (!GetBootImageAddressRange(filename, &image_start, &image_end, &oat_end, error_msg)) {
+    if (!GetBootImageAddressRange(filename, &image_start, &image_end, error_msg)) {
       return false;
     }
     if (locations.size() > 1u) {
       std::string last_filename = GetSystemImageFilename(locations.back().c_str(), image_isa_);
       uint32_t dummy;
-      if (!GetBootImageAddressRange(last_filename, &dummy, &image_end, &oat_end, error_msg)) {
+      if (!GetBootImageAddressRange(last_filename, &dummy, &image_end, error_msg)) {
         return false;
       }
     }
     MemMap image_reservation;
-    MemMap oat_reservation;
     MemMap local_extra_reservation;
-    if (!ReserveBootImageMemory(image_start,
-                                image_end,
-                                oat_end,
+    if (!ReserveBootImageMemory(/*reservation_size=*/ image_end - image_start,
+                                image_start,
                                 extra_reservation_size,
                                 &image_reservation,
-                                &oat_reservation,
                                 &local_extra_reservation,
                                 error_msg)) {
       return false;
@@ -1361,28 +1280,29 @@ class ImageSpace::BootImageLoader {
     spaces.reserve(locations.size());
     for (const std::string& location : locations) {
       filename = GetSystemImageFilename(location.c_str(), image_isa_);
-      spaces.push_back(Load(location,
-                            filename,
-                            /*validate_oat_file=*/ false,
-                            &logger,
-                            &image_reservation,
-                            &oat_reservation,
-                            error_msg));
+      spaces.push_back(Load(location, filename, &logger, &image_reservation, error_msg));
       if (spaces.back() == nullptr) {
         return false;
       }
     }
-    if (!CheckReservationsExhausted(image_reservation, oat_reservation, error_msg)) {
+    for (std::unique_ptr<ImageSpace>& space : spaces) {
+      static constexpr bool kValidateOatFile = false;
+      if (!OpenOatFile(space.get(), kValidateOatFile, &logger, &image_reservation, error_msg)) {
+        return false;
+      }
+    }
+    if (!CheckReservationExhausted(image_reservation, error_msg)) {
       return false;
     }
 
     MaybeRelocateSpaces(spaces, &logger);
     InitRuntimeMethods(spaces);
-    *extra_reservation = std::move(local_extra_reservation);
-    VLOG(image) << "ImageSpace::BootImageLoader::InitFromDalvikCache exiting " << *spaces.front();
     boot_image_spaces->swap(spaces);
+    *extra_reservation = std::move(local_extra_reservation);
 
     if (VLOG_IS_ON(image)) {
+      LOG(INFO) << "ImageSpace::BootImageLoader::LoadFromSystem exiting "
+          << boot_image_spaces->front();
       logger.Dump(LOG_STREAM(INFO));
     }
     return true;
@@ -1402,8 +1322,7 @@ class ImageSpace::BootImageLoader {
     }
     uint32_t image_start;
     uint32_t image_end;
-    uint32_t oat_end;
-    if (!GetBootImageAddressRange(cache_filename_, &image_start, &image_end, &oat_end, error_msg)) {
+    if (!GetBootImageAddressRange(cache_filename_, &image_start, &image_end, error_msg)) {
       return false;
     }
     if (locations.size() > 1u) {
@@ -1415,19 +1334,16 @@ class ImageSpace::BootImageLoader {
         return false;
       }
       uint32_t dummy;
-      if (!GetBootImageAddressRange(last_filename, &dummy, &image_end, &oat_end, error_msg)) {
+      if (!GetBootImageAddressRange(last_filename, &dummy, &image_end, error_msg)) {
         return false;
       }
     }
     MemMap image_reservation;
-    MemMap oat_reservation;
     MemMap local_extra_reservation;
-    if (!ReserveBootImageMemory(image_start,
-                                image_end,
-                                oat_end,
+    if (!ReserveBootImageMemory(/*reservation_size=*/ image_end - image_start,
+                                image_start,
                                 extra_reservation_size,
                                 &image_reservation,
-                                &oat_reservation,
                                 &local_extra_reservation,
                                 error_msg)) {
       return false;
@@ -1440,28 +1356,28 @@ class ImageSpace::BootImageLoader {
       if (!GetDalvikCacheFilename(location.c_str(), dalvik_cache_.c_str(), &filename, error_msg)) {
         return false;
       }
-      spaces.push_back(Load(location,
-                            filename,
-                            validate_oat_file,
-                            &logger,
-                            &image_reservation,
-                            &oat_reservation,
-                            error_msg));
+      spaces.push_back(Load(location, filename, &logger, &image_reservation, error_msg));
       if (spaces.back() == nullptr) {
         return false;
       }
     }
-    if (!CheckReservationsExhausted(image_reservation, oat_reservation, error_msg)) {
+    for (std::unique_ptr<ImageSpace>& space : spaces) {
+      if (!OpenOatFile(space.get(), validate_oat_file, &logger, &image_reservation, error_msg)) {
+        return false;
+      }
+    }
+    if (!CheckReservationExhausted(image_reservation, error_msg)) {
       return false;
     }
 
     MaybeRelocateSpaces(spaces, &logger);
     InitRuntimeMethods(spaces);
-    *extra_reservation = std::move(local_extra_reservation);
     boot_image_spaces->swap(spaces);
+    *extra_reservation = std::move(local_extra_reservation);
 
-    VLOG(image) << "ImageSpace::BootImageLoader::InitFromDalvikCache exiting " << *spaces.front();
     if (VLOG_IS_ON(image)) {
+      LOG(INFO) << "ImageSpace::BootImageLoader::LoadFromDalvikCache exiting "
+          << boot_image_spaces->front();
       logger.Dump(LOG_STREAM(INFO));
     }
     return true;
@@ -1994,10 +1910,8 @@ class ImageSpace::BootImageLoader {
 
   std::unique_ptr<ImageSpace> Load(const std::string& image_location,
                                    const std::string& image_filename,
-                                   bool validate_oat_file,
                                    TimingLogger* logger,
                                    /*inout*/MemMap* image_reservation,
-                                   /*inout*/MemMap* oat_reservation,
                                    /*out*/std::string* error_msg)
       REQUIRES_SHARED(Locks::mutator_lock_) {
     // Should this be a RDWR lock? This is only a defensive measure, as at
@@ -2026,14 +1940,80 @@ class ImageSpace::BootImageLoader {
     // file name.
     return Loader::Init(image_filename.c_str(),
                         image_location.c_str(),
-                        validate_oat_file,
                         /*oat_file=*/ nullptr,
                         logger,
                         image_reservation,
-                        oat_reservation,
                         error_msg);
   }
 
+  bool OpenOatFile(ImageSpace* space,
+                   bool validate_oat_file,
+                   TimingLogger* logger,
+                   /*inout*/MemMap* image_reservation,
+                   /*out*/std::string* error_msg) {
+    // VerifyImageAllocations() will be called later in Runtime::Init()
+    // as some class roots like ArtMethod::java_lang_reflect_ArtMethod_
+    // and ArtField::java_lang_reflect_ArtField_, which are used from
+    // Object::SizeOf() which VerifyImageAllocations() calls, are not
+    // set yet at this point.
+    DCHECK(image_reservation != nullptr);
+    std::unique_ptr<OatFile> oat_file;
+    {
+      TimingLogger::ScopedTiming timing("OpenOatFile", logger);
+      std::string oat_filename =
+          ImageHeader::GetOatLocationFromImageLocation(space->GetImageFilename());
+
+      oat_file.reset(OatFile::Open(/*zip_fd=*/ -1,
+                                   oat_filename,
+                                   oat_filename,
+                                   !Runtime::Current()->IsAotCompiler(),
+                                   /*low_4gb=*/ false,
+                                   /*abs_dex_location=*/ nullptr,
+                                   image_reservation,
+                                   error_msg));
+      if (oat_file == nullptr) {
+        *error_msg = StringPrintf("Failed to open oat file '%s' referenced from image %s: %s",
+                                  oat_filename.c_str(),
+                                  space->GetName(),
+                                  error_msg->c_str());
+        return false;
+      }
+      const ImageHeader& image_header = space->GetImageHeader();
+      uint32_t oat_checksum = oat_file->GetOatHeader().GetChecksum();
+      uint32_t image_oat_checksum = image_header.GetOatChecksum();
+      if (oat_checksum != image_oat_checksum) {
+        *error_msg = StringPrintf("Failed to match oat file checksum 0x%x to expected oat checksum"
+                                  " 0x%x in image %s",
+                                  oat_checksum,
+                                  image_oat_checksum,
+                                  space->GetName());
+        return false;
+      }
+      ptrdiff_t relocation_diff = space->Begin() - image_header.GetImageBegin();
+      CHECK(image_header.GetOatDataBegin() != nullptr);
+      uint8_t* oat_data_begin = image_header.GetOatDataBegin() + relocation_diff;
+      if (oat_file->Begin() != oat_data_begin) {
+        *error_msg = StringPrintf("Oat file '%s' referenced from image %s has unexpected begin"
+                                      " %p v. %p",
+                                  oat_filename.c_str(),
+                                  space->GetName(),
+                                  oat_file->Begin(),
+                                  oat_data_begin);
+        return false;
+      }
+    }
+    if (validate_oat_file) {
+      TimingLogger::ScopedTiming timing("ValidateOatFile", logger);
+      if (!ImageSpace::ValidateOatFile(*oat_file, error_msg)) {
+        DCHECK(!error_msg->empty());
+        return false;
+      }
+    }
+    space->oat_file_ = std::move(oat_file);
+    space->oat_file_non_owned_ = space->oat_file_.get();
+    return true;
+  }
+
   // Extract boot class path from oat file associated with `image_filename`
   // and list all associated image locations.
   static bool GetBootClassPathImageLocations(const std::string& image_location,
@@ -2068,7 +2048,6 @@ class ImageSpace::BootImageLoader {
   bool GetBootImageAddressRange(const std::string& filename,
                                 /*out*/uint32_t* start,
                                 /*out*/uint32_t* end,
-                                /*out*/uint32_t* oat_end,
                                 /*out*/std::string* error_msg) {
     ImageHeader system_hdr;
     if (!ReadSpecificImageHeader(filename.c_str(), &system_hdr)) {
@@ -2077,22 +2056,19 @@ class ImageSpace::BootImageLoader {
     }
     *start = reinterpret_cast32<uint32_t>(system_hdr.GetImageBegin());
     CHECK_ALIGNED(*start, kPageSize);
-    *end = RoundUp(*start + system_hdr.GetImageSize(), kPageSize);
-    *oat_end = RoundUp(reinterpret_cast32<uint32_t>(system_hdr.GetOatFileEnd()), kPageSize);
+    *end = RoundUp(reinterpret_cast32<uint32_t>(system_hdr.GetOatFileEnd()), kPageSize);
     return true;
   }
 
-  bool ReserveBootImageMemory(uint32_t image_start,
-                              uint32_t image_end,
-                              uint32_t oat_end,
+  bool ReserveBootImageMemory(uint32_t reservation_size,
+                              uint32_t image_start,
                               size_t extra_reservation_size,
                               /*out*/MemMap* image_reservation,
-                              /*out*/MemMap* oat_reservation,
                               /*out*/MemMap* extra_reservation,
                               /*out*/std::string* error_msg) {
     DCHECK(!image_reservation->IsValid());
-    size_t total_size =
-        dchecked_integral_cast<size_t>(oat_end - image_start) + extra_reservation_size;
+    DCHECK_LT(extra_reservation_size, std::numeric_limits<uint32_t>::max() - reservation_size);
+    size_t total_size = reservation_size + extra_reservation_size;
     bool relocate = Runtime::Current()->ShouldRelocate();
     // If relocating, choose a random address for ALSR.
     uint32_t addr = relocate ? ART_BASE_ADDRESS + ChooseRelocationOffsetDelta() : image_start;
@@ -2121,37 +2097,17 @@ class ImageSpace::BootImageLoader {
         return false;
       }
     }
-    uint32_t diff = reinterpret_cast32<uint32_t>(image_reservation->Begin()) - image_start;
-    image_start += diff;
-    image_end += diff;
-    oat_end += diff;
-    DCHECK(!oat_reservation->IsValid());
-    *oat_reservation = image_reservation->RemapAtEnd(reinterpret_cast32<uint8_t*>(image_end),
-                                                     "Boot image oat reservation",
-                                                     PROT_NONE,
-                                                     error_msg);
-    if (!oat_reservation->IsValid()) {
-      return false;
-    }
 
     return true;
   }
 
-  bool CheckReservationsExhausted(const MemMap& image_reservation,
-                                  const MemMap& oat_reservation,
-                                  /*out*/std::string* error_msg) {
+  bool CheckReservationExhausted(const MemMap& image_reservation, /*out*/std::string* error_msg) {
     if (image_reservation.IsValid()) {
       *error_msg = StringPrintf("Excessive image reservation after loading boot image: %p-%p",
                                 image_reservation.Begin(),
                                 image_reservation.End());
       return false;
     }
-    if (oat_reservation.IsValid()) {
-      *error_msg = StringPrintf("Excessive oat reservation after loading boot image: %p-%p",
-                                image_reservation.Begin(),
-                                image_reservation.End());
-      return false;
-    }
     return true;
   }
 
@@ -2355,12 +2311,11 @@ ImageSpace::~ImageSpace() {
 std::unique_ptr<ImageSpace> ImageSpace::CreateFromAppImage(const char* image,
                                                            const OatFile* oat_file,
                                                            std::string* error_msg) {
+  // Note: The oat file has already been validated.
   return Loader::InitAppImage(image,
                               image,
-                              /*validate_oat_file=*/ false,
                               oat_file,
                               /*image_reservation=*/ nullptr,
-                              /*oat_reservation=*/ nullptr,
                               error_msg);
 }
 
diff --git a/runtime/image.h b/runtime/image.h
index 87050ecb52..f33b9b2a2e 100644
--- a/runtime/image.h
+++ b/runtime/image.h
@@ -93,21 +93,7 @@ class PACKED(4) ImageHeader {
   };
   static constexpr StorageMode kDefaultStorageMode = kStorageModeUncompressed;
 
-  ImageHeader()
-      : image_begin_(0U),
-        image_size_(0U),
-        image_checksum_(0u),
-        oat_checksum_(0U),
-        oat_file_begin_(0U),
-        oat_data_begin_(0U),
-        oat_data_end_(0U),
-        oat_file_end_(0U),
-        boot_image_begin_(0U),
-        boot_image_size_(0U),
-        image_roots_(0U),
-        pointer_size_(0U),
-        storage_mode_(kDefaultStorageMode),
-        data_size_(0) {}
+  ImageHeader() {}
 
   ImageHeader(uint32_t image_begin,
               uint32_t image_size,
@@ -378,39 +364,39 @@ class PACKED(4) ImageHeader {
   uint8_t version_[4];
 
   // Required base address for mapping the image.
-  uint32_t image_begin_;
+  uint32_t image_begin_ = 0u;
 
   // Image size, not page aligned.
-  uint32_t image_size_;
+  uint32_t image_size_ = 0u;
 
   // Image file checksum (calculated with the checksum field set to 0).
-  uint32_t image_checksum_;
+  uint32_t image_checksum_ = 0u;
 
   // Checksum of the oat file we link to for load time sanity check.
-  uint32_t oat_checksum_;
+  uint32_t oat_checksum_ = 0u;
 
   // Start address for oat file. Will be before oat_data_begin_ for .so files.
-  uint32_t oat_file_begin_;
+  uint32_t oat_file_begin_ = 0u;
 
   // Required oat address expected by image Method::GetCode() pointers.
-  uint32_t oat_data_begin_;
+  uint32_t oat_data_begin_ = 0u;
 
   // End of oat data address range for this image file.
-  uint32_t oat_data_end_;
+  uint32_t oat_data_end_ = 0u;
 
   // End of oat file address range. will be after oat_data_end_ for
   // .so files. Used for positioning a following alloc spaces.
-  uint32_t oat_file_end_;
+  uint32_t oat_file_end_ = 0u;
 
   // Boot image begin and end (app image headers only).
-  uint32_t boot_image_begin_;
-  uint32_t boot_image_size_;  // Includes heap (*.art) and code (.oat).
+  uint32_t boot_image_begin_ = 0u;
+  uint32_t boot_image_size_ = 0u;  // Includes heap (*.art) and code (.oat).
 
   // Absolute address of an Object[] of objects needed to reinitialize from an image.
-  uint32_t image_roots_;
+  uint32_t image_roots_ = 0u;
 
   // Pointer size, this affects the size of the ArtMethods.
-  uint32_t pointer_size_;
+  uint32_t pointer_size_ = 0u;
 
   // Image section sizes/offsets correspond to the uncompressed form.
   ImageSection sections_[kSectionCount];
@@ -419,11 +405,11 @@ class PACKED(4) ImageHeader {
   uint64_t image_methods_[kImageMethodsCount];
 
   // Storage method for the image, the image may be compressed.
-  StorageMode storage_mode_;
+  StorageMode storage_mode_ = kDefaultStorageMode;
 
   // Data size for the image data excluding the bitmap and the header. For compressed images, this
   // is the compressed size in the file.
-  uint32_t data_size_;
+  uint32_t data_size_ = 0u;
 
   friend class linker::ImageWriter;
 };