marsupial created this revision.

This is mostly affecting usage of the JIT...
When using DynamicLibrary::getPermanentLibrary(0,0) to get a list of the loaded 
libs, on Windows this is currently stored in a set which makes iteration 
unordered / undefined. Additionally there is a problem as newer Windows are 
using ucrt.dll for a lot of stdlib functions which cause a disagreement between 
the JIT and native code as to what the address and implementation of that 
function is:

JIT: putenv_s("TEST", "VALUE") calls msvcrt.dll, putenv_s
JIT: getenv("TEST") -> "VALUE" calls msvcrt.dll, getenv
BIN: getenv("TEST") -> NULL // calls ucrt.dll, getenv

The patch simply changes the way modules are loaded and stored to how Windows 
says they were.
Searches for symbols in reverse order so newer modules will override older ones.
And always tries to give priority to the process' symbols (what dlsym does I 
believe)


https://reviews.llvm.org/D30107

Files:
  lib/Support/Windows/DynamicLibrary.inc

Index: lib/Support/Windows/DynamicLibrary.inc
===================================================================
--- lib/Support/Windows/DynamicLibrary.inc
+++ lib/Support/Windows/DynamicLibrary.inc
@@ -12,16 +12,18 @@
 //===----------------------------------------------------------------------===//
 
 #include "WindowsSupport.h"
+#include "llvm/ADT/iterator_range.h"
 
 #ifdef __MINGW32__
  #include <imagehlp.h>
 #else
- #include <dbghelp.h>
+ #include <Psapi.h>
 #endif
 
 #ifdef _MSC_VER
  #include <ntverp.h>
 #endif
+#include <vector>
 
 namespace llvm {
 using namespace sys;
@@ -31,25 +33,9 @@
 //===          and must not be UNIX code.
 //===----------------------------------------------------------------------===//
 
-typedef BOOL (WINAPI *fpEnumerateLoadedModules)(HANDLE,PENUMLOADED_MODULES_CALLBACK64,PVOID);
-static fpEnumerateLoadedModules fEnumerateLoadedModules;
-static DenseSet<HMODULE> *OpenedHandles;
-
-static bool loadDebugHelp(void) {
-  HMODULE hLib = ::LoadLibraryW(L"Dbghelp.dll");
-  if (hLib) {
-    fEnumerateLoadedModules = (fpEnumerateLoadedModules)
-      ::GetProcAddress(hLib, "EnumerateLoadedModules64");
-  }
-  return fEnumerateLoadedModules != 0;
-}
-
-static BOOL CALLBACK
-ELM_Callback(PCSTR ModuleName, DWORD64 ModuleBase,
-             ULONG ModuleSize, PVOID UserContext) {
-  OpenedHandles->insert((HMODULE)ModuleBase);
-  return TRUE;
-}
+typedef std::vector<HMODULE> ModuleHandles;
+static std::unique_ptr<ModuleHandles> OpenedHandles;
+static bool KeepFront;
 
 DynamicLibrary DynamicLibrary::getPermanentLibrary(const char *filename,
                                                    std::string *errMsg) {
@@ -57,20 +43,58 @@
 
   if (!filename) {
     // When no file is specified, enumerate all DLLs and EXEs in the process.
-    if (OpenedHandles == 0)
-      OpenedHandles = new DenseSet<HMODULE>();
 
-    if (!fEnumerateLoadedModules) {
-      if (!loadDebugHelp()) {
-        assert(false && "These APIs should always be available");
+#ifdef _WIN64
+    const DWORD Flags = LIST_MODULES_64BIT;
+#else
+    const DWORD Flags = LIST_MODULES_32BIT;
+#endif
+
+    DWORD Bytes;
+    HMODULE Self = (HMODULE)GetCurrentProcess();
+    if (!EnumProcessModulesEx(Self, nullptr, 0, &Bytes, Flags)) {
+      MakeErrMsg(errMsg, "EnumProcessModulesEx failure");
+      return DynamicLibrary();
+    }
+
+    // Get the most recent list in case any modules added between calls.
+    ModuleHandles Current;
+    do {
+      assert(Bytes && ((Bytes % sizeof(HMODULE)) == 0) &&
+             "Should have at least one module and be aligned");
+      Current.resize(Bytes / sizeof(HMODULE));
+      if (!EnumProcessModulesEx(Self, Current.data(), Bytes, &Bytes, Flags)) {
+        MakeErrMsg(errMsg, "EnumProcessModulesEx failure");
         return DynamicLibrary();
       }
+    } while (Bytes != (Current.size() * sizeof(HMODULE)));
+
+#ifndef NDEBUG
+    if (OpenedHandles) {
+      auto Begin = Current.begin(), End = Current.end();
+      for (HMODULE Mod : *OpenedHandles) {
+        assert((std::find(Begin, End, Mod) != End) &&
+               "Module in older list that is not in newer one");
+      }
     }
+#endif
+
+    // Keep current process handle at top of list
+    if (Current.size() > 1) {
+      Self = Current.front();
+      memmove(Current.data(), Current.data()+1, Bytes - sizeof(HMODULE));
+      Current.back() = Self;
+    }
+    KeepFront = true;
+
+    if (!OpenedHandles)
+      OpenedHandles.reset(new ModuleHandles(std::move(Current)));
+    else
+      OpenedHandles->swap(Current);
 
-    fEnumerateLoadedModules(GetCurrentProcess(), ELM_Callback, 0);
     // Dummy library that represents "search all handles".
     // This is mostly to ensure that the return value still shows up as "valid".
-    return DynamicLibrary(&OpenedHandles);
+    return DynamicLibrary(OpenedHandles.get());
   }
 
   SmallVector<wchar_t, MAX_PATH> filenameUnicode;
@@ -79,7 +103,7 @@
     MakeErrMsg(errMsg, std::string(filename) + ": Can't convert to UTF-16");
     return DynamicLibrary();
   }
-  
+
   HMODULE a_handle = LoadLibraryW(filenameUnicode.data());
 
   if (a_handle == 0) {
@@ -87,13 +111,25 @@
     return DynamicLibrary();
   }
 
-  if (OpenedHandles == 0)
-    OpenedHandles = new DenseSet<HMODULE>();
+  if (OpenedHandles) {
+    // If we've already loaded this library, FreeLibrary() the handle in order
+    // to keep the internal refcount at +1.
+    auto It = std::find(OpenedHandles->begin(), OpenedHandles->end(), a_handle);
+    if (It != OpenedHandles->end()) {
+      FreeLibrary(a_handle);
+      return DynamicLibrary(a_handle);
+    }
+  } else
+    OpenedHandles.reset(new ModuleHandles);
 
-  // If we've already loaded this library, FreeLibrary() the handle in order to
-  // keep the internal refcount at +1.
-  if (!OpenedHandles->insert(a_handle).second)
-    FreeLibrary(a_handle);
+  OpenedHandles->push_back(a_handle);
+
+  if (KeepFront) {
+    // Keep current process handle at top of list
+    const size_t Len = OpenedHandles->size();
+    if (Len > 2)
+      std::swap((*OpenedHandles)[Len-1], (*OpenedHandles)[Len-2]);
+  }
 
   return DynamicLibrary(a_handle);
 }
@@ -134,11 +170,11 @@
       return i->second;
   }
 
-  // Now search the libraries.
+  // Now search the libraries (in reverse, newer symbols override oldest)
   if (OpenedHandles) {
-    for (DenseSet<HMODULE>::iterator I = OpenedHandles->begin(),
-         E = OpenedHandles->end(); I != E; ++I) {
-      FARPROC ptr = GetProcAddress((HMODULE)*I, symbolName);
+    for (HMODULE Module : llvm::make_range(OpenedHandles->rbegin(),
+                                           OpenedHandles->rend())) {
+      FARPROC ptr = GetProcAddress(Module, symbolName);
       if (ptr) {
         return (void *)(intptr_t)ptr;
       }
@@ -174,7 +210,7 @@
 void *DynamicLibrary::getAddressOfSymbol(const char *symbolName) {
   if (!isValid())
     return NULL;
-  if (Data == &OpenedHandles)
+  if (Data == OpenedHandles.get())
     return SearchForAddressOfSymbol(symbolName);
   return (void *)(intptr_t)GetProcAddress((HMODULE)Data, symbolName);
 }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to