From 688e2b031a0a0d85cc6c7405922212b20fd19d30 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Thu, 22 May 2025 18:53:18 +0800 Subject: [PATCH 01/28] Use STL for MethodCallSummarizer --- .../methodcallsummarizer.cpp | 99 ++++--------------- .../methodcallsummarizer.h | 10 +- 2 files changed, 23 insertions(+), 86 deletions(-) diff --git a/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.cpp b/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.cpp index 6928201fce4979..3866d965efa97d 100644 --- a/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.cpp @@ -5,102 +5,39 @@ #include "methodcallsummarizer.h" #include "logging.h" #include "spmiutil.h" +#include +#include MethodCallSummarizer::MethodCallSummarizer(WCHAR* logPath) { - numNames = 0; - names = nullptr; - counts = nullptr; - - const WCHAR* fileName = GetCommandLineW(); + std::wstring fileName(GetCommandLineW()); const WCHAR* extension = W(".csv"); - dataFileName = GetResultFileName(logPath, fileName, extension); + dataFileName = GetResultFileName(logPath, fileName.c_str(), extension); } -MethodCallSummarizer::~MethodCallSummarizer() -{ - delete [] dataFileName; - delete [] counts; - for (int i = 0; i < numNames; i++) - { - delete [] names[i]; - } - delete [] names; -} - -// 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 +// 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) { - // 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) - { - memcpy(names, tnames, numNames * sizeof(char*)); - delete [] tnames; - } - - size_t tlen = strlen(name); - names[numNames] = new char[tlen + 1]; - memcpy(names[numNames], name, tlen + 1); - - counts = new unsigned int[numNames + 1]; - if (tcounts != nullptr) - { - memcpy(counts, tcounts, numNames * sizeof(unsigned int)); - delete [] tcounts; - } - counts[numNames] = 1; - - numNames++; + // std::map initialized integer values to zero + namesAndCounts[std::string(name)]++; } void MethodCallSummarizer::SaveTextFile() { - 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) + try { - 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); + std::ofstream outFile(dataFileName); + outFile << "FunctionName,Count" << std::endl; - for (int i = 0; i < numNames; i++) + for (auto& elem : namesAndCounts) + { + outFile << elem.first << "," << elem.second << std::endl; + } + } + catch (std::exception& ex) { - len = sprintf_s(buff, 512, "%s,%u\n", names[i], counts[i]); - WriteFile(hFile, buff, len, &bytesWritten, NULL); + LogError("Couldn't write file '%ws': %s", dataFileName.c_str(), ex.what()); } - CloseHandle(hFile); } diff --git a/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.h b/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.h index 333e4a1e88b7a2..c98eab03202568 100644 --- a/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.h +++ b/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.h @@ -4,19 +4,19 @@ #ifndef _MethodCallSummarizer #define _MethodCallSummarizer +#include +#include + class MethodCallSummarizer { public: MethodCallSummarizer(WCHAR* name); - ~MethodCallSummarizer(); void AddCall(const char* name); void SaveTextFile(); private: - char** names; - unsigned int* counts; - int numNames; - WCHAR* dataFileName; + std::map namesAndCounts; + std::wstring dataFileName; }; #endif From 6df1501bfa1b95f06fcaee62d14203987c7a1934 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Sun, 8 Jun 2025 16:24:06 +0800 Subject: [PATCH 02/28] Use std::string for manipulating file name --- .../superpmi/superpmi-shared/spmiutil.cpp | 52 +++++++++++++++++++ .../tools/superpmi/superpmi-shared/spmiutil.h | 2 + .../methodcallsummarizer.cpp | 19 ++++--- .../methodcallsummarizer.h | 2 +- 4 files changed, 68 insertions(+), 7 deletions(-) diff --git a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp index 9bba21b1bfd8e5..329923000e13a8 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 @@ -187,6 +189,41 @@ void ReplaceIllegalCharacters(WCHAR* fileName) } } +void ReplaceIllegalCharacters(std::string& fileName) +{ + // 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 (char& quote : fileName) + { + char ch = quote; + if ((ch <= 32) || (ch >= 127)) // Only allow textual ASCII characters + { + quote = '_'; + } + else + { + switch (ch) + { + case '(': case ')': case '=': case '<': + case '>': case ':': case '/': case '\\': + case '|': case '?': case '!': case '*': + case '.': case ',': + quote = '_'; + break; + case '"': + quote = ' '; + break; + default: + break; + } + } + } +} + // All lengths in this function exclude the terminal NULL. WCHAR* GetResultFileName(const WCHAR* folderPath, const WCHAR* fileName, const WCHAR* extension) { @@ -250,6 +287,21 @@ WCHAR* GetResultFileName(const WCHAR* folderPath, const WCHAR* fileName, const W return fullPath; } +std::string GetResultFileName(const std::string& folderPath, const std::string& fileName, const std::string& extension) +{ + // Append a random string to improve uniqueness. + // + uint32_t randomNumber = 0; + minipal_get_non_cryptographically_secure_random_bytes((uint8_t*)&randomNumber, sizeof(randomNumber)); + + std::stringstream ss; + ss << folderPath << DIRECTORY_SEPARATOR_STR_A << fileName << std::hex << std::setw(8) << std::setfill('0') << randomNumber << extension; + + std::string result = ss.str(); + ReplaceIllegalCharacters(result); + return result; +} + #ifdef TARGET_AMD64 static SPMI_TARGET_ARCHITECTURE SpmiTargetArchitecture = SPMI_TARGET_ARCHITECTURE_AMD64; #elif defined(TARGET_X86) diff --git a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.h b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.h index 9ff49500321217..e0971202d6c945 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.h +++ b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.h @@ -8,6 +8,7 @@ #define _SPMIUtil #include "methodcontext.h" +#include bool BreakOnDebugBreakorAV(); void SetBreakOnDebugBreakOrAV(bool value); @@ -28,6 +29,7 @@ LPSTR GetCommandLineA(); bool LoadRealJitLib(HMODULE& realJit, WCHAR* realJitPath); 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); // 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-counter/methodcallsummarizer.cpp b/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.cpp index 3866d965efa97d..5610ef9a8524dd 100644 --- a/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.cpp @@ -5,15 +5,22 @@ #include "methodcallsummarizer.h" #include "logging.h" #include "spmiutil.h" -#include #include MethodCallSummarizer::MethodCallSummarizer(WCHAR* logPath) { - std::wstring fileName(GetCommandLineW()); - const WCHAR* extension = W(".csv"); - - dataFileName = GetResultFileName(logPath, fileName.c_str(), extension); +#ifdef HOST_WINDOWS + std::string fileName(GetCommandLineA()); +#else + std::string fileName(""); + std::ifstream proc("/proc/self/cmdline"); + if (proc) + { + std::getline(proc, fileName); + } +#endif + const std::string extension = ".csv"; + dataFileName = GetResultFileName(ConvertToUtf8(logPath), fileName, extension); } // Use ordered map to make the most commonly added items are at the top of the list... @@ -38,6 +45,6 @@ void MethodCallSummarizer::SaveTextFile() } catch (std::exception& ex) { - LogError("Couldn't write file '%ws': %s", dataFileName.c_str(), ex.what()); + LogError("Couldn't write file '%s': %s", dataFileName.c_str(), ex.what()); } } diff --git a/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.h b/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.h index c98eab03202568..4a71d18bb28bdc 100644 --- a/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.h +++ b/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.h @@ -16,7 +16,7 @@ class MethodCallSummarizer private: std::map namesAndCounts; - std::wstring dataFileName; + std::string dataFileName; }; #endif From 0ba0e96d76f839d75c2cd77fc58ea572d2f8f5bd Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Sun, 8 Jun 2025 17:29:09 +0800 Subject: [PATCH 03/28] Use std::string for global strings --- .../superpmi/superpmi-shared/spmiutil.cpp | 7 +++ .../tools/superpmi/superpmi-shared/spmiutil.h | 2 + .../methodcallsummarizer.cpp | 4 +- .../methodcallsummarizer.h | 2 +- .../superpmi-shim-counter.cpp | 44 ++++++++----------- 5 files changed, 30 insertions(+), 29 deletions(-) diff --git a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp index 329923000e13a8..9221c736b11c17 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp @@ -101,6 +101,13 @@ WCHAR* GetEnvironmentVariableWithDefaultW(const WCHAR* envVarName, const WCHAR* return retString; } +std::string GetEnvWithDefault(const std::string& envVarName, const std::string& defaultValue = "") +{ + // getenv isn't thread safe, but it's simple and sufficient since we are development-only tool + char* env = getenv(envVarName.c_str()); + return env ? env : defaultValue; +} + #ifdef TARGET_UNIX // For some reason, the PAL doesn't have GetCommandLineA(). So write it. LPSTR GetCommandLineA() diff --git a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.h b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.h index e0971202d6c945..b656cb6904c966 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.h +++ b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.h @@ -22,6 +22,8 @@ char* GetEnvironmentVariableWithDefaultA(const char* envVarName, const char* def WCHAR* GetEnvironmentVariableWithDefaultW(const WCHAR* envVarName, const WCHAR* defaultValue = nullptr); +std::string GetEnvWithDefault(const std::string& envVarName, const std::string& defaultValue = ""); + #ifdef TARGET_UNIX LPSTR GetCommandLineA(); #endif // TARGET_UNIX diff --git a/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.cpp b/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.cpp index 5610ef9a8524dd..5c25f72fb3a2f0 100644 --- a/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.cpp @@ -7,7 +7,7 @@ #include "spmiutil.h" #include -MethodCallSummarizer::MethodCallSummarizer(WCHAR* logPath) +MethodCallSummarizer::MethodCallSummarizer(const std::string& logPath) { #ifdef HOST_WINDOWS std::string fileName(GetCommandLineA()); @@ -20,7 +20,7 @@ MethodCallSummarizer::MethodCallSummarizer(WCHAR* logPath) } #endif const std::string extension = ".csv"; - dataFileName = GetResultFileName(ConvertToUtf8(logPath), fileName, extension); + dataFileName = GetResultFileName(logPath, fileName, extension); } // Use ordered map to make the most commonly added items are at the top of the list... diff --git a/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.h b/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.h index 4a71d18bb28bdc..befc5c4bfeb70e 100644 --- a/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.h +++ b/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.h @@ -10,7 +10,7 @@ class MethodCallSummarizer { public: - MethodCallSummarizer(WCHAR* name); + MethodCallSummarizer(const std::string& name); void AddCall(const char* name); void SaveTextFile(); 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..c7ea918f834330 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 @@ -15,53 +15,48 @@ #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) -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; +std::string g_realJitPath = ""; // std::string will be cleaned up and won't leak +std::string g_logPath = ""; +std::string g_logFilePath = ""; +std::string g_HomeDirectory = ""; +std::string g_DefaultRealJitPath = ""; MethodCallSummarizer* g_globalContext = nullptr; 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_STR_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); } } void SetLogPath() { - if (g_logPath == nullptr) + if (g_logPath.empty()) { - g_logPath = GetEnvironmentVariableWithDefaultW(W("SuperPMIShimLogPath"), g_HomeDirectory); + g_logPath = GetEnvWithDefault("SuperPMIShimLogPath", g_HomeDirectory); } } -// TODO: this only works for ANSI file paths... void SetLogFilePath() { - if (g_logFilePath == nullptr) + if (g_logFilePath.empty()) { // If the environment variable isn't set, we don't enable file logging - g_logFilePath = GetEnvironmentVariableWithDefaultA("SuperPMIShimLogFilePath", nullptr); + g_logFilePath = GetEnvWithDefault("SuperPMIShimLogFilePath", nullptr); } } @@ -85,15 +80,12 @@ extern "C" Logger::Initialize(); SetLogFilePath(); - Logger::OpenLogFile(g_logFilePath); + Logger::OpenLogFile(g_logFilePath.c_str()); break; case DLL_PROCESS_DETACH: Logger::Shutdown(); - delete[] g_logFilePath; - g_logFilePath = nullptr; - if (g_globalContext != nullptr) { g_globalContext->SaveTextFile(); @@ -114,7 +106,7 @@ extern "C" DLLEXPORT void jitStartup(ICorJitHost* host) SetLibName(); SetDebugDumpVariables(); - if (!LoadRealJitLib(g_hRealJit, g_realJitPath)) + if (!LoadRealJitLib(g_hRealJit, g_realJitPath.c_str())) { return; } @@ -152,7 +144,7 @@ extern "C" DLLEXPORT ICorJitCompiler* getJit() SetLibName(); SetDebugDumpVariables(); - if (!LoadRealJitLib(g_hRealJit, g_realJitPath)) + if (!LoadRealJitLib(g_hRealJit, g_realJitPath.c_str())) { return nullptr; } From ff5058e3baad14aa44ac0d7113f363e763001731 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Sun, 8 Jun 2025 17:36:17 +0800 Subject: [PATCH 04/28] Use unique_ptr for destruction --- .../methodcallsummarizer.cpp | 5 ++++ .../methodcallsummarizer.h | 1 + .../superpmi-shim-counter.cpp | 29 +++++++++---------- 3 files changed, 19 insertions(+), 16 deletions(-) diff --git a/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.cpp b/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.cpp index 5c25f72fb3a2f0..6ec8575a76ac97 100644 --- a/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.cpp @@ -48,3 +48,8 @@ void MethodCallSummarizer::SaveTextFile() LogError("Couldn't write file '%s': %s", dataFileName.c_str(), ex.what()); } } + +MethodCallSummarizer::~MethodCallSummarizer() +{ + SaveTextFile(); +} diff --git a/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.h b/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.h index befc5c4bfeb70e..3a2b3a67d04ec8 100644 --- a/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.h +++ b/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.h @@ -13,6 +13,7 @@ class MethodCallSummarizer MethodCallSummarizer(const std::string& name); void AddCall(const char* name); void SaveTextFile(); + ~MethodCallSummarizer(); private: std::map namesAndCounts; 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 c7ea918f834330..a41e4f4cb8c304 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,14 +13,16 @@ #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) -std::string g_realJitPath = ""; // std::string will be cleaned up and won't leak -std::string g_logPath = ""; -std::string g_logFilePath = ""; -std::string g_HomeDirectory = ""; -std::string g_DefaultRealJitPath = ""; -MethodCallSummarizer* g_globalContext = nullptr; +HMODULE g_hRealJit = 0; // We leak this currently (could do the proper shutdown in process_detach) +std::string g_realJitPath = ""; // std::string will be cleaned up and won't leak +std::string g_logPath = ""; +std::string g_logFilePath = ""; +std::string g_HomeDirectory = ""; +std::string g_DefaultRealJitPath = ""; + +std::unique_ptr g_globalContext = nullptr; void SetDefaultPaths() { @@ -86,11 +88,6 @@ extern "C" case DLL_PROCESS_DETACH: Logger::Shutdown(); - if (g_globalContext != nullptr) - { - g_globalContext->SaveTextFile(); - } - break; case DLL_THREAD_ATTACH: @@ -125,10 +122,10 @@ extern "C" DLLEXPORT void jitStartup(ICorJitHost* host) if (g_globalContext == nullptr) { SetLogPath(); - g_globalContext = new MethodCallSummarizer(g_logPath); + g_globalContext = std::make_unique(g_logPath); } - g_ourJitHost->setMethodCallSummarizer(g_globalContext); + g_ourJitHost->setMethodCallSummarizer(g_globalContext.get()); pnjitStartup(g_ourJitHost); } @@ -170,8 +167,8 @@ extern "C" DLLEXPORT ICorJitCompiler* getJit() if (g_globalContext == nullptr) { SetLogPath(); - g_globalContext = new MethodCallSummarizer(g_logPath); + g_globalContext = std::make_unique(g_logPath); } - pJitInstance->mcs = g_globalContext; + pJitInstance->mcs = g_globalContext.get(); return pJitInstance; } From 01f4be790cabbe24fed7c6b57913a4700113a281 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Sun, 8 Jun 2025 17:45:36 +0800 Subject: [PATCH 05/28] Use RAII for logger --- .../superpmi/superpmi-shared/logging.cpp | 2 +- .../tools/superpmi/superpmi-shared/logging.h | 2 +- .../superpmi/superpmi-shared/spmiutil.cpp | 4 +- .../tools/superpmi/superpmi-shared/spmiutil.h | 2 +- .../superpmi-shim-counter.cpp | 43 ++++++++++--------- 5 files changed, 28 insertions(+), 25 deletions(-) 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 9221c736b11c17..940dd672cdc4e3 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp @@ -101,10 +101,10 @@ WCHAR* GetEnvironmentVariableWithDefaultW(const WCHAR* envVarName, const WCHAR* return retString; } -std::string GetEnvWithDefault(const std::string& envVarName, const std::string& defaultValue = "") +const char* GetEnvWithDefault(const char* envVarName, const char* defaultValue) { // getenv isn't thread safe, but it's simple and sufficient since we are development-only tool - char* env = getenv(envVarName.c_str()); + char* env = getenv(envVarName); return env ? env : defaultValue; } diff --git a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.h b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.h index b656cb6904c966..be11a886a49c08 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.h +++ b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.h @@ -22,7 +22,7 @@ char* GetEnvironmentVariableWithDefaultA(const char* envVarName, const char* def WCHAR* GetEnvironmentVariableWithDefaultW(const WCHAR* envVarName, const WCHAR* defaultValue = nullptr); -std::string GetEnvWithDefault(const std::string& envVarName, const std::string& defaultValue = ""); +const char* GetEnvWithDefault(const char* envVarName, const char* defaultValue = nullptr); #ifdef TARGET_UNIX LPSTR GetCommandLineA(); 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 a41e4f4cb8c304..26fb81f71222d7 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 @@ -18,12 +18,32 @@ HMODULE g_hRealJit = 0; // We leak this currently (could do the proper shutdown in process_detach) std::string g_realJitPath = ""; // std::string will be cleaned up and won't leak std::string g_logPath = ""; -std::string g_logFilePath = ""; std::string g_HomeDirectory = ""; std::string g_DefaultRealJitPath = ""; std::unique_ptr g_globalContext = nullptr; +// RAII holder for logger +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.empty()) @@ -41,7 +61,7 @@ void SetLibName() { if (g_realJitPath.empty()) { - g_realJitPath = GetEnvWithDefault("SuperPMIShimPath", g_DefaultRealJitPath); + g_realJitPath = GetEnvWithDefault("SuperPMIShimPath", g_DefaultRealJitPath.c_str()); } } @@ -49,16 +69,7 @@ void SetLogPath() { if (g_logPath.empty()) { - g_logPath = GetEnvWithDefault("SuperPMIShimLogPath", g_HomeDirectory); - } -} - -void SetLogFilePath() -{ - if (g_logFilePath.empty()) - { - // If the environment variable isn't set, we don't enable file logging - g_logFilePath = GetEnvWithDefault("SuperPMIShimLogFilePath", nullptr); + g_logPath = GetEnvWithDefault("SuperPMIShimLogPath", g_HomeDirectory.c_str()); } } @@ -79,17 +90,9 @@ extern "C" exit(1); } #endif // HOST_UNIX - - Logger::Initialize(); - SetLogFilePath(); - Logger::OpenLogFile(g_logFilePath.c_str()); break; case DLL_PROCESS_DETACH: - Logger::Shutdown(); - - break; - case DLL_THREAD_ATTACH: case DLL_THREAD_DETACH: break; From 0a95bc99343049c52c4b11104f4331098da342be Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Sun, 8 Jun 2025 18:10:29 +0800 Subject: [PATCH 06/28] Load JIT with std::string --- .../superpmi/superpmi-shared/spmiutil.cpp | 27 +++++++++++++++++++ .../tools/superpmi/superpmi-shared/spmiutil.h | 1 + .../superpmi-shim-counter.cpp | 18 ++++++------- 3 files changed, 37 insertions(+), 9 deletions(-) diff --git a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp index 940dd672cdc4e3..9a1652028a60e1 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp @@ -13,6 +13,9 @@ #include #include #include +#ifdef TARGET_UNIX +#include +#endif static bool breakOnDebugBreakorAV = false; @@ -160,6 +163,30 @@ bool LoadRealJitLib(HMODULE& jitLib, WCHAR* jitLibPath) return true; } +bool LoadRealJitLib(HMODULE& jitLib, const std::string& jitLibPath) +{ + // Load Library + if (jitLib == NULL) + { + if (jitLibPath.empty()) + { + LogError("LoadRealJitLib - No real jit path"); + return false; + } +#ifdef TARGET_WINDOWS + jitLib = ::LoadLibraryExA(jitLibPath.c_str(), NULL, 0); +#else + jitLib = ::dlopen(jitLibPath.c_str(), RTLD_LAZY); +#endif + if (jitLib == NULL) + { + LogError("LoadRealJitLib - LoadLibrary failed to load '%s' (%d)", jitLibPath.c_str(), errno); + return false; + } + } + return true; +} + void ReplaceIllegalCharacters(WCHAR* fileName) { WCHAR* quote = nullptr; diff --git a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.h b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.h index be11a886a49c08..8653befb7b7df7 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.h +++ b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.h @@ -29,6 +29,7 @@ LPSTR GetCommandLineA(); #endif // TARGET_UNIX bool LoadRealJitLib(HMODULE& realJit, WCHAR* realJitPath); +bool LoadRealJitLib(HMODULE& realJit, const std::string& realJitPath); 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); 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 26fb81f71222d7..74a50b10d4dcc0 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 @@ -29,14 +29,14 @@ 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); + 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() { @@ -106,7 +106,7 @@ extern "C" DLLEXPORT void jitStartup(ICorJitHost* host) SetLibName(); SetDebugDumpVariables(); - if (!LoadRealJitLib(g_hRealJit, g_realJitPath.c_str())) + if (!LoadRealJitLib(g_hRealJit, g_realJitPath)) { return; } @@ -144,7 +144,7 @@ extern "C" DLLEXPORT ICorJitCompiler* getJit() SetLibName(); SetDebugDumpVariables(); - if (!LoadRealJitLib(g_hRealJit, g_realJitPath.c_str())) + if (!LoadRealJitLib(g_hRealJit, g_realJitPath)) { return nullptr; } From 84b95f34c03811db4cf9827141fd0cd9b4f0eb78 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Sun, 8 Jun 2025 18:18:45 +0800 Subject: [PATCH 07/28] Enable C++ 17 for superpmi --- src/coreclr/tools/superpmi/CMakeLists.txt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/coreclr/tools/superpmi/CMakeLists.txt b/src/coreclr/tools/superpmi/CMakeLists.txt index d4308d2e03cabd..e92d5e235e9ed8 100644 --- a/src/coreclr/tools/superpmi/CMakeLists.txt +++ b/src/coreclr/tools/superpmi/CMakeLists.txt @@ -1,3 +1,8 @@ +# SuperPMI is development-only toolset. +# Enable C++ 17 to get more handy stuffs from STL. +set(CMAKE_CXX_STANDARD 17) +set(CMAKE_CXX_STANDARD_REQUIRED ON) + add_subdirectory(superpmi) add_subdirectory(mcs) add_subdirectory(superpmi-shim-collector) From 9d8e705360e5637f04179d94e0ab2cb68700f44f Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Sun, 8 Jun 2025 19:56:20 +0800 Subject: [PATCH 08/28] Use std::filesystem for path manipulation --- .../superpmi/superpmi-shared/spmiutil.cpp | 20 ++++++++++--------- .../tools/superpmi/superpmi-shared/spmiutil.h | 7 ++++--- .../superpmi/superpmi-shared/standardpch.h | 3 +++ .../methodcallsummarizer.cpp | 2 +- .../methodcallsummarizer.h | 4 ++-- .../superpmi-shim-counter.cpp | 16 +++++++-------- 6 files changed, 29 insertions(+), 23 deletions(-) diff --git a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp index 9a1652028a60e1..00ad82dc6e6e9b 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp @@ -163,7 +163,7 @@ bool LoadRealJitLib(HMODULE& jitLib, WCHAR* jitLibPath) return true; } -bool LoadRealJitLib(HMODULE& jitLib, const std::string& jitLibPath) +bool LoadRealJitLib(HMODULE& jitLib, const std::filesystem::path& jitLibPath) { // Load Library if (jitLib == NULL) @@ -174,13 +174,14 @@ bool LoadRealJitLib(HMODULE& jitLib, const std::string& jitLibPath) return false; } #ifdef TARGET_WINDOWS - jitLib = ::LoadLibraryExA(jitLibPath.c_str(), NULL, 0); + jitLib = ::LoadLibraryExW(jitLibPath.c_str(), NULL, 0); #else jitLib = ::dlopen(jitLibPath.c_str(), RTLD_LAZY); + // The simulated DllMain of JIT doesn't do any meaningful initialization. Skip it. #endif if (jitLib == NULL) { - LogError("LoadRealJitLib - LoadLibrary failed to load '%s' (%d)", jitLibPath.c_str(), errno); + LogError("LoadRealJitLib - LoadLibrary failed to load '%s' (%d)", jitLibPath.string().c_str(), errno); return false; } } @@ -321,19 +322,20 @@ WCHAR* GetResultFileName(const WCHAR* folderPath, const WCHAR* fileName, const W return fullPath; } -std::string GetResultFileName(const std::string& folderPath, const std::string& fileName, const std::string& extension) +std::filesystem::path GetResultFileName(const std::filesystem::path& folderPath, + const std::string& fileName, + const std::string& extension) { // Append a random string to improve uniqueness. // uint32_t randomNumber = 0; minipal_get_non_cryptographically_secure_random_bytes((uint8_t*)&randomNumber, sizeof(randomNumber)); - std::stringstream ss; - ss << folderPath << DIRECTORY_SEPARATOR_STR_A << fileName << std::hex << std::setw(8) << std::setfill('0') << randomNumber << extension; + ss << std::hex << std::setw(8) << std::setfill('0') << randomNumber; - std::string result = ss.str(); - ReplaceIllegalCharacters(result); - return result; + std::string nameAndExtension = fileName + ss.str() + extension; + ReplaceIllegalCharacters(nameAndExtension); + return folderPath / nameAndExtension; } #ifdef TARGET_AMD64 diff --git a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.h b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.h index 8653befb7b7df7..8160ba913e0dbe 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.h +++ b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.h @@ -8,7 +8,6 @@ #define _SPMIUtil #include "methodcontext.h" -#include bool BreakOnDebugBreakorAV(); void SetBreakOnDebugBreakOrAV(bool value); @@ -29,10 +28,12 @@ LPSTR GetCommandLineA(); #endif // TARGET_UNIX bool LoadRealJitLib(HMODULE& realJit, WCHAR* realJitPath); -bool LoadRealJitLib(HMODULE& realJit, const std::string& realJitPath); +bool LoadRealJitLib(HMODULE& realJit, const std::filesystem::path& realJitPath); 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); +std::filesystem::path GetResultFileName(const std::filesystem::path& 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-shared/standardpch.h b/src/coreclr/tools/superpmi/superpmi-shared/standardpch.h index e18cb57a76a002..195230468b8f80 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/standardpch.h +++ b/src/coreclr/tools/superpmi/superpmi-shared/standardpch.h @@ -15,6 +15,9 @@ #define DEBUG_BREAK DebugBreak() #endif +// filesystem fails parsing if included after PAL header +#include + #ifndef WIN32_LEAN_AND_MEAN #define WIN32_LEAN_AND_MEAN #endif // WIN32_LEAN_AND_MEAN diff --git a/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.cpp b/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.cpp index 6ec8575a76ac97..8bedbcd672a684 100644 --- a/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.cpp @@ -7,7 +7,7 @@ #include "spmiutil.h" #include -MethodCallSummarizer::MethodCallSummarizer(const std::string& logPath) +MethodCallSummarizer::MethodCallSummarizer(const std::filesystem::path& logPath) { #ifdef HOST_WINDOWS std::string fileName(GetCommandLineA()); diff --git a/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.h b/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.h index 3a2b3a67d04ec8..df45654f594c59 100644 --- a/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.h +++ b/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.h @@ -10,14 +10,14 @@ class MethodCallSummarizer { public: - MethodCallSummarizer(const std::string& name); + MethodCallSummarizer(const std::filesystem::path& name); void AddCall(const char* name); void SaveTextFile(); ~MethodCallSummarizer(); private: std::map namesAndCounts; - std::string dataFileName; + std::filesystem::path dataFileName; }; #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 74a50b10d4dcc0..58abb428697064 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 @@ -15,11 +15,11 @@ #include "jithost.h" #include -HMODULE g_hRealJit = 0; // We leak this currently (could do the proper shutdown in process_detach) -std::string g_realJitPath = ""; // std::string will be cleaned up and won't leak -std::string g_logPath = ""; -std::string g_HomeDirectory = ""; -std::string g_DefaultRealJitPath = ""; +HMODULE g_hRealJit = 0; // We leak this currently (could do the proper shutdown in process_detach) +std::filesystem::path g_realJitPath{""}; // Destructable objects will be cleaned up and won't leak +std::filesystem::path g_logPath{""}; +std::filesystem::path g_HomeDirectory{""}; +std::filesystem::path g_DefaultRealJitPath{""}; std::unique_ptr g_globalContext = nullptr; @@ -53,7 +53,7 @@ void SetDefaultPaths() if (g_DefaultRealJitPath.empty()) { - g_DefaultRealJitPath = g_HomeDirectory + DIRECTORY_SEPARATOR_STR_A + DEFAULT_REAL_JIT_NAME_A; + g_DefaultRealJitPath = g_HomeDirectory / DEFAULT_REAL_JIT_NAME_A; } } @@ -61,7 +61,7 @@ void SetLibName() { if (g_realJitPath.empty()) { - g_realJitPath = GetEnvWithDefault("SuperPMIShimPath", g_DefaultRealJitPath.c_str()); + g_realJitPath = GetEnvWithDefault("SuperPMIShimPath", g_DefaultRealJitPath.string().c_str()); } } @@ -69,7 +69,7 @@ void SetLogPath() { if (g_logPath.empty()) { - g_logPath = GetEnvWithDefault("SuperPMIShimLogPath", g_HomeDirectory.c_str()); + g_logPath = GetEnvWithDefault("SuperPMIShimLogPath", g_HomeDirectory.string().c_str()); } } From 137e9e3eee2b302ddcab5acb816e5ddd005f9b09 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Sun, 8 Jun 2025 20:18:11 +0800 Subject: [PATCH 09/28] Use dlsym since dlopen has been used --- src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp | 3 --- src/coreclr/tools/superpmi/superpmi-shared/spmiutil.h | 6 ++++++ .../superpmi-shim-counter/superpmi-shim-counter.cpp | 4 ++-- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp index 00ad82dc6e6e9b..96a353ebdbdce4 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp @@ -13,9 +13,6 @@ #include #include #include -#ifdef TARGET_UNIX -#include -#endif static bool breakOnDebugBreakorAV = false; diff --git a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.h b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.h index 8160ba913e0dbe..b49f59b6210a83 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); 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 58abb428697064..8e33d5f46a9978 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 @@ -112,7 +112,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. @@ -150,7 +150,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()); From 65cdfa52d5ca8325e885193e4a552adbf92eb137 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Sun, 8 Jun 2025 20:34:47 +0800 Subject: [PATCH 10/28] Port changes to other shims --- .../superpmi-shim-collector.cpp | 85 +++++++++---------- .../superpmi-shim-simple.cpp | 69 +++++++-------- 2 files changed, 73 insertions(+), 81 deletions(-) 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..e486051b6f9a81 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,47 +17,63 @@ // -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::filesystem::path g_realJitPath{""}; // Destructable objects will be cleaned up and won't leak +std::filesystem::path g_logPath{""}; +std::filesystem::path g_HomeDirectory{""}; +std::filesystem::path g_DefaultRealJitPath{""}; +WCHAR* g_dataFileName = nullptr; // We leak this +MethodContext* g_globalContext = nullptr; +bool g_initialized = false; +char* g_collectionFilter = nullptr; + +// RAII holder for logger +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 / 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.string().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.string().c_str()); } } @@ -71,16 +87,6 @@ void SetLogPathName() 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); - } -} - void SetCollectionFilter() { g_collectionFilter = GetEnvironmentVariableWithDefaultA("SuperPMIShimFilter", nullptr); @@ -114,10 +120,6 @@ void InitializeShim() _CrtSetReportFile(_CRT_ASSERT, _CRTDBG_FILE_STDERR); #endif // HOST_WINDOWS - Logger::Initialize(); - SetLogFilePath(); - Logger::OpenLogFile(g_logFilePath); - g_initialized = true; } @@ -135,13 +137,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 +160,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 +191,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()); 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..ea764b921c390d 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,52 @@ #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::filesystem::path g_realJitPath{""}; // Destructable objects will be cleaned up and won't leak +std::filesystem::path g_logPath{""}; +std::filesystem::path g_HomeDirectory{""}; +std::filesystem::path g_DefaultRealJitPath{""}; + +// RAII holder for logger +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 / 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.string().c_str()); } } @@ -72,20 +80,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 +102,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 +131,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()); From 67d2f11d70e3b448962114265e71ad0cf9b06415 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Sun, 8 Jun 2025 20:42:41 +0800 Subject: [PATCH 11/28] Update g_dataFileName --- .../superpmi-shim-collector.cpp | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) 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 e486051b6f9a81..727745a4e47d97 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 @@ -12,6 +12,7 @@ #include "logging.h" #include "spmiutil.h" #include "jithost.h" +#include // Assumptions: // -We'll never be unloaded - we leak memory and have no facility to unload libraries @@ -22,7 +23,7 @@ std::filesystem::path g_realJitPath{""}; // Destructable objects will be cleaned std::filesystem::path g_logPath{""}; std::filesystem::path g_HomeDirectory{""}; std::filesystem::path g_DefaultRealJitPath{""}; -WCHAR* g_dataFileName = nullptr; // We leak this +std::filesystem::path g_dataFileName{""}; MethodContext* g_globalContext = nullptr; bool g_initialized = false; char* g_collectionFilter = nullptr; @@ -79,10 +80,17 @@ void SetLogPath() 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"); +#ifdef HOST_WINDOWS + std::string fileName(GetCommandLineA()); +#else + std::string fileName(""); + std::ifstream proc("/proc/self/cmdline"); + if (proc) + { + std::getline(proc, fileName); + } +#endif + const std::string extension = ".mc"; g_dataFileName = GetResultFileName(g_logPath, fileName, extension); } @@ -219,11 +227,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 = CreateFileW((const WCHAR*)g_dataFileName.u16string().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.string().c_str(), GetLastError()); } return pJitInstance; From ce5d486fde946bc925bd430dddacd9c3ca771eab Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Sun, 8 Jun 2025 21:02:26 +0800 Subject: [PATCH 12/28] Cleanup --- .../superpmi/superpmi-shared/spmiutil.cpp | 156 ++---------------- .../tools/superpmi/superpmi-shared/spmiutil.h | 6 +- .../superpmi/superpmi-shared/standardpch.h | 1 + .../icorjitcompiler.cpp | 2 +- .../superpmi-shim-collector.cpp | 15 +- .../methodcallsummarizer.cpp | 13 +- 6 files changed, 15 insertions(+), 178 deletions(-) diff --git a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp index 96a353ebdbdce4..783581912bd3ff 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp @@ -108,56 +108,19 @@ const char* GetEnvWithDefault(const char* envVarName, const char* defaultValue) return env ? env : defaultValue; } -#ifdef TARGET_UNIX -// For some reason, the PAL doesn't have GetCommandLineA(). So write it. -LPSTR GetCommandLineA() +std::string GetProcessCommandLine() { - LPSTR pCmdLine = nullptr; - LPWSTR pwCmdLine = GetCommandLineW(); - - if (pwCmdLine != nullptr) - { - // Convert to ASCII - - int n = WideCharToMultiByte(CP_ACP, 0, pwCmdLine, -1, nullptr, 0, nullptr, nullptr); - if (n == 0) - { - LogError("MultiByteToWideChar failed %d", GetLastError()); - return nullptr; - } - - pCmdLine = new char[n]; - - int n2 = WideCharToMultiByte(CP_ACP, 0, pwCmdLine, -1, pCmdLine, n, nullptr, nullptr); - if ((n2 == 0) || (n2 != n)) - { - LogError("MultiByteToWideChar failed %d", GetLastError()); - return nullptr; - } - } - - return pCmdLine; -} -#endif // TARGET_UNIX - -bool LoadRealJitLib(HMODULE& jitLib, WCHAR* jitLibPath) -{ - // Load Library - if (jitLib == NULL) +#ifdef TARGET_WINDOWS + return ::GetCommandLineA(); +#else + std::string cmdLine(""); + std::ifstream proc("/proc/self/cmdline"); + if (proc) { - if (jitLibPath == nullptr) - { - LogError("LoadRealJitLib - No real jit path"); - return false; - } - jitLib = ::LoadLibraryExW(jitLibPath, NULL, 0); - if (jitLib == NULL) - { - LogError("LoadRealJitLib - LoadLibrary failed to load '%ws' (0x%08x)", jitLibPath, ::GetLastError()); - return false; - } + std::getline(proc, cmdLine); } - return true; + return cmdLine; +#endif } bool LoadRealJitLib(HMODULE& jitLib, const std::filesystem::path& jitLibPath) @@ -185,42 +148,6 @@ bool LoadRealJitLib(HMODULE& jitLib, const std::filesystem::path& jitLibPath) return true; } -void ReplaceIllegalCharacters(WCHAR* 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++) - { - WCHAR ch = *quote; - if ((ch <= 32) || (ch >= 127)) // Only allow textual ASCII characters - { - *quote = W('_'); - } - 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('_'); - break; - case W('"'): - *quote = W(' '); - break; - default: - break; - } - } - } -} - void ReplaceIllegalCharacters(std::string& fileName) { // Perform the following transforms: @@ -256,69 +183,6 @@ void ReplaceIllegalCharacters(std::string& fileName) } } -// All lengths in this function exclude the terminal NULL. -WCHAR* GetResultFileName(const WCHAR* folderPath, const WCHAR* fileName, const WCHAR* 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. - // - WCHAR* fullPath = new WCHAR[MAX_PATH]; - fullPath[0] = W('\0'); - const size_t folderPathLength = GetFullPathNameW(folderPath, MAX_PATH, (LPWSTR)fullPath, NULL); - - if (folderPathLength == 0) - { - LogError("GetResultFileName - can't resolve folder path '%ws'", folderPath); - return nullptr; - } - - // 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) - { - LogError("GetResultFileName - folder path '%ws' length + minimal file name exceeds limit %d", fullPath, maxPathLength); - return nullptr; - } - - // 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; -} - std::filesystem::path GetResultFileName(const std::filesystem::path& folderPath, const std::string& fileName, const std::string& extension) diff --git a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.h b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.h index b49f59b6210a83..cd54d244966299 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.h +++ b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.h @@ -29,14 +29,10 @@ WCHAR* GetEnvironmentVariableWithDefaultW(const WCHAR* envVarName, const WCHAR* const char* GetEnvWithDefault(const char* envVarName, const char* defaultValue = nullptr); -#ifdef TARGET_UNIX -LPSTR GetCommandLineA(); -#endif // TARGET_UNIX +std::string GetProcessCommandLine(); -bool LoadRealJitLib(HMODULE& realJit, WCHAR* realJitPath); bool LoadRealJitLib(HMODULE& realJit, const std::filesystem::path& realJitPath); -WCHAR* GetResultFileName(const WCHAR* folderPath, const WCHAR* fileName, const WCHAR* extension); std::filesystem::path GetResultFileName(const std::filesystem::path& folderPath, const std::string& fileName, const std::string& extension); diff --git a/src/coreclr/tools/superpmi/superpmi-shared/standardpch.h b/src/coreclr/tools/superpmi/superpmi-shared/standardpch.h index 195230468b8f80..5e6dfbd0ad0569 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/standardpch.h +++ b/src/coreclr/tools/superpmi/superpmi-shared/standardpch.h @@ -71,6 +71,7 @@ #include #include #include +#include #ifdef USE_MSVCDIS 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 727745a4e47d97..ef25cfa82ddd40 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 @@ -12,7 +12,6 @@ #include "logging.h" #include "spmiutil.h" #include "jithost.h" -#include // Assumptions: // -We'll never be unloaded - we leak memory and have no facility to unload libraries @@ -80,19 +79,7 @@ void SetLogPath() void SetLogPathName() { -#ifdef HOST_WINDOWS - std::string fileName(GetCommandLineA()); -#else - std::string fileName(""); - std::ifstream proc("/proc/self/cmdline"); - if (proc) - { - std::getline(proc, fileName); - } -#endif - const std::string extension = ".mc"; - - g_dataFileName = GetResultFileName(g_logPath, fileName, extension); + g_dataFileName = GetResultFileName(g_logPath, GetProcessCommandLine(), ".mc"); } void SetCollectionFilter() diff --git a/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.cpp b/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.cpp index 8bedbcd672a684..69fb6b9cb0195d 100644 --- a/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.cpp @@ -9,18 +9,7 @@ MethodCallSummarizer::MethodCallSummarizer(const std::filesystem::path& logPath) { -#ifdef HOST_WINDOWS - std::string fileName(GetCommandLineA()); -#else - std::string fileName(""); - std::ifstream proc("/proc/self/cmdline"); - if (proc) - { - std::getline(proc, fileName); - } -#endif - const std::string extension = ".csv"; - dataFileName = GetResultFileName(logPath, fileName, extension); + dataFileName = GetResultFileName(logPath, GetProcessCommandLine(), ".csv"); } // Use ordered map to make the most commonly added items are at the top of the list... From 93224cfbf4c427ca9c0081417eb4eeed4b090e10 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Tue, 10 Jun 2025 15:51:29 +0800 Subject: [PATCH 13/28] Don't replace . in extension --- src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp index 783581912bd3ff..2dba9b53b40331 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp @@ -194,9 +194,9 @@ std::filesystem::path GetResultFileName(const std::filesystem::path& folderPath, std::stringstream ss; ss << std::hex << std::setw(8) << std::setfill('0') << randomNumber; - std::string nameAndExtension = fileName + ss.str() + extension; - ReplaceIllegalCharacters(nameAndExtension); - return folderPath / nameAndExtension; + std::string copy = fileName; + ReplaceIllegalCharacters(copy); + return folderPath / (copy + ss.str() + extension); } #ifdef TARGET_AMD64 From 08d99e415b4caf127a1a876f463e9802aa4bdf90 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Tue, 10 Jun 2025 16:51:30 +0800 Subject: [PATCH 14/28] Limit result file path --- .../tools/superpmi/superpmi-shared/spmiutil.cpp | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp index 2dba9b53b40331..c1a259217f1ba1 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp @@ -193,10 +193,24 @@ std::filesystem::path GetResultFileName(const std::filesystem::path& folderPath, 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; + + // Limit the total file name length to MAX_PATH - 50 + int usableLength = MAX_PATH - 50 - (int)folderPath.native().size() - (int)suffix.size(); + if (usableLength < 0) + { + LogError("GetResultFileName - folder path '%ws' length + minimal file name exceeds limit %d", folderPath.u16string().c_str(), MAX_PATH - 50); + return ""; + } std::string copy = fileName; + if (copy.size() > usableLength) + { + copy = copy.substr(0, usableLength); + } + ReplaceIllegalCharacters(copy); - return folderPath / (copy + ss.str() + extension); + return folderPath / (copy + suffix); } #ifdef TARGET_AMD64 From b22e3a972ecc381d8fcd0e1e44a983acb6f7bf63 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Tue, 10 Jun 2025 16:53:08 +0800 Subject: [PATCH 15/28] Add comment about destructor --- .../superpmi/superpmi-shim-collector/superpmi-shim-collector.cpp | 1 + .../superpmi/superpmi-shim-counter/superpmi-shim-counter.cpp | 1 + .../tools/superpmi/superpmi-shim-simple/superpmi-shim-simple.cpp | 1 + 3 files changed, 3 insertions(+) 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 ef25cfa82ddd40..bf2d26c0cf5fbe 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 @@ -28,6 +28,7 @@ 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: 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 8e33d5f46a9978..d7848bee67d70b 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 @@ -24,6 +24,7 @@ std::filesystem::path g_DefaultRealJitPath{""}; std::unique_ptr g_globalContext = nullptr; // RAII holder for logger +// Global deconstructors are unreliable. We only use it for superpmi shim. class LoggerHolder { public: 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 ea764b921c390d..cd1d6255e4fad0 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 @@ -22,6 +22,7 @@ std::filesystem::path g_HomeDirectory{""}; std::filesystem::path g_DefaultRealJitPath{""}; // RAII holder for logger +// Global deconstructors are unreliable. We only use it for superpmi shim. class LoggerHolder { public: From 407af70c7f48c68caece6454c45a81156869f130 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Tue, 10 Jun 2025 17:36:53 +0800 Subject: [PATCH 16/28] Fix sign/unsigned mismatch --- src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp index c1a259217f1ba1..78505e904c326c 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp @@ -204,7 +204,7 @@ std::filesystem::path GetResultFileName(const std::filesystem::path& folderPath, } std::string copy = fileName; - if (copy.size() > usableLength) + if ((int)copy.size() > usableLength) { copy = copy.substr(0, usableLength); } From 9f1fcc808cba968dca705393827e4e16e39d6b2d Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Wed, 13 Aug 2025 01:17:56 +0800 Subject: [PATCH 17/28] Remove C++ 17 and fstream usage --- src/coreclr/tools/superpmi/CMakeLists.txt | 5 ----- src/coreclr/tools/superpmi/superpmi-shared/standardpch.h | 4 ---- 2 files changed, 9 deletions(-) diff --git a/src/coreclr/tools/superpmi/CMakeLists.txt b/src/coreclr/tools/superpmi/CMakeLists.txt index e92d5e235e9ed8..d4308d2e03cabd 100644 --- a/src/coreclr/tools/superpmi/CMakeLists.txt +++ b/src/coreclr/tools/superpmi/CMakeLists.txt @@ -1,8 +1,3 @@ -# SuperPMI is development-only toolset. -# Enable C++ 17 to get more handy stuffs from STL. -set(CMAKE_CXX_STANDARD 17) -set(CMAKE_CXX_STANDARD_REQUIRED ON) - add_subdirectory(superpmi) add_subdirectory(mcs) add_subdirectory(superpmi-shim-collector) diff --git a/src/coreclr/tools/superpmi/superpmi-shared/standardpch.h b/src/coreclr/tools/superpmi/superpmi-shared/standardpch.h index 5e6dfbd0ad0569..e18cb57a76a002 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/standardpch.h +++ b/src/coreclr/tools/superpmi/superpmi-shared/standardpch.h @@ -15,9 +15,6 @@ #define DEBUG_BREAK DebugBreak() #endif -// filesystem fails parsing if included after PAL header -#include - #ifndef WIN32_LEAN_AND_MEAN #define WIN32_LEAN_AND_MEAN #endif // WIN32_LEAN_AND_MEAN @@ -71,7 +68,6 @@ #include #include #include -#include #ifdef USE_MSVCDIS From 70152a8d89c677957341828db7670a45cce9c56e Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Wed, 13 Aug 2025 01:34:35 +0800 Subject: [PATCH 18/28] Remove usages of std::filesystem::path --- .../superpmi/superpmi-shared/spmiutil.cpp | 18 ++++++------ .../tools/superpmi/superpmi-shared/spmiutil.h | 8 +++--- .../superpmi-shim-collector.cpp | 28 +++++++++---------- .../methodcallsummarizer.cpp | 2 +- .../methodcallsummarizer.h | 4 +-- .../superpmi-shim-counter.cpp | 16 +++++------ .../superpmi-shim-simple.cpp | 14 +++++----- 7 files changed, 45 insertions(+), 45 deletions(-) diff --git a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp index 14616dd074f04f..243d3b61777fe6 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp @@ -123,7 +123,7 @@ std::string GetProcessCommandLine() #endif } -bool LoadRealJitLib(HMODULE& jitLib, const std::filesystem::path& jitLibPath) +bool LoadRealJitLib(HMODULE& jitLib, const std::string& jitLibPath) { // Load Library if (jitLib == NULL) @@ -134,14 +134,14 @@ bool LoadRealJitLib(HMODULE& jitLib, const std::filesystem::path& jitLibPath) return false; } #ifdef TARGET_WINDOWS - jitLib = ::LoadLibraryExW(jitLibPath.c_str(), NULL, 0); + jitLib = ::LoadLibraryExA(jitLibPath.c_str(), NULL, 0); #else jitLib = ::dlopen(jitLibPath.c_str(), RTLD_LAZY); // The simulated DllMain of JIT doesn't do any meaningful initialization. Skip it. #endif if (jitLib == NULL) { - LogError("LoadRealJitLib - LoadLibrary failed to load '%s' (%d)", jitLibPath.string().c_str(), errno); + LogError("LoadRealJitLib - LoadLibrary failed to load '%s' (%d)", jitLibPath.c_str(), errno); return false; } } @@ -183,9 +183,9 @@ void ReplaceIllegalCharacters(std::string& fileName) } } -std::filesystem::path GetResultFileName(const std::filesystem::path& folderPath, - const std::string& fileName, - const std::string& extension) +std::string GetResultFileName(const std::string& folderPath, + const std::string& fileName, + const std::string& extension) { // Append a random string to improve uniqueness. // @@ -196,10 +196,10 @@ std::filesystem::path GetResultFileName(const std::filesystem::path& folderPath, std::string suffix = ss.str() + extension; // Limit the total file name length to MAX_PATH - 50 - int usableLength = MAX_PATH - 50 - (int)folderPath.native().size() - (int)suffix.size(); + int usableLength = MAX_PATH - 50 - (int)folderPath.size() - (int)suffix.size(); if (usableLength < 0) { - LogError("GetResultFileName - folder path '%ws' length + minimal file name exceeds limit %d", folderPath.u16string().c_str(), MAX_PATH - 50); + LogError("GetResultFileName - folder path '%s' length + minimal file name exceeds limit %d", folderPath.c_str(), MAX_PATH - 50); return ""; } @@ -210,7 +210,7 @@ std::filesystem::path GetResultFileName(const std::filesystem::path& folderPath, } ReplaceIllegalCharacters(copy); - return folderPath / (copy + suffix); + 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 26d5656c179ff6..93e6547d1cf1b5 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.h +++ b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.h @@ -31,11 +31,11 @@ const char* GetEnvWithDefault(const char* envVarName, const char* defaultValue = std::string GetProcessCommandLine(); -bool LoadRealJitLib(HMODULE& realJit, const std::filesystem::path& realJitPath); +bool LoadRealJitLib(HMODULE& realJit, const std::string& realJitPath); -std::filesystem::path GetResultFileName(const std::filesystem::path& folderPath, - const std::string& fileName, - const std::string& extension); +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/superpmi-shim-collector.cpp b/src/coreclr/tools/superpmi/superpmi-shim-collector/superpmi-shim-collector.cpp index bf2d26c0cf5fbe..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,15 +17,15 @@ // -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) -std::filesystem::path g_realJitPath{""}; // Destructable objects will be cleaned up and won't leak -std::filesystem::path g_logPath{""}; -std::filesystem::path g_HomeDirectory{""}; -std::filesystem::path g_DefaultRealJitPath{""}; -std::filesystem::path g_dataFileName{""}; -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. @@ -58,7 +58,7 @@ void SetDefaultPaths() if (g_DefaultRealJitPath.empty()) { - g_DefaultRealJitPath = g_HomeDirectory / DEFAULT_REAL_JIT_NAME_A; + g_DefaultRealJitPath = g_HomeDirectory + DIRECTORY_SEPARATOR_CHAR_A + DEFAULT_REAL_JIT_NAME_A; } } @@ -66,7 +66,7 @@ void SetLibName() { if (g_realJitPath.empty()) { - g_realJitPath = GetEnvWithDefault("SuperPMIShimPath", g_DefaultRealJitPath.string().c_str()); + g_realJitPath = GetEnvWithDefault("SuperPMIShimPath", g_DefaultRealJitPath.c_str()); } } @@ -74,7 +74,7 @@ void SetLogPath() { if (g_logPath.empty()) { - g_logPath = GetEnvWithDefault("SuperPMIShimLogPath", g_HomeDirectory.string().c_str()); + g_logPath = GetEnvWithDefault("SuperPMIShimLogPath", g_HomeDirectory.c_str()); } } @@ -215,11 +215,11 @@ extern "C" DLLEXPORT ICorJitCompiler* getJit() #endif // create our datafile - pJitInstance->hFile = CreateFileW((const WCHAR*)g_dataFileName.u16string().c_str(), GENERIC_READ | GENERIC_WRITE, 0, 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 '%s': error %d", g_dataFileName.string().c_str(), 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 69fb6b9cb0195d..4bacebbb53ae35 100644 --- a/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.cpp @@ -7,7 +7,7 @@ #include "spmiutil.h" #include -MethodCallSummarizer::MethodCallSummarizer(const std::filesystem::path& logPath) +MethodCallSummarizer::MethodCallSummarizer(const std::string& logPath) { dataFileName = GetResultFileName(logPath, GetProcessCommandLine(), ".csv"); } diff --git a/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.h b/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.h index df45654f594c59..3a2b3a67d04ec8 100644 --- a/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.h +++ b/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.h @@ -10,14 +10,14 @@ class MethodCallSummarizer { public: - MethodCallSummarizer(const std::filesystem::path& name); + MethodCallSummarizer(const std::string& name); void AddCall(const char* name); void SaveTextFile(); ~MethodCallSummarizer(); private: std::map namesAndCounts; - std::filesystem::path dataFileName; + std::string dataFileName; }; #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 d7848bee67d70b..c479ca85ad9c04 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 @@ -15,11 +15,11 @@ #include "jithost.h" #include -HMODULE g_hRealJit = 0; // We leak this currently (could do the proper shutdown in process_detach) -std::filesystem::path g_realJitPath{""}; // Destructable objects will be cleaned up and won't leak -std::filesystem::path g_logPath{""}; -std::filesystem::path g_HomeDirectory{""}; -std::filesystem::path g_DefaultRealJitPath{""}; +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::unique_ptr g_globalContext = nullptr; @@ -54,7 +54,7 @@ void SetDefaultPaths() if (g_DefaultRealJitPath.empty()) { - g_DefaultRealJitPath = g_HomeDirectory / DEFAULT_REAL_JIT_NAME_A; + g_DefaultRealJitPath = g_HomeDirectory + DIRECTORY_SEPARATOR_CHAR_A + DEFAULT_REAL_JIT_NAME_A; } } @@ -62,7 +62,7 @@ void SetLibName() { if (g_realJitPath.empty()) { - g_realJitPath = GetEnvWithDefault("SuperPMIShimPath", g_DefaultRealJitPath.string().c_str()); + g_realJitPath = GetEnvWithDefault("SuperPMIShimPath", g_DefaultRealJitPath.c_str()); } } @@ -70,7 +70,7 @@ void SetLogPath() { if (g_logPath.empty()) { - g_logPath = GetEnvWithDefault("SuperPMIShimLogPath", g_HomeDirectory.string().c_str()); + g_logPath = GetEnvWithDefault("SuperPMIShimLogPath", g_HomeDirectory.c_str()); } } 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 cd1d6255e4fad0..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 @@ -15,11 +15,11 @@ #include "jithost.h" -HMODULE g_hRealJit = 0; // We leak this currently (could do the proper shutdown in process_detach) -std::filesystem::path g_realJitPath{""}; // Destructable objects will be cleaned up and won't leak -std::filesystem::path g_logPath{""}; -std::filesystem::path g_HomeDirectory{""}; -std::filesystem::path g_DefaultRealJitPath{""}; +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. @@ -52,7 +52,7 @@ void SetDefaultPaths() if (g_DefaultRealJitPath.empty()) { - g_DefaultRealJitPath = g_HomeDirectory / DEFAULT_REAL_JIT_NAME_A; + g_DefaultRealJitPath = g_HomeDirectory + DIRECTORY_SEPARATOR_CHAR_A + DEFAULT_REAL_JIT_NAME_A; } } @@ -60,7 +60,7 @@ void SetLibName() { if (g_realJitPath.empty()) { - g_realJitPath = GetEnvWithDefault("SuperPMIShimPath", g_DefaultRealJitPath.string().c_str()); + g_realJitPath = GetEnvWithDefault("SuperPMIShimPath", g_DefaultRealJitPath.c_str()); } } From 4db4066afb5d3d32ed2b745efc42df3f3c6e14e6 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Wed, 13 Aug 2025 01:48:34 +0800 Subject: [PATCH 19/28] Don't introduce fstream --- .../methodcallsummarizer.cpp | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.cpp b/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.cpp index 4bacebbb53ae35..ce9f142119c9c9 100644 --- a/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.cpp @@ -5,7 +5,6 @@ #include "methodcallsummarizer.h" #include "logging.h" #include "spmiutil.h" -#include MethodCallSummarizer::MethodCallSummarizer(const std::string& logPath) { @@ -22,20 +21,21 @@ void MethodCallSummarizer::AddCall(const char* name) void MethodCallSummarizer::SaveTextFile() { - try + FILE* fp = fopen(dataFileName.c_str(), "w"); + if (fp == NULL) { - std::ofstream outFile(dataFileName); - outFile << "FunctionName,Count" << std::endl; - - for (auto& elem : namesAndCounts) - { - outFile << elem.first << "," << elem.second << std::endl; - } + LogError("Couldn't open file '%s': error %d", dataFileName.c_str(), errno); + return; } - catch (std::exception& ex) + + fprintf(fp, "FunctionName,Count\n"); + + for (auto& elem : namesAndCounts) { - LogError("Couldn't write file '%s': %s", dataFileName.c_str(), ex.what()); + fprintf(fp, "%s,%d\n", elem.first.c_str(), elem.second); } + + fclose(fp); } MethodCallSummarizer::~MethodCallSummarizer() From 58183b3bd7486038c32b24be8a5483bcd82646a9 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Wed, 13 Aug 2025 02:10:42 +0800 Subject: [PATCH 20/28] Cleanup UNIX usage --- .../tools/superpmi/superpmi-shared/spmiutil.cpp | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp index 243d3b61777fe6..98161f7a537652 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp @@ -113,13 +113,17 @@ std::string GetProcessCommandLine() #ifdef TARGET_WINDOWS return ::GetCommandLineA(); #else - std::string cmdLine(""); - std::ifstream proc("/proc/self/cmdline"); - if (proc) + char cmdLine[256]; + + FILE* fp = fopen("/proc/self/cmdline", "r"); + if (fp != NULL) { - std::getline(proc, cmdLine); + fgets(cmdLine, sizeof(cmdLine), fp); + fclose(fp); + return cmdLine; } - return cmdLine; + + return ""; #endif } From b3ce44d8ef67e20e1d4365299c26af50b7f3f1eb Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Wed, 13 Aug 2025 10:35:28 +0800 Subject: [PATCH 21/28] Mark method as private --- .../superpmi/superpmi-shim-counter/methodcallsummarizer.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.h b/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.h index 3a2b3a67d04ec8..aca87d55e91dd8 100644 --- a/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.h +++ b/src/coreclr/tools/superpmi/superpmi-shim-counter/methodcallsummarizer.h @@ -12,12 +12,13 @@ class MethodCallSummarizer public: MethodCallSummarizer(const std::string& name); void AddCall(const char* name); - void SaveTextFile(); ~MethodCallSummarizer(); private: std::map namesAndCounts; std::string dataFileName; + + void SaveTextFile(); }; #endif From 7e81613032ace94ac6114851b5412b820130f07c Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Wed, 13 Aug 2025 10:44:26 +0800 Subject: [PATCH 22/28] make_unique is not available in C++ 11 --- .../superpmi/superpmi-shim-counter/superpmi-shim-counter.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 c479ca85ad9c04..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 @@ -126,7 +126,7 @@ extern "C" DLLEXPORT void jitStartup(ICorJitHost* host) if (g_globalContext == nullptr) { SetLogPath(); - g_globalContext = std::make_unique(g_logPath); + g_globalContext = std::unique_ptr(new MethodCallSummarizer(g_logPath)); } g_ourJitHost->setMethodCallSummarizer(g_globalContext.get()); @@ -171,7 +171,7 @@ extern "C" DLLEXPORT ICorJitCompiler* getJit() if (g_globalContext == nullptr) { SetLogPath(); - g_globalContext = std::make_unique(g_logPath); + g_globalContext = std::unique_ptr(new MethodCallSummarizer(g_logPath)); } pJitInstance->mcs = g_globalContext.get(); return pJitInstance; From c16969529259e85e233e2dbe604c162955e1402a Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Wed, 13 Aug 2025 10:45:10 +0800 Subject: [PATCH 23/28] Update LoadLibrary error checking --- src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp index 98161f7a537652..ecf20b095e1f02 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp @@ -139,15 +139,20 @@ bool LoadRealJitLib(HMODULE& jitLib, const std::string& jitLibPath) } #ifdef TARGET_WINDOWS jitLib = ::LoadLibraryExA(jitLibPath.c_str(), NULL, 0); + if (jitLib == NULL) + { + LogError("LoadRealJitLib - LoadLibrary failed to load '%s' (%d)", 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. -#endif if (jitLib == NULL) { - LogError("LoadRealJitLib - LoadLibrary failed to load '%s' (%d)", jitLibPath.c_str(), errno); + LogError("LoadRealJitLib - dlopen failed to load '%s' (%s)", jitLibPath.c_str(), ::dlerror()); return false; } +#endif } return true; } From f0f1857bded3a114bdd959cd1180adc922b63cad Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Wed, 13 Aug 2025 23:09:32 +0800 Subject: [PATCH 24/28] Handle long cmdline --- src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp index ecf20b095e1f02..1f3bed55e1483c 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp @@ -118,9 +118,13 @@ std::string GetProcessCommandLine() FILE* fp = fopen("/proc/self/cmdline", "r"); if (fp != NULL) { - fgets(cmdLine, sizeof(cmdLine), fp); + std::string result; + while (fgets(cmdLine, sizeof(cmdLine), fp) != NULL) + { + result += cmdLine; + } fclose(fp); - return cmdLine; + return result; } return ""; From b1d29874b250101d117c9d718e86aabf230c6a40 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Thu, 14 Aug 2025 00:49:24 +0800 Subject: [PATCH 25/28] Use getline instead --- .../tools/superpmi/superpmi-shared/spmiutil.cpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp index 1f3bed55e1483c..2f9d6fc7bee148 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp @@ -113,16 +113,21 @@ std::string GetProcessCommandLine() #ifdef TARGET_WINDOWS return ::GetCommandLineA(); #else - char cmdLine[256]; - FILE* fp = fopen("/proc/self/cmdline", "r"); if (fp != NULL) { std::string result; - while (fgets(cmdLine, sizeof(cmdLine), fp) != NULL) + char* cmdLine = nullptr; + size_t size = 0; + + while (getline(&cmdLine, &size, fp) != -1) { result += cmdLine; + free(cmdLine); + cmdLine = nullptr; + size = 0; } + fclose(fp); return result; } From 899f12de6ec957390c244c6590069a279561cdae Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Thu, 14 Aug 2025 12:44:59 +0800 Subject: [PATCH 26/28] Handle \0 delimiter --- src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp index 2f9d6fc7bee148..45abb61485df17 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp @@ -120,9 +120,12 @@ std::string GetProcessCommandLine() char* cmdLine = nullptr; size_t size = 0; - while (getline(&cmdLine, &size, fp) != -1) + while (getdelim(&cmdLine, &size, '\0', fp) != -1) { + // /proc/self/cmdline uses \0 as delimeter, convert it to space result += cmdLine; + result += ' '; + free(cmdLine); cmdLine = nullptr; size = 0; From 16f6ee199940f804a4dc236bde37536907ec849c Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Thu, 14 Aug 2025 13:03:25 +0800 Subject: [PATCH 27/28] Update src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp Co-authored-by: Jan Kotas --- src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp index 45abb61485df17..e57ae820e56df1 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp @@ -123,8 +123,10 @@ std::string GetProcessCommandLine() while (getdelim(&cmdLine, &size, '\0', fp) != -1) { // /proc/self/cmdline uses \0 as delimeter, convert it to space + if (!result.empty()) + result += ' '; + result += cmdLine; - result += ' '; free(cmdLine); cmdLine = nullptr; From dd225a6f5cd287710460e2073bb1ef3cef171a90 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Fri, 15 Aug 2025 10:18:48 +0800 Subject: [PATCH 28/28] Apply suggestions from code review Co-authored-by: Aaron Robinson --- src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp index e57ae820e56df1..20f5f1bf295b33 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shared/spmiutil.cpp @@ -122,7 +122,7 @@ std::string GetProcessCommandLine() while (getdelim(&cmdLine, &size, '\0', fp) != -1) { - // /proc/self/cmdline uses \0 as delimeter, convert it to space + // /proc/self/cmdline uses \0 as delimiter, convert it to space if (!result.empty()) result += ' '; @@ -155,7 +155,7 @@ bool LoadRealJitLib(HMODULE& jitLib, const std::string& jitLibPath) jitLib = ::LoadLibraryExA(jitLibPath.c_str(), NULL, 0); if (jitLib == NULL) { - LogError("LoadRealJitLib - LoadLibrary failed to load '%s' (%d)", jitLibPath.c_str(), ::GetLastError()); + LogError("LoadRealJitLib - LoadLibrary failed to load '%s' (0x%08x)", jitLibPath.c_str(), ::GetLastError()); return false; } #else