From a682d7fe750f162e4e014b2d34e129bcc06c8e47 Mon Sep 17 00:00:00 2001 From: Mike Hommey Date: Thu, 27 Jun 2013 09:35:48 +0900 Subject: [PATCH] Bug 885336 - Fix various issues with the dl_mmap interface. r=nfroyd --- mozglue/linker/CustomElf.cpp | 3 +-- mozglue/linker/CustomElf.h | 4 +--- mozglue/linker/ElfLoader.cpp | 18 ++++++++---------- mozglue/linker/ElfLoader.h | 6 ++---- mozglue/linker/Mappable.h | 5 ++++- 5 files changed, 16 insertions(+), 20 deletions(-) diff --git a/mozglue/linker/CustomElf.cpp b/mozglue/linker/CustomElf.cpp index 1833d47b38b..8d14761c250 100644 --- a/mozglue/linker/CustomElf.cpp +++ b/mozglue/linker/CustomElf.cpp @@ -95,7 +95,7 @@ private: mappable->munmap(buf, length); } - Mappable *mappable; + mozilla::RefPtr mappable; }; @@ -233,7 +233,6 @@ CustomElf::~CustomElf() * 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); } diff --git a/mozglue/linker/CustomElf.h b/mozglue/linker/CustomElf.h index 4a29727af1e..f3d8d8651dd 100644 --- a/mozglue/linker/CustomElf.h +++ b/mozglue/linker/CustomElf.h @@ -9,8 +9,6 @@ #include "Logging.h" #include "Elfxx.h" -class Mappable; - /** * Library Handle class for ELF libraries we don't let the system linker * handle. @@ -157,7 +155,7 @@ private: } /* Appropriated Mappable */ - Mappable *mappable; + mozilla::RefPtr mappable; /* Base address where the library is loaded */ MappedPtr base; diff --git a/mozglue/linker/ElfLoader.cpp b/mozglue/linker/ElfLoader.cpp index 955eacb42dc..6eabe54d3a7 100644 --- a/mozglue/linker/ElfLoader.cpp +++ b/mozglue/linker/ElfLoader.cpp @@ -182,8 +182,6 @@ LeafName(const char *path) LibHandle::~LibHandle() { free(path); - if (mappable && mappable->GetKind() != Mappable::MAPPABLE_EXTRACT_FILE) - delete mappable; } const char * @@ -195,9 +193,8 @@ LibHandle::GetName() const 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(); @@ -206,8 +203,10 @@ LibHandle::GetMappableLength() const 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 */ @@ -221,9 +220,8 @@ LibHandle::MappableMMap(void *addr, size_t length, off_t offset) const 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); } /** diff --git a/mozglue/linker/ElfLoader.h b/mozglue/linker/ElfLoader.h index ae44bfe5434..effe198ab67 100644 --- a/mozglue/linker/ElfLoader.h +++ b/mozglue/linker/ElfLoader.h @@ -11,6 +11,7 @@ #include "mozilla/RefPtr.h" #include "Zip.h" #include "Elfxx.h" +#include "Mappable.h" /** * dlfcn.h replacement functions @@ -76,9 +77,6 @@ template <> inline RefCounted::~RefCounted() } /* 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. @@ -200,7 +198,7 @@ private: char *path; /* Mappable object keeping the result of GetMappable() */ - mutable Mappable *mappable; + mutable mozilla::RefPtr mappable; }; /** diff --git a/mozglue/linker/Mappable.h b/mozglue/linker/Mappable.h index dea213d22cb..82e61e8f473 100644 --- a/mozglue/linker/Mappable.h +++ b/mozglue/linker/Mappable.h @@ -22,7 +22,7 @@ * - memory after length and up to the end of the corresponding page is nulled * out. */ -class Mappable +class Mappable: public mozilla::RefCounted { public: virtual ~Mappable() { } @@ -120,6 +120,9 @@ public: */ 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)