Separate MemMap from DexFile completely

Create a container class for holding ownership of MemMap and segregate
that functionality to dex_file_loader.  This removes the dependency
between dex_file.cc and mem_map.cc.

Bug: 22322814
Test: make test-art-host
Change-Id: I96db6fd10cdbad774c2f1f85c249418a154fbd52

8 files changed, 102 insertions, 54 deletions

diff --git a/runtime/cdex/compact_dex_file.h b/runtime/cdex/compact_dex_file.h
index 910473be3f..3c1b638f03 100644
--- a/runtime/cdex/compact_dex_file.h
+++ b/runtime/cdex/compact_dex_file.h
@@ -51,8 +51,9 @@ class CompactDexFile : public DexFile {
                  size_t size,
                  const std::string& location,
                  uint32_t location_checksum,
-                 const OatDexFile* oat_dex_file)
-      : DexFile(base, size, location, location_checksum, oat_dex_file) {}
+                 const OatDexFile* oat_dex_file,
+                 DexFileContainer* container)
+      : DexFile(base, size, location, location_checksum, oat_dex_file, container) {}
 
   friend class DexFile;
   friend class DexFileLoader;
diff --git a/runtime/class_linker_test.cc b/runtime/class_linker_test.cc
index bd736929d8..3e92317682 100644
--- a/runtime/class_linker_test.cc
+++ b/runtime/class_linker_test.cc
@@ -1466,6 +1466,7 @@ TEST_F(ClassLinkerTest, RegisterDexFileName) {
                                                         old_dex_file->Size(),
                                                         location->ToModifiedUtf8(),
                                                         0u,
+                                                        nullptr,
                                                         nullptr));
   {
     WriterMutexLock mu(soa.Self(), *Locks::dex_lock_);
diff --git a/runtime/dex_file.cc b/runtime/dex_file.cc
index f2c43f7f87..974c7acbb2 100644
--- a/runtime/dex_file.cc
+++ b/runtime/dex_file.cc
@@ -16,13 +16,10 @@
 
 #include "dex_file.h"
 
-#include <fcntl.h>
 #include <limits.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
-#include <sys/file.h>
-#include <sys/mman.h>  // For the PROT_* and MAP_* constants.
 #include <zlib.h>
 
 #include <memory>
@@ -35,11 +32,7 @@
 #include "base/logging.h"
 #include "base/stl_util.h"
 #include "dex_file-inl.h"
-#include "dex_file_loader.h"
-#include "jvalue.h"
 #include "leb128.h"
-#include "mem_map.h"
-#include "os.h"
 #include "standard_dex_file.h"
 #include "utf-inl.h"
 #include "utils.h"
@@ -59,46 +52,32 @@ uint32_t DexFile::CalculateChecksum() const {
   return adler32(adler32(0L, Z_NULL, 0), non_sum_ptr, Size() - non_sum);
 }
 
-struct DexFile::AnnotationValue {
-  JValue value_;
-  uint8_t type_;
-};
-
 int DexFile::GetPermissions() const {
-  if (mem_map_.get() == nullptr) {
-    return 0;
-  } else {
-    return mem_map_->GetProtect();
-  }
+  CHECK(container_.get() != nullptr);
+  return container_->GetPermissions();
 }
 
 bool DexFile::IsReadOnly() const {
-  return GetPermissions() == PROT_READ;
+  CHECK(container_.get() != nullptr);
+  return container_->IsReadOnly();
 }
 
 bool DexFile::EnableWrite() const {
-  CHECK(IsReadOnly());
-  if (mem_map_.get() == nullptr) {
-    return false;
-  } else {
-    return mem_map_->Protect(PROT_READ | PROT_WRITE);
-  }
+  CHECK(container_.get() != nullptr);
+  return container_->EnableWrite();
 }
 
 bool DexFile::DisableWrite() const {
-  CHECK(!IsReadOnly());
-  if (mem_map_.get() == nullptr) {
-    return false;
-  } else {
-    return mem_map_->Protect(PROT_READ);
-  }
+  CHECK(container_.get() != nullptr);
+  return container_->DisableWrite();
 }
 
 DexFile::DexFile(const uint8_t* base,
                  size_t size,
                  const std::string& location,
                  uint32_t location_checksum,
-                 const OatDexFile* oat_dex_file)
+                 const OatDexFile* oat_dex_file,
+                 DexFileContainer* container)
     : begin_(base),
       size_(size),
       location_(location),
@@ -114,7 +93,8 @@ DexFile::DexFile(const uint8_t* base,
       num_method_handles_(0),
       call_site_ids_(nullptr),
       num_call_site_ids_(0),
-      oat_dex_file_(oat_dex_file) {
+      oat_dex_file_(oat_dex_file),
+      container_(container) {
   CHECK(begin_ != nullptr) << GetLocation();
   CHECK_GT(size_, 0U) << GetLocation();
   // Check base (=header) alignment.
diff --git a/runtime/dex_file.h b/runtime/dex_file.h
index 53e1f9ed96..c895e0d1da 100644
--- a/runtime/dex_file.h
+++ b/runtime/dex_file.h
@@ -39,6 +39,21 @@ class Signature;
 class StringPiece;
 class ZipArchive;
 
+// Some instances of DexFile own the storage referred to by DexFile.  Clients who create
+// such management do so by subclassing Container.
+class DexFileContainer {
+ public:
+  DexFileContainer() { }
+  virtual ~DexFileContainer() { }
+  virtual int GetPermissions() = 0;
+  virtual bool IsReadOnly() = 0;
+  virtual bool EnableWrite() = 0;
+  virtual bool DisableWrite() = 0;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(DexFileContainer);
+};
+
 // Dex file is the API that exposes native dex files (ordinary dex files) and CompactDex.
 // Originally, the dex file format used by ART was mostly the same as APKs. The only change was
 // quickened opcodes and layout optimizations.
@@ -993,7 +1008,8 @@ class DexFile {
           size_t size,
           const std::string& location,
           uint32_t location_checksum,
-          const OatDexFile* oat_dex_file);
+          const OatDexFile* oat_dex_file,
+          DexFileContainer* container);
 
   // Top-level initializer that calls other Init methods.
   bool Init(std::string* error_msg);
@@ -1018,9 +1034,6 @@ class DexFile {
 
   const uint32_t location_checksum_;
 
-  // Manages the underlying memory allocation.
-  std::unique_ptr<MemMap> mem_map_;
-
   // Points to the header section.
   const Header* const header_;
 
@@ -1059,6 +1072,9 @@ class DexFile {
   // null.
   mutable const OatDexFile* oat_dex_file_;
 
+  // Manages the underlying memory allocation.
+  std::unique_ptr<DexFileContainer> container_;
+
   friend class DexFileLoader;
   friend class DexFileVerifierTest;
   friend class OatWriter;
diff --git a/runtime/dex_file_loader.cc b/runtime/dex_file_loader.cc
index 26f7e7628c..06e3397754 100644
--- a/runtime/dex_file_loader.cc
+++ b/runtime/dex_file_loader.cc
@@ -33,6 +33,50 @@
 
 namespace art {
 
+namespace {
+
+class MemMapContainer : public DexFileContainer {
+ public:
+  explicit MemMapContainer(std::unique_ptr<MemMap>&& mem_map) : mem_map_(std::move(mem_map)) { }
+  virtual ~MemMapContainer() OVERRIDE { }
+
+  int GetPermissions() OVERRIDE {
+    if (mem_map_.get() == nullptr) {
+      return 0;
+    } else {
+      return mem_map_->GetProtect();
+    }
+  }
+
+  bool IsReadOnly() OVERRIDE {
+    return GetPermissions() == PROT_READ;
+  }
+
+  bool EnableWrite() OVERRIDE {
+    CHECK(IsReadOnly());
+    if (mem_map_.get() == nullptr) {
+      return false;
+    } else {
+      return mem_map_->Protect(PROT_READ | PROT_WRITE);
+    }
+  }
+
+  bool DisableWrite() OVERRIDE {
+    CHECK(!IsReadOnly());
+    if (mem_map_.get() == nullptr) {
+      return false;
+    } else {
+      return mem_map_->Protect(PROT_READ);
+    }
+  }
+
+ private:
+  std::unique_ptr<MemMap> mem_map_;
+  DISALLOW_COPY_AND_ASSIGN(MemMapContainer);
+};
+
+}  // namespace
+
 using android::base::StringPrintf;
 
 static constexpr OatDexFile* kNoOatDexFile = nullptr;
@@ -151,7 +195,9 @@ std::unique_ptr<const DexFile> DexFileLoader::Open(const uint8_t* base,
                     oat_dex_file,
                     verify,
                     verify_checksum,
-                    error_msg);
+                    error_msg,
+                    /*container*/ nullptr,
+                    /*verify_result*/ nullptr);
 }
 
 std::unique_ptr<const DexFile> DexFileLoader::Open(const std::string& location,
@@ -177,10 +223,9 @@ std::unique_ptr<const DexFile> DexFileLoader::Open(const std::string& location,
                                                  kNoOatDexFile,
                                                  verify,
                                                  verify_checksum,
-                                                 error_msg);
-  if (dex_file != nullptr) {
-    dex_file->mem_map_ = std::move(map);
-  }
+                                                 error_msg,
+                                                 new MemMapContainer(std::move(map)),
+                                                 /*verify_result*/ nullptr);
   return dex_file;
 }
 
@@ -296,10 +341,9 @@ std::unique_ptr<const DexFile> DexFileLoader::OpenFile(int fd,
                                                  kNoOatDexFile,
                                                  verify,
                                                  verify_checksum,
-                                                 error_msg);
-  if (dex_file != nullptr) {
-    dex_file->mem_map_ = std::move(map);
-  }
+                                                 error_msg,
+                                                 new MemMapContainer(std::move(map)),
+                                                 /*verify_result*/ nullptr);
 
   return dex_file;
 }
@@ -365,6 +409,7 @@ std::unique_ptr<const DexFile> DexFileLoader::OpenOneDexFileFromZip(
                                                  verify,
                                                  verify_checksum,
                                                  error_msg,
+                                                 new MemMapContainer(std::move(map)),
                                                  &verify_result);
   if (dex_file == nullptr) {
     if (verify_result == VerifyResult::kVerifyNotAttempted) {
@@ -374,7 +419,6 @@ std::unique_ptr<const DexFile> DexFileLoader::OpenOneDexFileFromZip(
     }
     return nullptr;
   }
-  dex_file->mem_map_ = std::move(map);
   if (!dex_file->DisableWrite()) {
     *error_msg = StringPrintf("Failed to make dex file '%s' read only", location.c_str());
     *error_code = ZipOpenErrorCode::kMakeReadOnlyError;
@@ -465,15 +509,18 @@ std::unique_ptr<DexFile> DexFileLoader::OpenCommon(const uint8_t* base,
                                                    bool verify,
                                                    bool verify_checksum,
                                                    std::string* error_msg,
+                                                   DexFileContainer* container,
                                                    VerifyResult* verify_result) {
   if (verify_result != nullptr) {
     *verify_result = VerifyResult::kVerifyNotAttempted;
   }
   std::unique_ptr<DexFile> dex_file;
   if (StandardDexFile::IsMagicValid(base)) {
-    dex_file.reset(new StandardDexFile(base, size, location, location_checksum, oat_dex_file));
+    dex_file.reset(
+        new StandardDexFile(base, size, location, location_checksum, oat_dex_file, container));
   } else if (CompactDexFile::IsMagicValid(base)) {
-    dex_file.reset(new CompactDexFile(base, size, location, location_checksum, oat_dex_file));
+    dex_file.reset(
+        new CompactDexFile(base, size, location, location_checksum, oat_dex_file, container));
   }
   if (dex_file == nullptr) {
     *error_msg = StringPrintf("Failed to open dex file '%s' from memory: %s", location.c_str(),
diff --git a/runtime/dex_file_loader.h b/runtime/dex_file_loader.h
index a1167dc3fd..97c886ac7d 100644
--- a/runtime/dex_file_loader.h
+++ b/runtime/dex_file_loader.h
@@ -25,6 +25,7 @@
 namespace art {
 
 class DexFile;
+class DexFileContainer;
 class MemMap;
 class OatDexFile;
 class ZipArchive;
@@ -190,7 +191,8 @@ class DexFileLoader {
                                              bool verify,
                                              bool verify_checksum,
                                              std::string* error_msg,
-                                             VerifyResult* verify_result = nullptr);
+                                             DexFileContainer* container,
+                                             VerifyResult* verify_result);
 };
 
 }  // namespace art
diff --git a/runtime/dex_file_verifier_test.cc b/runtime/dex_file_verifier_test.cc
index 7bda89dd8d..ee577e7d9a 100644
--- a/runtime/dex_file_verifier_test.cc
+++ b/runtime/dex_file_verifier_test.cc
@@ -57,7 +57,7 @@ static void FixUpChecksum(uint8_t* dex_file) {
 class DexFileVerifierTest : public CommonRuntimeTest {
  protected:
   DexFile* GetDexFile(const uint8_t* dex_bytes, size_t length) {
-    return new StandardDexFile(dex_bytes, length, "tmp", 0, nullptr);
+    return new StandardDexFile(dex_bytes, length, "tmp", 0, nullptr, nullptr);
   }
 
   void VerifyModification(const char* dex_file_base64_content,
diff --git a/runtime/standard_dex_file.h b/runtime/standard_dex_file.h
index 4fa3efe343..906b0b7b8e 100644
--- a/runtime/standard_dex_file.h
+++ b/runtime/standard_dex_file.h
@@ -49,8 +49,9 @@ class StandardDexFile : public DexFile {
                   size_t size,
                   const std::string& location,
                   uint32_t location_checksum,
-                  const OatDexFile* oat_dex_file)
-      : DexFile(base, size, location, location_checksum, oat_dex_file) {}
+                  const OatDexFile* oat_dex_file,
+                  DexFileContainer* container)
+      : DexFile(base, size, location, location_checksum, oat_dex_file, container) {}
 
   friend class DexFileLoader;
   friend class DexFileVerifierTest;