Bug 885336 - Fix various issues with the dl_mmap interface. r=nfroyd
authorMike Hommey <mh+mozilla@glandium.org>
Thu, 27 Jun 2013 09:35:48 +0900
changeset 148918 c4abdcc3451cb6cc74d5b87ca6a8cac5557fbe29
parent 148917 8d8e2e2734fc5df3f63832cf2e799bd0b3ba3594
child 148919 ba30d5bdaf222e51b28c1da52badcfc54b6bd797
push id2859
push userakeybl@mozilla.com
push dateMon, 16 Sep 2013 19:14:59 +0000
treeherdermozilla-beta@87d3c51cd2bf [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnfroyd
bugs885336
milestone25.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 885336 - Fix various issues with the dl_mmap interface. r=nfroyd
mozglue/linker/CustomElf.cpp
mozglue/linker/CustomElf.h
mozglue/linker/ElfLoader.cpp
mozglue/linker/ElfLoader.h
mozglue/linker/Mappable.h
--- a/mozglue/linker/CustomElf.cpp
+++ b/mozglue/linker/CustomElf.cpp
@@ -90,17 +90,17 @@ public:
   }
 
 private:
   friend class GenericMappedPtr<Mappable1stPagePtr>;
   void munmap(void *buf, size_t length) {
     mappable->munmap(buf, length);
   }
 
-  Mappable *mappable;
+  mozilla::RefPtr<Mappable> mappable;
 };
 
 
 TemporaryRef<LibHandle>
 CustomElf::Load(Mappable *mappable, const char *path, int flags)
 {
   debug("CustomElf::Load(\"%s\", 0x%x) = ...", path, flags);
   if (!mappable)
@@ -228,17 +228,16 @@ CustomElf::~CustomElf()
 {
   debug("CustomElf::~CustomElf(%p [\"%s\"])",
         reinterpret_cast<void *>(this), GetPath());
   CallFini();
   /* Normally, __cxa_finalize is called by the .fini function. However,
    * Android NDK before r6b doesn't do that. Our wrapped cxa_finalize only
    * calls destructors once, so call it in all cases. */
   ElfLoader::__wrap_cxa_finalize(this);
-  delete mappable;
   ElfLoader::Singleton.Forget(this);
 }
 
 namespace {
 
 /**
  * Hash function for symbol lookup, as defined in ELF standard for System V
  */
--- a/mozglue/linker/CustomElf.h
+++ b/mozglue/linker/CustomElf.h
@@ -4,18 +4,16 @@
 
 #ifndef CustomElf_h
 #define CustomElf_h
 
 #include "ElfLoader.h"
 #include "Logging.h"
 #include "Elfxx.h"
 
-class Mappable;
-
 /**
  * Library Handle class for ELF libraries we don't let the system linker
  * handle.
  */
 class CustomElf: public LibHandle, private ElfLoader::link_map
 {
   friend class ElfLoader;
   friend class SEGVHandler;
@@ -152,17 +150,17 @@ private:
    * Call a function given a an address relative to the library base
    */
   void CallFunction(Elf::Addr addr) const
   {
     return CallFunction(GetPtr(addr));
   }
 
   /* Appropriated Mappable */
-  Mappable *mappable;
+  mozilla::RefPtr<Mappable> mappable;
 
   /* Base address where the library is loaded */
   MappedPtr base;
 
   /* String table */
   Elf::Strtab strtab;
 
   /* Symbol table */
--- a/mozglue/linker/ElfLoader.cpp
+++ b/mozglue/linker/ElfLoader.cpp
@@ -177,58 +177,56 @@ LeafName(const char *path)
 } /* Anonymous namespace */
 
 /**
  * LibHandle
  */
 LibHandle::~LibHandle()
 {
   free(path);
-  if (mappable && mappable->GetKind() != Mappable::MAPPABLE_EXTRACT_FILE)
-    delete mappable;
 }
 
 const char *
 LibHandle::GetName() const
 {
   return path ? LeafName(path) : NULL;
 }
 
 size_t
 LibHandle::GetMappableLength() const
 {
-  MOZ_ASSERT(mappable != NULL, "GetMappableLength needs to be called first,"
-                               " and only once");
-  mappable = GetMappable();
+  if (!mappable)
+    mappable = GetMappable();
   if (!mappable)
     return 0;
   return mappable->GetLength();
 }
 
 void *
 LibHandle::MappableMMap(void *addr, size_t length, off_t offset) const
 {
-  MOZ_ASSERT(mappable == NULL, "MappableMMap must be called after"
-                               " GetMappableLength");
+  if (!mappable)
+    mappable = GetMappable();
+  if (!mappable)
+    return MAP_FAILED;
   void* mapped = mappable->mmap(addr, length, PROT_READ, MAP_PRIVATE, offset);
   if (mapped != MAP_FAILED) {
     /* Ensure the availability of all pages within the mapping */
     for (size_t off = 0; off < length; off += PAGE_SIZE) {
       mappable->ensure(reinterpret_cast<char *>(mapped) + off);
     }
   }
   return mapped;
 }
 
 void
 LibHandle::MappableMUnmap(void *addr, size_t length) const
 {
-  MOZ_ASSERT(mappable == NULL, "MappableMUnmap must be called after"
-                               " MappableMMap and GetMappableLength");
-  mappable->munmap(addr, length);
+  if (mappable)
+    mappable->munmap(addr, length);
 }
 
 /**
  * SystemElf
  */
 TemporaryRef<LibHandle>
 SystemElf::Load(const char *path, int flags)
 {
--- a/mozglue/linker/ElfLoader.h
+++ b/mozglue/linker/ElfLoader.h
@@ -6,16 +6,17 @@
 #define ElfLoader_h
 
 #include <vector>
 #include <dlfcn.h>
 #include <signal.h>
 #include "mozilla/RefPtr.h"
 #include "Zip.h"
 #include "Elfxx.h"
+#include "Mappable.h"
 
 /**
  * dlfcn.h replacement functions
  */
 extern "C" {
   void *__wrap_dlopen(const char *path, int flags);
   const char *__wrap_dlerror(void);
   void *__wrap_dlsym(void *handle, const char *symbol);
@@ -71,19 +72,16 @@ template <> inline void RefCounted<LibHa
 template <> inline RefCounted<LibHandle, AtomicRefCount>::~RefCounted()
 {
   MOZ_ASSERT(refCnt == 0x7fffdead);
 }
 
 } /* namespace detail */
 } /* namespace mozilla */
 
-/* Forward declaration */
-class Mappable;
-
 /**
  * Abstract class for loaded libraries. Libraries may be loaded through the
  * system linker or this linker, both cases will be derived from this class.
  */
 class LibHandle: public mozilla::AtomicRefCounted<LibHandle>
 {
 public:
   /**
@@ -195,17 +193,17 @@ protected:
   friend class SEGVHandler;
   virtual bool IsSystemElf() const { return false; }
 
 private:
   int directRefCnt;
   char *path;
 
   /* Mappable object keeping the result of GetMappable() */
-  mutable Mappable *mappable;
+  mutable mozilla::RefPtr<Mappable> mappable;
 };
 
 /**
  * Specialized RefCounted<LibHandle>::Release. Under normal operation, when
  * refCnt reaches 0, the LibHandle is deleted. Its refCnt is however increased
  * to 1 on normal builds, and 0x7fffdead on debug builds so that the LibHandle
  * can still be referenced while the destructor is executing. The refCnt is
  * allowed to grow > 0x7fffdead, but not to decrease under that value, which
--- a/mozglue/linker/Mappable.h
+++ b/mozglue/linker/Mappable.h
@@ -17,17 +17,17 @@
  * Abstract class to handle mmap()ing from various kind of entities, such as
  * plain files or Zip entries. The virtual members are meant to act as the
  * equivalent system functions, with a few differences:
  * - mapped memory is always MAP_PRIVATE, even though a given implementation
  *   may use something different internally.
  * - memory after length and up to the end of the corresponding page is nulled
  *   out.
  */
-class Mappable
+class Mappable: public mozilla::RefCounted<Mappable>
 {
 public:
   virtual ~Mappable() { }
 
   virtual void *mmap(const void *addr, size_t length, int prot, int flags,
                      off_t offset) = 0;
 
   enum Kind {
@@ -115,16 +115,19 @@ public:
   ~MappableExtractFile();
 
   /**
    * Create a MappableExtractFile instance for the given Zip stream. The name
    * argument is used to create the cache file in the cache directory.
    */
   static Mappable *Create(const char *name, Zip *zip, Zip::Stream *stream);
 
+  /* Override finalize from MappableFile */
+  virtual void finalize() {}
+
   virtual Kind GetKind() const { return MAPPABLE_EXTRACT_FILE; };
 private:
   MappableExtractFile(int fd, char *path)
   : MappableFile(fd), path(path), pid(getpid()) { }
 
   /**
    * AutoUnlinkFile keeps track or a file name and removes (unlinks) the file
    * when the instance is destroyed.