diff --git a/src/coreclr/tools/superpmi/superpmi-shared/logging.cpp b/src/coreclr/tools/superpmi/superpmi-shared/logging.cpp index 63b2c5ee4a1121..a0de0cda60d122 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/logging.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shared/logging.cpp @@ -53,7 +53,7 @@ void Logger::Shutdown() // Opens a log file at the given path and enables file-based logging, if the given path is valid. // /* static */ -void Logger::OpenLogFile(char* logFilePath) +void Logger::OpenLogFile(const char* logFilePath) { if (s_logFile == INVALID_HANDLE_VALUE && logFilePath != nullptr) { diff --git a/src/coreclr/tools/superpmi/superpmi-shared/logging.h b/src/coreclr/tools/superpmi/superpmi-shared/logging.h index f7af0132630284..83559df9befdbf 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/logging.h +++ b/src/coreclr/tools/superpmi/superpmi-shared/logging.h @@ -73,7 +73,7 @@ class Logger static void Initialize(); static void Shutdown(); - static void OpenLogFile(char* logFilePath); + static void OpenLogFile(const char* logFilePath); static void CloseLogFile(); static UINT32 ParseLogLevelString(const char* specifierStr); diff --git a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp index 2b17d0ac8352ad..20f5f1bf295b33 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp @@ -9,6 +9,8 @@ #include "logging.h" #include "spmiutil.h" +#include +#include #include #include @@ -99,86 +101,103 @@ WCHAR* GetEnvironmentVariableWithDefaultW(const WCHAR* envVarName, const WCHAR* return retString; } -#ifdef TARGET_UNIX -// For some reason, the PAL doesn't have GetCommandLineA(). So write it. -LPSTR GetCommandLineA() +const char* GetEnvWithDefault(const char* envVarName, const char* defaultValue) { - LPSTR pCmdLine = nullptr; - LPWSTR pwCmdLine = GetCommandLineW(); + // getenv isn't thread safe, but it's simple and sufficient since we are development-only tool + char* env = getenv(envVarName); + return env ? env : defaultValue; +} - if (pwCmdLine != nullptr) +std::string GetProcessCommandLine() +{ +#ifdef TARGET_WINDOWS + return ::GetCommandLineA(); +#else + FILE* fp = fopen("/proc/self/cmdline", "r"); + if (fp != NULL) { - // Convert to ASCII + std::string result; + char* cmdLine = nullptr; + size_t size = 0; - int n = WideCharToMultiByte(CP_ACP, 0, pwCmdLine, -1, nullptr, 0, nullptr, nullptr); - if (n == 0) + while (getdelim(&cmdLine, &size, '\0', fp) != -1) { - LogError("MultiByteToWideChar failed %d", GetLastError()); - return nullptr; - } + // /proc/self/cmdline uses \0 as delimiter, convert it to space + if (!result.empty()) + result += ' '; - pCmdLine = new char[n]; + result += cmdLine; - int n2 = WideCharToMultiByte(CP_ACP, 0, pwCmdLine, -1, pCmdLine, n, nullptr, nullptr); - if ((n2 == 0) || (n2 != n)) - { - LogError("MultiByteToWideChar failed %d", GetLastError()); - return nullptr; + free(cmdLine); + cmdLine = nullptr; + size = 0; } + + fclose(fp); + return result; } - return pCmdLine; + return ""; +#endif } -#endif // TARGET_UNIX -bool LoadRealJitLib(HMODULE& jitLib, WCHAR* jitLibPath) +bool LoadRealJitLib(HMODULE& jitLib, const std::string& jitLibPath) { // Load Library if (jitLib == NULL) { - if (jitLibPath == nullptr) + if (jitLibPath.empty()) { LogError("LoadRealJitLib - No real jit path"); return false; } - jitLib = ::LoadLibraryExW(jitLibPath, NULL, 0); +#ifdef TARGET_WINDOWS + jitLib = ::LoadLibraryExA(jitLibPath.c_str(), NULL, 0); if (jitLib == NULL) { - LogError("LoadRealJitLib - LoadLibrary failed to load '%ws' (0x%08x)", jitLibPath, ::GetLastError()); + LogError("LoadRealJitLib - LoadLibrary failed to load '%s' (0x%08x)", jitLibPath.c_str(), ::GetLastError()); return false; } +#else + jitLib = ::dlopen(jitLibPath.c_str(), RTLD_LAZY); + // The simulated DllMain of JIT doesn't do any meaningful initialization. Skip it. + if (jitLib == NULL) + { + LogError("LoadRealJitLib - dlopen failed to load '%s' (%s)", jitLibPath.c_str(), ::dlerror()); + return false; + } +#endif } return true; } -void ReplaceIllegalCharacters(WCHAR* fileName) +void ReplaceIllegalCharacters(std::string& fileName) { - WCHAR* quote = nullptr; - // Perform the following transforms: // - Convert non-ASCII to ASCII for simplicity // - Remove any illegal or annoying characters from the file name by // converting them to underscores. // - Replace any quotes in the file name with spaces. - for (quote = fileName; *quote != '\0'; quote++) + + for (char& quote : fileName) { - WCHAR ch = *quote; + char ch = quote; if ((ch <= 32) || (ch >= 127)) // Only allow textual ASCII characters { - *quote = W('_'); + quote = '_'; } else { switch (ch) { - case W('('): case W(')'): case W('='): case W('<'): - case W('>'): case W(':'): case W('/'): case W('\\'): - case W('|'): case W('?'): case W('!'): case W('*'): - case W('.'): case W(','): - *quote = W('_'); + case '(': case ')': case '=': case '<': + case '>': case ':': case '/': case '\\': + case '|': case '?': case '!': case '*': + case '.': case ',': + quote = '_'; break; - case W('"'): - *quote = W(' '); + case '"': + quote = ' '; break; default: break; @@ -187,67 +206,34 @@ void ReplaceIllegalCharacters(WCHAR* fileName) } } -// All lengths in this function exclude the terminal NULL. -WCHAR* GetResultFileName(const WCHAR* folderPath, const WCHAR* fileName, const WCHAR* extension) +std::string GetResultFileName(const std::string& folderPath, + const std::string& fileName, + const std::string& extension) { - const size_t extensionLength = u16_strlen(extension); - const size_t fileNameLength = u16_strlen(fileName); - const size_t randomStringLength = 8; - const size_t maxPathLength = MAX_PATH - 50; - - // See how long the folder part is, and start building the file path with the folder part. + // Append a random string to improve uniqueness. // - WCHAR* fullPath = new WCHAR[MAX_PATH]; - fullPath[0] = W('\0'); - const size_t folderPathLength = GetFullPathNameW(folderPath, MAX_PATH, (LPWSTR)fullPath, NULL); + uint32_t randomNumber = 0; + minipal_get_non_cryptographically_secure_random_bytes((uint8_t*)&randomNumber, sizeof(randomNumber)); + std::stringstream ss; + ss << std::hex << std::setw(8) << std::setfill('0') << randomNumber; + std::string suffix = ss.str() + extension; - if (folderPathLength == 0) + // Limit the total file name length to MAX_PATH - 50 + int usableLength = MAX_PATH - 50 - (int)folderPath.size() - (int)suffix.size(); + if (usableLength < 0) { - LogError("GetResultFileName - can't resolve folder path '%ws'", folderPath); - return nullptr; + LogError("GetResultFileName - folder path '%s' length + minimal file name exceeds limit %d", folderPath.c_str(), MAX_PATH - 50); + return ""; } - // Account for the folder, directory separator and extension. - // - size_t fullPathLength = folderPathLength + 1 + extensionLength; - - // If we won't have room for a minimal file name part, bail. - // - if ((fullPathLength + randomStringLength) > maxPathLength) + std::string copy = fileName; + if ((int)copy.size() > usableLength) { - LogError("GetResultFileName - folder path '%ws' length + minimal file name exceeds limit %d", fullPath, maxPathLength); - return nullptr; + copy = copy.substr(0, usableLength); } - // Now figure out the file name part. - // - const size_t maxFileNameLength = maxPathLength - fullPathLength; - size_t usableFileNameLength = min(fileNameLength, maxFileNameLength - randomStringLength); - fullPathLength += usableFileNameLength + randomStringLength; - - // Append the file name part - // - wcsncat_s(fullPath, fullPathLength + 1, DIRECTORY_SEPARATOR_STR_W, 1); - wcsncat_s(fullPath, fullPathLength + 1, fileName, usableFileNameLength); - - // Clean up anything in the file part that can't be in a file name. - // - ReplaceIllegalCharacters(fullPath + folderPathLength + 1); - - // Append a random string to improve uniqueness. - // - unsigned int randomNumber = 0; - minipal_get_non_cryptographically_secure_random_bytes((uint8_t*)&randomNumber, sizeof(randomNumber)); - - WCHAR randomString[randomStringLength + 1]; - FormatInteger(randomString, randomStringLength + 1, "%08X", randomNumber); - wcsncat_s(fullPath, fullPathLength + 1, randomString, randomStringLength); - - // Append extension - // - wcsncat_s(fullPath, fullPathLength + 1, extension, extensionLength); - - return fullPath; + ReplaceIllegalCharacters(copy); + return folderPath + DIRECTORY_SEPARATOR_CHAR_A + copy + suffix; } #ifdef TARGET_AMD64 diff --git a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.h b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.h index 72e3b2b0f93a50..93e6547d1cf1b5 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.h +++ b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.h @@ -8,6 +8,12 @@ #define _SPMIUtil #include "methodcontext.h" +#ifdef TARGET_WINDOWS +#define GET_PROC_ADDRESS ::GetProcAddress +#else +#include +#define GET_PROC_ADDRESS ::dlsym +#endif bool BreakOnDebugBreakorAV(); void SetBreakOnDebugBreakOrAV(bool value); @@ -21,13 +27,15 @@ char* GetEnvironmentVariableWithDefaultA(const char* envVarName, const char* def WCHAR* GetEnvironmentVariableWithDefaultW(const WCHAR* envVarName, const WCHAR* defaultValue = nullptr); -#ifdef TARGET_UNIX -LPSTR GetCommandLineA(); -#endif // TARGET_UNIX +const char* GetEnvWithDefault(const char* envVarName, const char* defaultValue = nullptr); -bool LoadRealJitLib(HMODULE& realJit, WCHAR* realJitPath); +std::string GetProcessCommandLine(); -WCHAR* GetResultFileName(const WCHAR* folderPath, const WCHAR* fileName, const WCHAR* extension); +bool LoadRealJitLib(HMODULE& realJit, const std::string& realJitPath); + +std::string GetResultFileName(const std::string& folderPath, + const std::string& fileName, + const std::string& extension); // SuperPMI stores handles as unsigned 64-bit integers, no matter the platform the collection happens on // (32 or 64 bit). Handles are defined as pointers. We need to be careful when converting from a handle diff --git a/src/coreclr/tools/superpmi/superpmi-shim-collector/icorjitcompiler.cpp b/src/coreclr/tools/superpmi/superpmi-shim-collector/icorjitcompiler.cpp index b6209f97a325f7..d7f15ea6cd1fc3 100644 --- a/src/coreclr/tools/superpmi/superpmi-shim-collector/icorjitcompiler.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shim-collector/icorjitcompiler.cpp @@ -68,7 +68,7 @@ CorJitResult interceptor_ICJC::compileMethod(ICorJitInfo* comp, auto* mc = new MethodContext(); interceptor_ICJI our_ICorJitInfo(this, comp, mc); - mc->cr->recProcessName(GetCommandLineA()); + mc->cr->recProcessName(GetProcessCommandLine().c_str()); mc->recCompileMethod(info, flags, currentOs); diff --git a/src/coreclr/tools/superpmi/superpmi-shim-collector/superpmi-shim-collector.cpp b/src/coreclr/tools/superpmi/superpmi-shim-collector/superpmi-shim-collector.cpp index a8f4046d83d660..f3ff4af0e6f9db 100644 --- a/src/coreclr/tools/superpmi/superpmi-shim-collector/superpmi-shim-collector.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shim-collector/superpmi-shim-collector.cpp @@ -17,68 +17,70 @@ // -We'll never be unloaded - we leak memory and have no facility to unload libraries // -printf output to console is okay -HMODULE g_hRealJit = 0; // We leak this currently (could do the proper shutdown in process_detach) -WCHAR* g_realJitPath = nullptr; // We leak this (could do the proper shutdown in process_detach) -WCHAR* g_logPath = nullptr; // Again, we leak this one too... -WCHAR* g_dataFileName = nullptr; // We leak this -char* g_logFilePath = nullptr; // We *don't* leak this, hooray! -WCHAR* g_HomeDirectory = nullptr; -WCHAR* g_DefaultRealJitPath = nullptr; -MethodContext* g_globalContext = nullptr; -bool g_initialized = false; -char* g_collectionFilter = nullptr; +HMODULE g_hRealJit = 0; // We leak this currently (could do the proper shutdown in process_detach) +std::string g_realJitPath{""}; // Destructable objects will be cleaned up and won't leak +std::string g_logPath{""}; +std::string g_HomeDirectory{""}; +std::string g_DefaultRealJitPath{""}; +std::string g_dataFileName{""}; +MethodContext* g_globalContext = nullptr; +bool g_initialized = false; +char* g_collectionFilter = nullptr; + +// RAII holder for logger +// Global deconstructors are unreliable. We only use it for superpmi shim. +class LoggerHolder +{ +public: + LoggerHolder() + { + Logger::Initialize(); + // If the environment variable isn't set, we don't enable file logging + const char* logFilePath = GetEnvWithDefault("SuperPMIShimLogFilePath", nullptr); + if (logFilePath) + { + Logger::OpenLogFile(logFilePath); + } + } + + ~LoggerHolder() + { + Logger::Shutdown(); + } +} loggerHolder; void SetDefaultPaths() { - if (g_HomeDirectory == nullptr) + if (g_HomeDirectory.empty()) { - g_HomeDirectory = GetEnvironmentVariableWithDefaultW(W("HOME"), W(".")); + g_HomeDirectory = GetEnvWithDefault("HOME", "."); } - if (g_DefaultRealJitPath == nullptr) + if (g_DefaultRealJitPath.empty()) { - size_t len = u16_strlen(g_HomeDirectory) + 1 + u16_strlen(DEFAULT_REAL_JIT_NAME_W) + 1; - g_DefaultRealJitPath = new WCHAR[len]; - wcscpy_s(g_DefaultRealJitPath, len, g_HomeDirectory); - wcscat_s(g_DefaultRealJitPath, len, DIRECTORY_SEPARATOR_STR_W); - wcscat_s(g_DefaultRealJitPath, len, DEFAULT_REAL_JIT_NAME_W); + g_DefaultRealJitPath = g_HomeDirectory + DIRECTORY_SEPARATOR_CHAR_A + DEFAULT_REAL_JIT_NAME_A; } } void SetLibName() { - if (g_realJitPath == nullptr) + if (g_realJitPath.empty()) { - g_realJitPath = GetEnvironmentVariableWithDefaultW(W("SuperPMIShimPath"), g_DefaultRealJitPath); + g_realJitPath = GetEnvWithDefault("SuperPMIShimPath", g_DefaultRealJitPath.c_str()); } } void SetLogPath() { - if (g_logPath == nullptr) + if (g_logPath.empty()) { - g_logPath = GetEnvironmentVariableWithDefaultW(W("SuperPMIShimLogPath"), g_HomeDirectory); + g_logPath = GetEnvWithDefault("SuperPMIShimLogPath", g_HomeDirectory.c_str()); } } void SetLogPathName() { - // NOTE: under PAL, we don't get the command line, so we depend on the random number generator to give us a unique - // filename - const WCHAR* fileName = GetCommandLineW(); - const WCHAR* extension = W(".mc"); - - g_dataFileName = GetResultFileName(g_logPath, fileName, extension); -} - -// TODO: this only works for ANSI file paths... -void SetLogFilePath() -{ - if (g_logFilePath == nullptr) - { - // If the environment variable isn't set, we don't enable file logging - g_logFilePath = GetEnvironmentVariableWithDefaultA("SuperPMIShimLogFilePath", nullptr); - } + g_dataFileName = GetResultFileName(g_logPath, GetProcessCommandLine(), ".mc"); } void SetCollectionFilter() @@ -114,10 +116,6 @@ void InitializeShim() _CrtSetReportFile(_CRT_ASSERT, _CRTDBG_FILE_STDERR); #endif // HOST_WINDOWS - Logger::Initialize(); - SetLogFilePath(); - Logger::OpenLogFile(g_logFilePath); - g_initialized = true; } @@ -135,13 +133,6 @@ extern "C" break; case DLL_PROCESS_DETACH: - Logger::Shutdown(); - - delete[] g_logFilePath; - g_logFilePath = nullptr; - - break; - case DLL_THREAD_ATTACH: case DLL_THREAD_DETACH: break; @@ -165,7 +156,7 @@ extern "C" DLLEXPORT void jitStartup(ICorJitHost* host) } // Get the required entrypoint - PjitStartup pnjitStartup = (PjitStartup)::GetProcAddress(g_hRealJit, "jitStartup"); + PjitStartup pnjitStartup = (PjitStartup)GET_PROC_ADDRESS(g_hRealJit, "jitStartup"); if (pnjitStartup == nullptr) { // This portion of the interface is not used by the JIT under test. @@ -196,7 +187,7 @@ extern "C" DLLEXPORT ICorJitCompiler* getJit() } // get the required entrypoints - pngetJit = (PgetJit)::GetProcAddress(g_hRealJit, "getJit"); + pngetJit = (PgetJit)GET_PROC_ADDRESS(g_hRealJit, "getJit"); if (pngetJit == 0) { LogError("getJit() - GetProcAddress 'getJit' failed (0x%08x)", ::GetLastError()); @@ -224,11 +215,11 @@ extern "C" DLLEXPORT ICorJitCompiler* getJit() #endif // create our datafile - pJitInstance->hFile = CreateFileW(g_dataFileName, GENERIC_READ | GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, - FILE_ATTRIBUTE_NORMAL | FILE_FLAG_SEQUENTIAL_SCAN, NULL); + pJitInstance->hFile = CreateFileA(g_dataFileName.c_str(), GENERIC_READ | GENERIC_WRITE, 0, NULL, + CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL | FILE_FLAG_SEQUENTIAL_SCAN, NULL); if (pJitInstance->hFile == INVALID_HANDLE_VALUE) { - LogError("Couldn't open file '%ws': error %d", g_dataFileName, GetLastError()); + LogError("Couldn't open file '%s': error %d", g_dataFileName.c_str(), GetLastError()); } return pJitInstance; diff --git a/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.cpp b/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.cpp index 6928201fce4979..ce9f142119c9c9 100644 --- a/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.cpp @@ -6,101 +6,39 @@ #include "logging.h" #include "spmiutil.h" -MethodCallSummarizer::MethodCallSummarizer(WCHAR* logPath) +MethodCallSummarizer::MethodCallSummarizer(const std::string& logPath) { - numNames = 0; - names = nullptr; - counts = nullptr; - - const WCHAR* fileName = GetCommandLineW(); - const WCHAR* extension = W(".csv"); - - dataFileName = GetResultFileName(logPath, fileName, extension); + dataFileName = GetResultFileName(logPath, GetProcessCommandLine(), ".csv"); } -MethodCallSummarizer::~MethodCallSummarizer() +// Use ordered map to make the most commonly added items are at the top of the list... +// 60% landed in the first three slots in short runs +void MethodCallSummarizer::AddCall(const char* name) { - delete [] dataFileName; - delete [] counts; - for (int i = 0; i < numNames; i++) - { - delete [] names[i]; - } - delete [] names; + // std::map initialized integer values to zero + namesAndCounts[std::string(name)]++; } -// lots of ways will be faster.. this happens to be decently simple and good enough for the task at hand and nicely -// sorts the output. in this approach the most commonly added items are at the top of the list... 60% landed in the -// first -// three slots in short runs -void MethodCallSummarizer::AddCall(const char* name) +void MethodCallSummarizer::SaveTextFile() { - // if we can find it already in our list, increment the count - for (int i = 0; i < numNames; i++) - { - if (strcmp(name, names[i]) == 0) - { - counts[i]++; - for (i = 1; i < numNames; i++) - if (counts[i] > counts[i - 1]) - { - unsigned int tempui = counts[i - 1]; - counts[i - 1] = counts[i]; - counts[i] = tempui; - char* tempc = names[i - 1]; - names[i - 1] = names[i]; - names[i] = tempc; - } - return; - } - } - - // else we didn't find it, so add it - char** tnames = names; - unsigned int* tcounts = counts; - - names = new char*[numNames + 1]; - if (tnames != nullptr) + FILE* fp = fopen(dataFileName.c_str(), "w"); + if (fp == NULL) { - memcpy(names, tnames, numNames * sizeof(char*)); - delete [] tnames; + LogError("Couldn't open file '%s': error %d", dataFileName.c_str(), errno); + return; } - size_t tlen = strlen(name); - names[numNames] = new char[tlen + 1]; - memcpy(names[numNames], name, tlen + 1); + fprintf(fp, "FunctionName,Count\n"); - counts = new unsigned int[numNames + 1]; - if (tcounts != nullptr) + for (auto& elem : namesAndCounts) { - memcpy(counts, tcounts, numNames * sizeof(unsigned int)); - delete [] tcounts; + fprintf(fp, "%s,%d\n", elem.first.c_str(), elem.second); } - counts[numNames] = 1; - numNames++; + fclose(fp); } -void MethodCallSummarizer::SaveTextFile() +MethodCallSummarizer::~MethodCallSummarizer() { - char buff[512]; - DWORD bytesWritten = 0; - HANDLE hFile = CreateFileW(dataFileName, GENERIC_READ | GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, - FILE_ATTRIBUTE_NORMAL | FILE_FLAG_SEQUENTIAL_SCAN, NULL); - - if (hFile == INVALID_HANDLE_VALUE) - { - LogError("Couldn't open file '%ws': error %d", dataFileName, ::GetLastError()); - return; - } - - DWORD len = (DWORD)sprintf_s(buff, 512, "FunctionName,Count\n"); - WriteFile(hFile, buff, len, &bytesWritten, NULL); - - for (int i = 0; i < numNames; i++) - { - len = sprintf_s(buff, 512, "%s,%u\n", names[i], counts[i]); - WriteFile(hFile, buff, len, &bytesWritten, NULL); - } - CloseHandle(hFile); + SaveTextFile(); } diff --git a/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.h b/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.h index 333e4a1e88b7a2..aca87d55e91dd8 100644 --- a/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.h +++ b/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.h @@ -4,19 +4,21 @@ #ifndef _MethodCallSummarizer #define _MethodCallSummarizer +#include +#include + class MethodCallSummarizer { public: - MethodCallSummarizer(WCHAR* name); - ~MethodCallSummarizer(); + MethodCallSummarizer(const std::string& name); void AddCall(const char* name); - void SaveTextFile(); + ~MethodCallSummarizer(); private: - char** names; - unsigned int* counts; - int numNames; - WCHAR* dataFileName; + std::map namesAndCounts; + std::string dataFileName; + + void SaveTextFile(); }; #endif diff --git a/src/coreclr/tools/superpmi/superpmi-shim-counter/superpmi-shim-counter.cpp b/src/coreclr/tools/superpmi/superpmi-shim-counter/superpmi-shim-counter.cpp index de3cac7cec4b6c..a56c6d1bd8c264 100644 --- a/src/coreclr/tools/superpmi/superpmi-shim-counter/superpmi-shim-counter.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shim-counter/superpmi-shim-counter.cpp @@ -13,55 +13,64 @@ #include "logging.h" #include "spmiutil.h" #include "jithost.h" +#include -HMODULE g_hRealJit = 0; // We leak this currently (could do the proper shutdown in process_detach) -WCHAR* g_realJitPath = nullptr; // We leak this (could do the proper shutdown in process_detach) -WCHAR* g_logPath = nullptr; // Again, we leak this one too... -char* g_logFilePath = nullptr; // We *don't* leak this, hooray! -WCHAR* g_HomeDirectory = nullptr; -WCHAR* g_DefaultRealJitPath = nullptr; -MethodCallSummarizer* g_globalContext = nullptr; +HMODULE g_hRealJit = 0; // We leak this currently (could do the proper shutdown in process_detach) +std::string g_realJitPath{""}; // Destructable objects will be cleaned up and won't leak +std::string g_logPath{""}; +std::string g_HomeDirectory{""}; +std::string g_DefaultRealJitPath{""}; -void SetDefaultPaths() +std::unique_ptr g_globalContext = nullptr; + +// RAII holder for logger +// Global deconstructors are unreliable. We only use it for superpmi shim. +class LoggerHolder { - if (g_HomeDirectory == nullptr) +public: + LoggerHolder() { - g_HomeDirectory = GetEnvironmentVariableWithDefaultW(W("HOME"), W(".")); + Logger::Initialize(); + // If the environment variable isn't set, we don't enable file logging + const char* logFilePath = GetEnvWithDefault("SuperPMIShimLogFilePath", nullptr); + if (logFilePath) + { + Logger::OpenLogFile(logFilePath); + } } - if (g_DefaultRealJitPath == nullptr) + ~LoggerHolder() { - size_t len = u16_strlen(g_HomeDirectory) + 1 + u16_strlen(DEFAULT_REAL_JIT_NAME_W) + 1; - g_DefaultRealJitPath = new WCHAR[len]; - wcscpy_s(g_DefaultRealJitPath, len, g_HomeDirectory); - wcscat_s(g_DefaultRealJitPath, len, DIRECTORY_SEPARATOR_STR_W); - wcscat_s(g_DefaultRealJitPath, len, DEFAULT_REAL_JIT_NAME_W); + Logger::Shutdown(); } -} +} loggerHolder; -void SetLibName() +void SetDefaultPaths() { - if (g_realJitPath == nullptr) + if (g_HomeDirectory.empty()) { - g_realJitPath = GetEnvironmentVariableWithDefaultW(W("SuperPMIShimPath"), g_DefaultRealJitPath); + g_HomeDirectory = GetEnvWithDefault("HOME", "."); + } + + if (g_DefaultRealJitPath.empty()) + { + g_DefaultRealJitPath = g_HomeDirectory + DIRECTORY_SEPARATOR_CHAR_A + DEFAULT_REAL_JIT_NAME_A; } } -void SetLogPath() +void SetLibName() { - if (g_logPath == nullptr) + if (g_realJitPath.empty()) { - g_logPath = GetEnvironmentVariableWithDefaultW(W("SuperPMIShimLogPath"), g_HomeDirectory); + g_realJitPath = GetEnvWithDefault("SuperPMIShimPath", g_DefaultRealJitPath.c_str()); } } -// TODO: this only works for ANSI file paths... -void SetLogFilePath() +void SetLogPath() { - if (g_logFilePath == nullptr) + if (g_logPath.empty()) { - // If the environment variable isn't set, we don't enable file logging - g_logFilePath = GetEnvironmentVariableWithDefaultA("SuperPMIShimLogFilePath", nullptr); + g_logPath = GetEnvWithDefault("SuperPMIShimLogPath", g_HomeDirectory.c_str()); } } @@ -82,25 +91,9 @@ extern "C" exit(1); } #endif // HOST_UNIX - - Logger::Initialize(); - SetLogFilePath(); - Logger::OpenLogFile(g_logFilePath); break; case DLL_PROCESS_DETACH: - Logger::Shutdown(); - - delete[] g_logFilePath; - g_logFilePath = nullptr; - - if (g_globalContext != nullptr) - { - g_globalContext->SaveTextFile(); - } - - break; - case DLL_THREAD_ATTACH: case DLL_THREAD_DETACH: break; @@ -120,7 +113,7 @@ extern "C" DLLEXPORT void jitStartup(ICorJitHost* host) } // Get the required entrypoint - PjitStartup pnjitStartup = (PjitStartup)::GetProcAddress(g_hRealJit, "jitStartup"); + PjitStartup pnjitStartup = (PjitStartup)GET_PROC_ADDRESS(g_hRealJit, "jitStartup"); if (pnjitStartup == nullptr) { // This portion of the interface is not used by the JIT under test. @@ -133,10 +126,10 @@ extern "C" DLLEXPORT void jitStartup(ICorJitHost* host) if (g_globalContext == nullptr) { SetLogPath(); - g_globalContext = new MethodCallSummarizer(g_logPath); + g_globalContext = std::unique_ptr(new MethodCallSummarizer(g_logPath)); } - g_ourJitHost->setMethodCallSummarizer(g_globalContext); + g_ourJitHost->setMethodCallSummarizer(g_globalContext.get()); pnjitStartup(g_ourJitHost); } @@ -158,7 +151,7 @@ extern "C" DLLEXPORT ICorJitCompiler* getJit() } // get the required entrypoints - pngetJit = (PgetJit)::GetProcAddress(g_hRealJit, "getJit"); + pngetJit = (PgetJit)GET_PROC_ADDRESS(g_hRealJit, "getJit"); if (pngetJit == 0) { LogError("getJit() - GetProcAddress 'getJit' failed (0x%08x)", ::GetLastError()); @@ -178,8 +171,8 @@ extern "C" DLLEXPORT ICorJitCompiler* getJit() if (g_globalContext == nullptr) { SetLogPath(); - g_globalContext = new MethodCallSummarizer(g_logPath); + g_globalContext = std::unique_ptr(new MethodCallSummarizer(g_logPath)); } - pJitInstance->mcs = g_globalContext; + pJitInstance->mcs = g_globalContext.get(); return pJitInstance; } diff --git a/src/coreclr/tools/superpmi/superpmi-shim-simple/superpmi-shim-simple.cpp b/src/coreclr/tools/superpmi/superpmi-shim-simple/superpmi-shim-simple.cpp index 27fe53c8b4782f..2cc1ef2a268170 100644 --- a/src/coreclr/tools/superpmi/superpmi-shim-simple/superpmi-shim-simple.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shim-simple/superpmi-shim-simple.cpp @@ -14,44 +14,53 @@ #include "spmiutil.h" #include "jithost.h" -HMODULE g_hRealJit = 0; // We leak this currently (could do the proper shutdown in process_detach) -WCHAR* g_realJitPath = nullptr; // We leak this (could do the proper shutdown in process_detach) -char* g_logFilePath = nullptr; // We *don't* leak this, hooray! -WCHAR* g_HomeDirectory = nullptr; -WCHAR* g_DefaultRealJitPath = nullptr; -void SetDefaultPaths() +HMODULE g_hRealJit = 0; // We leak this currently (could do the proper shutdown in process_detach) +std::string g_realJitPath{""}; // Destructable objects will be cleaned up and won't leak +std::string g_logPath{""}; +std::string g_HomeDirectory{""}; +std::string g_DefaultRealJitPath{""}; + +// RAII holder for logger +// Global deconstructors are unreliable. We only use it for superpmi shim. +class LoggerHolder { - if (g_HomeDirectory == nullptr) +public: + LoggerHolder() { - g_HomeDirectory = GetEnvironmentVariableWithDefaultW(W("HOME"), W(".")); + Logger::Initialize(); + // If the environment variable isn't set, we don't enable file logging + const char* logFilePath = GetEnvWithDefault("SuperPMIShimLogFilePath", nullptr); + if (logFilePath) + { + Logger::OpenLogFile(logFilePath); + } } - if (g_DefaultRealJitPath == nullptr) + ~LoggerHolder() { - size_t len = u16_strlen(g_HomeDirectory) + 1 + u16_strlen(DEFAULT_REAL_JIT_NAME_W) + 1; - g_DefaultRealJitPath = new WCHAR[len]; - wcscpy_s(g_DefaultRealJitPath, len, g_HomeDirectory); - wcscat_s(g_DefaultRealJitPath, len, DIRECTORY_SEPARATOR_STR_W); - wcscat_s(g_DefaultRealJitPath, len, DEFAULT_REAL_JIT_NAME_W); + Logger::Shutdown(); } -} +} loggerHolder; -void SetLibName() +void SetDefaultPaths() { - if (g_realJitPath == nullptr) + if (g_HomeDirectory.empty()) + { + g_HomeDirectory = GetEnvWithDefault("HOME", "."); + } + + if (g_DefaultRealJitPath.empty()) { - g_realJitPath = GetEnvironmentVariableWithDefaultW(W("SuperPMIShimPath"), g_DefaultRealJitPath); + g_DefaultRealJitPath = g_HomeDirectory + DIRECTORY_SEPARATOR_CHAR_A + DEFAULT_REAL_JIT_NAME_A; } } -// TODO: this only works for ANSI file paths... -void SetLogFilePath() +void SetLibName() { - if (g_logFilePath == nullptr) + if (g_realJitPath.empty()) { - // If the environment variable isn't set, we don't enable file logging - g_logFilePath = GetEnvironmentVariableWithDefaultA("SuperPMIShimLogFilePath", nullptr); + g_realJitPath = GetEnvWithDefault("SuperPMIShimPath", g_DefaultRealJitPath.c_str()); } } @@ -72,20 +81,9 @@ extern "C" exit(1); } #endif // HOST_UNIX - - Logger::Initialize(); - SetLogFilePath(); - Logger::OpenLogFile(g_logFilePath); break; case DLL_PROCESS_DETACH: - Logger::Shutdown(); - - delete[] g_logFilePath; - g_logFilePath = nullptr; - - break; - case DLL_THREAD_ATTACH: case DLL_THREAD_DETACH: break; @@ -105,7 +103,7 @@ extern "C" DLLEXPORT void jitStartup(ICorJitHost* host) } // Get the required entrypoint - PjitStartup pnjitStartup = (PjitStartup)::GetProcAddress(g_hRealJit, "jitStartup"); + PjitStartup pnjitStartup = (PjitStartup)GET_PROC_ADDRESS(g_hRealJit, "jitStartup"); if (pnjitStartup == nullptr) { // This portion of the interface is not used by the JIT under test. @@ -134,7 +132,7 @@ extern "C" DLLEXPORT ICorJitCompiler* getJit() } // get the required entrypoints - pngetJit = (PgetJit)::GetProcAddress(g_hRealJit, "getJit"); + pngetJit = (PgetJit)GET_PROC_ADDRESS(g_hRealJit, "getJit"); if (pngetJit == 0) { LogError("getJit() - GetProcAddress 'getJit' failed (0x%08x)", ::GetLastError());