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