Bug 1412405 - fix isnanf symbol lookup by using a LoadedElf for libm; r=glandium
authorNathan Froyd <froydnj@mozilla.com>
Sat, 28 Oct 2017 08:51:23 -0400
changeset 439626 339e44e41f22670bd665d935ef56f2262f9360db
parent 439625 ca254ca987755ab903cd6a8a4370346d6c8e28d4
child 439627 c0eb1f08953b31362483a415465d2964a67a5f0c
push id8114
push userjlorenzo@mozilla.com
push dateThu, 02 Nov 2017 16:33:21 +0000
treeherdermozilla-beta@73e0d89a540f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersglandium
bugs1412405, 1081034
milestone58.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 1412405 - fix isnanf symbol lookup by using a LoadedElf for libm; r=glandium We already dealt with issues around dlsym not resolving weak symbols with libc in bug 1081034. This fix applies the same workaround to libm, which solves the isnanf issue. The previous fix for looking up __isnanf is no longer needed.
mozglue/linker/ElfLoader.cpp
mozglue/linker/ElfLoader.h
--- a/mozglue/linker/ElfLoader.cpp
+++ b/mozglue/linker/ElfLoader.cpp
@@ -15,16 +15,17 @@
 #include "BaseElf.h"
 #include "CustomElf.h"
 #include "Mappable.h"
 #include "Logging.h"
 #include <inttypes.h>
 
 #if defined(ANDROID)
 #include <sys/syscall.h>
+#include <math.h>
 
 #include <android/api-level.h>
 #if __ANDROID_API__ < 8
 /* Android API < 8 doesn't provide sigaltstack */
 
 extern "C" {
 
 inline int sigaltstack(const stack_t *ss, stack_t *oss) {
@@ -326,35 +327,16 @@ SystemElf::~SystemElf()
   ElfLoader::Singleton.lastError = dlerror();
   ElfLoader::Singleton.Forget(this);
 }
 
 void *
 SystemElf::GetSymbolPtr(const char *symbol) const
 {
   void *sym = dlsym(dlhandle, symbol);
-  // Various bits of Gecko use isnanf, which gcc is happy to compile into
-  // inlined code using floating-point comparisons.  clang, on the other hand,
-  // does not use inline code and generates full calls to isnanf.
-  //
-  // libm.so on Android defines isnanf as weak.  dlsym always returns null for
-  // weak symbols.  Which means that we'll never be able to resolve the symbol
-  // that clang generates here.  However, said weak symbol for isnanf is just
-  // an alias for __isnanf, which is the real definition.  So if we're asked
-  // for isnanf and we can't find it, try looking for __isnanf instead.  The
-  // actual system linker uses alternate resolution interfaces and therefore
-  // does not encounter this issue.
-  //
-  // See also https://bugs.chromium.org/p/chromium/issues/detail?id=376828,
-  // from which this comment and this fix are adapted.
-  if (!sym &&
-      !strcmp(symbol, "isnanf") &&
-      !strcmp(GetName(), "libm.so")) {
-    sym = dlsym(dlhandle, "__isnanf");
-  }
   DEBUG_LOG("dlsym(%p [\"%s\"], \"%s\") = %p", dlhandle, GetPath(), symbol, sym);
   ElfLoader::Singleton.lastError = dlerror();
   return sym;
 }
 
 Mappable *
 SystemElf::GetMappable() const
 {
@@ -571,31 +553,35 @@ ElfLoader::Init()
    * static initializer. */
   if (dladdr(_DYNAMIC, &info) != 0) {
     self_elf = LoadedElf::Create(info.dli_fname, info.dli_fbase);
   }
 #if defined(ANDROID)
   if (dladdr(FunctionPtr(syscall), &info) != 0) {
     libc = LoadedElf::Create(info.dli_fname, info.dli_fbase);
   }
+  if (dladdr(FunctionPtr<int (*)(double)>(isnan), &info) != 0) {
+    libm = LoadedElf::Create(info.dli_fname, info.dli_fbase);
+  }
 #endif
 }
 
 ElfLoader::~ElfLoader()
 {
   LibHandleList list;
 
   if (!Singleton.IsShutdownExpected()) {
     MOZ_CRASH("Unexpected shutdown");
   }
 
   /* Release self_elf and libc */
   self_elf = nullptr;
 #if defined(ANDROID)
   libc = nullptr;
+  libm = nullptr;
 #endif
 
   AutoLock lock(&handlesMutex);
   /* 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. */
--- a/mozglue/linker/ElfLoader.h
+++ b/mozglue/linker/ElfLoader.h
@@ -468,16 +468,19 @@ private:
   RefPtr<LibHandle> self_elf;
 
 #if defined(ANDROID)
   /* System loader handle for the libc. This is used to resolve weak symbols
    * that some libcs contain that the Android linker won't dlsym(). Normally,
    * we wouldn't treat non-Android differently, but glibc uses versioned
    * symbols which this linker doesn't support. */
   RefPtr<LibHandle> libc;
+
+  /* And for libm. */
+  RefPtr<LibHandle> libm;
 #endif
 
   /* Bookkeeping */
   typedef std::vector<LibHandle *> LibHandleList;
   LibHandleList handles;
 
   pthread_mutex_t handlesMutex;