Bug 1078837 part 2 - Replace IsSystemElf/reinterpret_cast dance with better API. r=nfroyd
authorMike Hommey <mh+mozilla@glandium.org>
Thu, 16 Oct 2014 09:19:45 +0900
changeset 210650 fd07aa0b6a04cfdffb01505315711a2869f96576
parent 210649 b483b12025b0f6135ffa65070cdc250942d91b10
child 210651 ab905a93d0eace4ca8fd85f8e0abe2fe80f303f2
push id9399
push usercbook@mozilla.com
push dateThu, 16 Oct 2014 14:14:26 +0000
treeherderfx-team@5e76f050709f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnfroyd
bugs1078837
milestone36.0a1
Bug 1078837 part 2 - Replace IsSystemElf/reinterpret_cast dance with better API. r=nfroyd
mozglue/linker/BaseElf.h
mozglue/linker/CustomElf.cpp
mozglue/linker/CustomElf.h
mozglue/linker/ElfLoader.cpp
mozglue/linker/ElfLoader.h
--- a/mozglue/linker/BaseElf.h
+++ b/mozglue/linker/BaseElf.h
@@ -74,16 +74,18 @@ public:
   const T *GetPtr(const Elf::Addr offset) const
   {
     if (reinterpret_cast<void *>(offset) > base)
       return reinterpret_cast<const T *>(offset);
     return reinterpret_cast<const T *>(base + offset);
   }
 
   /* Appropriated Mappable */
+  /* /!\ we rely on this being nullptr for BaseElf instances, but not
+   * CustomElf instances. */
   mozilla::RefPtr<Mappable> mappable;
 
   /* Base address where the library is loaded */
   MappedPtr base;
 
   /* Buckets and chains for the System V symbol hash table */
   Array<Elf::Word> buckets;
   UnsizedArray<Elf::Word> chains;
--- a/mozglue/linker/CustomElf.cpp
+++ b/mozglue/linker/CustomElf.cpp
@@ -343,19 +343,18 @@ CustomElf::GetSymbolPtrInDeps(const char
   /* Then search the symbol in our dependencies. Since we already searched in
    * libraries the system linker loaded, skip those (on glibc systems). We
    * also assume the symbol is to be found in one of the dependent libraries
    * directly, not in their own dependent libraries. Building libraries with
    * --no-allow-shlib-undefined ensures such indirect symbol dependency don't
    * happen. */
   for (std::vector<RefPtr<LibHandle> >::const_iterator it = dependencies.begin();
        it < dependencies.end(); ++it) {
-    if (!(*it)->IsSystemElf()) {
-      sym = static_cast<BaseElf *>(
-        static_cast<CustomElf *>((*it).get()))->GetSymbolPtr(symbol, hash);
+    if (BaseElf *be = (*it)->AsBaseElf()) {
+      sym = be->GetSymbolPtr(symbol, hash);
     } else {
       sym = (*it)->GetSymbolPtr(symbol);
     }
     if (sym)
       return sym;
   }
   return nullptr;
 }
--- a/mozglue/linker/CustomElf.h
+++ b/mozglue/linker/CustomElf.h
@@ -49,16 +49,22 @@ protected:
 public:
   /**
    * Shows some stats about the Mappable instance. The when argument is to be
    * used by the caller to give an identifier of the when the stats call is
    * made.
    */
   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
    * given symbol name. This is used to find symbols that are undefined
    * in the Elf object.
    */
   void *GetSymbolPtrInDeps(const char *symbol) const;
 
--- a/mozglue/linker/ElfLoader.cpp
+++ b/mozglue/linker/ElfLoader.cpp
@@ -447,40 +447,54 @@ ElfLoader::GetMappableFromPath(const cha
 
   return mappable;
 }
 
 void
 ElfLoader::Register(LibHandle *handle)
 {
   handles.push_back(handle);
-  if (dbg && !handle->IsSystemElf())
-    dbg.Add(static_cast<CustomElf *>(handle));
+}
+
+void
+ElfLoader::Register(CustomElf *handle)
+{
+  Register(static_cast<LibHandle *>(handle));
+  if (dbg) {
+    dbg.Add(handle);
+  }
 }
 
 void
 ElfLoader::Forget(LibHandle *handle)
 {
   /* Ensure logging is initialized or refresh if environment changed. */
   Logging::Init();
 
   LibHandleList::iterator it = std::find(handles.begin(), handles.end(), handle);
   if (it != handles.end()) {
     DEBUG_LOG("ElfLoader::Forget(%p [\"%s\"])", reinterpret_cast<void *>(handle),
                                                 handle->GetPath());
-    if (dbg && !handle->IsSystemElf())
-      dbg.Remove(static_cast<CustomElf *>(handle));
     handles.erase(it);
   } else {
     DEBUG_LOG("ElfLoader::Forget(%p [\"%s\"]): Handle not found",
               reinterpret_cast<void *>(handle), handle->GetPath());
   }
 }
 
 void
+ElfLoader::Forget(CustomElf *handle)
+{
+  Forget(static_cast<LibHandle *>(handle));
+  if (dbg) {
+    dbg.Remove(handle);
+  }
+}
+
+void
 ElfLoader::Init()
 {
   Dl_info info;
   /* On Android < 4.1 can't reenter dl* functions. So when the library
    * containing this code is dlopen()ed, it can't call dladdr from a
    * static initializer. */
   if (dladdr(_DYNAMIC, &info) != 0) {
     /* Ideally, we wouldn't be initializing self_elf this way, but until
@@ -529,33 +543,33 @@ ElfLoader::~ElfLoader()
   /* Build up a list of all library handles with direct (external) references.
    * We actually skip system library handles because we want to keep at least
    * some of these open. Most notably, Mozilla codebase keeps a few libgnome
    * libraries deliberately open because of the mess that libORBit destruction
    * is. dlclose()ing these libraries actually leads to problems. */
   for (LibHandleList::reverse_iterator it = handles.rbegin();
        it < handles.rend(); ++it) {
     if ((*it)->DirectRefCount()) {
-      if ((*it)->IsSystemElf()) {
-        static_cast<SystemElf *>(*it)->Forget();
+      if (SystemElf *se = (*it)->AsSystemElf()) {
+        se->Forget();
       } else {
         list.push_back(*it);
       }
     }
   }
   /* Force release all external references to the handles collected above */
   for (LibHandleList::iterator it = list.begin(); it < list.end(); ++it) {
     while ((*it)->ReleaseDirectRef()) { }
   }
   /* Remove the remaining system handles. */
   if (handles.size()) {
     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());
       } else {
         DEBUG_LOG("ElfLoader::~ElfLoader(): Unexpected remaining handle for \"%s\" "
                   "[%d direct refs, %d refs total]", (*it)->GetPath(),
                   (*it)->DirectRefCount(), (*it)->refCount());
         /* Not removing, since it could have references to other libraries,
@@ -1198,21 +1212,22 @@ void SEGVHandler::handler(int signum, si
   //ASSERT(signum == SIGSEGV);
   DEBUG_LOG("Caught segmentation fault @%p", info->si_addr);
 
   /* Check whether we segfaulted in the address space of a CustomElf. We're
    * only expecting that to happen as an access error. */
   if (info->si_code == SEGV_ACCERR) {
     mozilla::RefPtr<LibHandle> handle =
       ElfLoader::Singleton.GetHandleByPtr(info->si_addr);
-    if (handle && !handle->IsSystemElf()) {
-      DEBUG_LOG("Within the address space of a CustomElf");
-      CustomElf *elf = static_cast<CustomElf *>(static_cast<LibHandle *>(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;
+      }
     }
   }
 
   /* Redispatch to the registered handler */
   SEGVHandler &that = ElfLoader::Singleton;
   if (that.action.sa_flags & SA_SIGINFO) {
     DEBUG_LOG("Redispatching to registered handler @%p",
               FunctionPtr(that.action.sa_sigaction));
--- a/mozglue/linker/ElfLoader.h
+++ b/mozglue/linker/ElfLoader.h
@@ -59,18 +59,20 @@ MFBT_API void *
 MFBT_API void
 __dl_munmap(void *handle, void *addr, size_t length);
 
 MFBT_API bool
 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
  * LibHandles during the execution of their destructor, so we need
  * RefCounted<LibHandle>::Release to support some reentrancy. See further
  * below.
  */
 class LibHandle;
@@ -212,23 +214,25 @@ public:
 
 protected:
   /**
    * Returns a mappable object for use by MappableMMap and related functions.
    */
   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;
   char *path;
 
   /* Mappable object keeping the result of GetMappable() */
   mutable mozilla::RefPtr<Mappable> mappable;
 };
@@ -288,21 +292,21 @@ public:
 #ifdef __ARM_EABI__
   virtual const void *FindExidx(int *pcount) const;
 #endif
 
 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()
    * being called when the instance is destroyed.
    */
   void Forget()
   {
     dlhandle = nullptr;
@@ -428,22 +432,24 @@ public:
   static Mappable *GetMappableFromPath(const char *path);
 
 protected:
   /**
    * Registers the given handle. This method is meant to be called by
    * 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;
   friend const char *__wrap_dlerror(void);
   friend void *__wrap_dlsym(void *handle, const char *symbol);
   friend int __wrap_dlclose(void *handle);
   const char *lastError;