From 84f2c13cef3094055d837f3dc18b74272158ebbb Mon Sep 17 00:00:00 2001 From: Mike Hommey Date: Thu, 16 Oct 2014 09:19:45 +0900 Subject: [PATCH] Bug 1078837 part 2 - Replace IsSystemElf/reinterpret_cast dance with better API. r=nfroyd --- mozglue/linker/BaseElf.h | 2 ++ mozglue/linker/CustomElf.cpp | 5 ++--- mozglue/linker/CustomElf.h | 6 ++++++ mozglue/linker/ElfLoader.cpp | 37 +++++++++++++++++++++++++----------- mozglue/linker/ElfLoader.h | 20 ++++++++++++------- 5 files changed, 49 insertions(+), 21 deletions(-) diff --git a/mozglue/linker/BaseElf.h b/mozglue/linker/BaseElf.h index 6ed2f91e8279..18091f6d15dd 100644 --- a/mozglue/linker/BaseElf.h +++ b/mozglue/linker/BaseElf.h @@ -79,6 +79,8 @@ public: } /* Appropriated Mappable */ + /* /!\ we rely on this being nullptr for BaseElf instances, but not + * CustomElf instances. */ mozilla::RefPtr mappable; /* Base address where the library is loaded */ diff --git a/mozglue/linker/CustomElf.cpp b/mozglue/linker/CustomElf.cpp index 683ef4689b77..64c8a48439ed 100644 --- a/mozglue/linker/CustomElf.cpp +++ b/mozglue/linker/CustomElf.cpp @@ -348,9 +348,8 @@ CustomElf::GetSymbolPtrInDeps(const char *symbol) const * happen. */ for (std::vector >::const_iterator it = dependencies.begin(); it < dependencies.end(); ++it) { - if (!(*it)->IsSystemElf()) { - sym = static_cast( - static_cast((*it).get()))->GetSymbolPtr(symbol, hash); + if (BaseElf *be = (*it)->AsBaseElf()) { + sym = be->GetSymbolPtr(symbol, hash); } else { sym = (*it)->GetSymbolPtr(symbol); } diff --git a/mozglue/linker/CustomElf.h b/mozglue/linker/CustomElf.h index d727b0b7e1d0..0c51c68ce2d3 100644 --- a/mozglue/linker/CustomElf.h +++ b/mozglue/linker/CustomElf.h @@ -54,6 +54,12 @@ public: */ virtual void stats(const char *when) const; + /** + * Returns the instance, casted as BaseElf. (short of a better way to do + * this without RTTI) + */ + virtual BaseElf *AsBaseElf() { return this; } + private: /** * Scan dependent libraries to find the address corresponding to the diff --git a/mozglue/linker/ElfLoader.cpp b/mozglue/linker/ElfLoader.cpp index 880ebb907e4d..2f434beb379f 100644 --- a/mozglue/linker/ElfLoader.cpp +++ b/mozglue/linker/ElfLoader.cpp @@ -452,8 +452,15 @@ void ElfLoader::Register(LibHandle *handle) { handles.push_back(handle); - if (dbg && !handle->IsSystemElf()) - dbg.Add(static_cast(handle)); +} + +void +ElfLoader::Register(CustomElf *handle) +{ + Register(static_cast(handle)); + if (dbg) { + dbg.Add(handle); + } } void @@ -466,8 +473,6 @@ ElfLoader::Forget(LibHandle *handle) if (it != handles.end()) { DEBUG_LOG("ElfLoader::Forget(%p [\"%s\"])", reinterpret_cast(handle), handle->GetPath()); - if (dbg && !handle->IsSystemElf()) - dbg.Remove(static_cast(handle)); handles.erase(it); } else { DEBUG_LOG("ElfLoader::Forget(%p [\"%s\"]): Handle not found", @@ -475,6 +480,15 @@ ElfLoader::Forget(LibHandle *handle) } } +void +ElfLoader::Forget(CustomElf *handle) +{ + Forget(static_cast(handle)); + if (dbg) { + dbg.Remove(handle); + } +} + void ElfLoader::Init() { @@ -534,8 +548,8 @@ ElfLoader::~ElfLoader() for (LibHandleList::reverse_iterator it = handles.rbegin(); it < handles.rend(); ++it) { if ((*it)->DirectRefCount()) { - if ((*it)->IsSystemElf()) { - static_cast(*it)->Forget(); + if (SystemElf *se = (*it)->AsSystemElf()) { + se->Forget(); } else { list.push_back(*it); } @@ -550,7 +564,7 @@ ElfLoader::~ElfLoader() list = handles; for (LibHandleList::reverse_iterator it = list.rbegin(); it < list.rend(); ++it) { - if ((*it)->IsSystemElf()) { + if ((*it)->AsSystemElf()) { DEBUG_LOG("ElfLoader::~ElfLoader(): Remaining handle for \"%s\" " "[%d direct refs, %d refs total]", (*it)->GetPath(), (*it)->DirectRefCount(), (*it)->refCount()); @@ -1203,11 +1217,12 @@ void SEGVHandler::handler(int signum, siginfo_t *info, void *context) if (info->si_code == SEGV_ACCERR) { mozilla::RefPtr handle = ElfLoader::Singleton.GetHandleByPtr(info->si_addr); - if (handle && !handle->IsSystemElf()) { - DEBUG_LOG("Within the address space of a CustomElf"); - CustomElf *elf = static_cast(static_cast(handle)); - if (elf->mappable->ensure(info->si_addr)) + BaseElf *elf; + if (handle && (elf = handle->AsBaseElf())) { + DEBUG_LOG("Within the address space of %s", handle->GetPath()); + if (elf->mappable && elf->mappable->ensure(info->si_addr)) { return; + } } } diff --git a/mozglue/linker/ElfLoader.h b/mozglue/linker/ElfLoader.h index 3c3753dbb37e..5017176d5090 100644 --- a/mozglue/linker/ElfLoader.h +++ b/mozglue/linker/ElfLoader.h @@ -64,8 +64,10 @@ IsSignalHandlingBroken(); } -/* Forward declaration because BaseElf.h includes ElfLoader.h */ +/* Forward declarations for use in LibHandle */ class BaseElf; +class CustomElf; +class SystemElf; /** * Specialize RefCounted template for LibHandle. We may get references to @@ -217,13 +219,15 @@ protected: virtual Mappable *GetMappable() const = 0; /** - * Returns whether the handle is a SystemElf or not. (short of a better way - * to do this without RTTI) + * Returns the instance, casted as the wanted type. Returns nullptr if + * that's not the actual type. (short of a better way to do this without + * RTTI) */ friend class ElfLoader; friend class CustomElf; friend class SEGVHandler; - virtual bool IsSystemElf() const { return false; } + virtual BaseElf *AsBaseElf() { return nullptr; } + virtual SystemElf *AsSystemElf() { return nullptr; } private: MozRefCountType directRefCnt; @@ -293,11 +297,11 @@ protected: virtual Mappable *GetMappable() const; /** - * Returns whether the handle is a SystemElf or not. (short of a better way - * to do this without RTTI) + * Returns the instance, casted as SystemElf. (short of a better way to do + * this without RTTI) */ friend class ElfLoader; - virtual bool IsSystemElf() const { return true; } + virtual SystemElf *AsSystemElf() { return this; } /** * Remove the reference to the system linker handle. This avoids dlclose() @@ -433,12 +437,14 @@ protected: * LibHandle subclass creators. */ void Register(LibHandle *handle); + void Register(CustomElf *handle); /** * Forget about the given handle. This method is meant to be called by * LibHandle subclass destructors. */ void Forget(LibHandle *handle); + void Forget(CustomElf *handle); /* Last error. Used for dlerror() */ friend class SystemElf;