Fix long spmi file names/clean few functions. (#19656)
authorSergey Andreenko <seandree@microsoft.com>
Sat, 25 Aug 2018 05:12:25 +0000 (22:12 -0700)
committerGitHub <noreply@github.com>
Sat, 25 Aug 2018 05:12:25 +0000 (22:12 -0700)
* delete extern functions

* extract LoadRealJitLib func

* extract getResultFileName

src/ToolBox/superpmi/superpmi-shared/spmiutil.cpp
src/ToolBox/superpmi/superpmi-shared/spmiutil.h
src/ToolBox/superpmi/superpmi-shim-collector/superpmi-shim-collector.cpp
src/ToolBox/superpmi/superpmi-shim-counter/methodcallsummarizer.cpp
src/ToolBox/superpmi/superpmi-shim-counter/superpmi-shim-counter.cpp
src/ToolBox/superpmi/superpmi-shim-simple/superpmi-shim-simple.cpp
src/ToolBox/superpmi/superpmi/icorjitinfo.cpp
src/ToolBox/superpmi/superpmi/jitdebugger.h
src/ToolBox/superpmi/superpmi/jitinstance.cpp

index 8f60984..dd72659 100644 (file)
 #include "logging.h"
 #include "spmiutil.h"
 
-bool breakOnDebugBreakorAV = false;
+static bool breakOnDebugBreakorAV = false;
+
+bool BreakOnDebugBreakorAV()
+{
+    return breakOnDebugBreakorAV;
+}
+
+void SetBreakOnDebugBreakOrAV(bool value)
+{
+    breakOnDebugBreakorAV = value;
+}
 
 void DebugBreakorAV(int val)
 {
@@ -19,7 +29,7 @@ void DebugBreakorAV(int val)
     {
         if (val == 0)
             __debugbreak();
-        if (breakOnDebugBreakorAV)
+        if (BreakOnDebugBreakorAV())
             __debugbreak();
     }
 
@@ -107,3 +117,111 @@ LPSTR GetCommandLineA()
     return pCmdLine;
 }
 #endif // FEATURE_PAL
+
+bool LoadRealJitLib(HMODULE& jitLib, WCHAR* jitLibPath)
+{
+    // Load Library
+    if (jitLib == NULL)
+    {
+        if (jitLibPath == nullptr)
+        {
+            LogError("LoadRealJitLib - No real jit path");
+            return false;
+        }
+        jitLib = ::LoadLibraryW(jitLibPath);
+        if (jitLib == NULL)
+        {
+            LogError("LoadRealJitLib - LoadLibrary failed to load '%ws' (0x%08x)", jitLibPath, ::GetLastError());
+            return false;
+        }
+    }
+    return true;
+}
+
+void cleanupExecutableName(WCHAR* executableName)
+{
+    WCHAR* quote = nullptr;
+
+    // If there are any quotes in the file name convert them to spaces.
+    while ((quote = wcsstr(executableName, W("\""))) != nullptr)
+    {
+        *quote = W(' ');
+    }
+
+    // Remove any illegal or annoying characters from the file name by converting them to underscores.
+    while ((quote = wcspbrk(executableName, W("=<>:\"/\\|?! *.,"))) != nullptr)
+    {
+        *quote = W('_');
+    }
+}
+
+void generateRandomSuffix(WCHAR* buffer, size_t length)
+{
+    unsigned randNumber = 0;
+#ifdef FEATURE_PAL
+    PAL_Random(&randNumber, sizeof(randNumber));
+#else  // !FEATURE_PAL
+    rand_s(&randNumber);
+#endif // !FEATURE_PAL
+
+    swprintf_s(buffer, length, W("%08X"), randNumber);
+}
+
+// All lengths in this function exclude the terminal NULL.
+WCHAR* getResultFileName(const WCHAR* folderPath, WCHAR* executableName, const WCHAR* extension)
+{
+    cleanupExecutableName(executableName);
+
+    size_t executableNameLength = wcslen(executableName);
+    size_t extensionLength      = wcslen(extension);
+    size_t folderPathLength     = wcslen(folderPath);
+
+    size_t dataFileNameLength = folderPathLength + 1 + executableNameLength + 1 + extensionLength;
+
+    const size_t maxAcceptablePathLength =
+        MAX_PATH - 50; // subtract 50 because excel doesn't like paths longer then 230.
+
+    if (dataFileNameLength > maxAcceptablePathLength)
+    {
+        // The path name is too long; creating the file will fail. This can happen because we use the command line,
+        // which for ngen includes lots of environment variables, for example.
+        // Shorten the executable file name and add a random string to it to avoid collisions.
+
+        const size_t randStringLength = 8;
+
+        size_t lengthToBeDeleted = (dataFileNameLength - maxAcceptablePathLength) + randStringLength;
+
+        if (executableNameLength <= lengthToBeDeleted)
+        {
+            LogError("getResultFileName - path to the output file is too long '%ws\\%ws.%ws(%d)'", folderPath,
+                     executableName, extension, dataFileNameLength);
+            return nullptr;
+        }
+
+        executableNameLength -= lengthToBeDeleted;
+        executableName[executableNameLength] = 0;
+
+        executableNameLength += randStringLength;
+        WCHAR randNumberString[randStringLength + 1];
+        generateRandomSuffix(randNumberString, randStringLength + 1);
+        wcsncat_s(executableName, executableNameLength + 1, randNumberString, randStringLength);
+
+        executableNameLength = wcslen(executableName);
+
+        dataFileNameLength = folderPathLength + 1 + executableNameLength + 1 + extensionLength;
+        if (dataFileNameLength > maxAcceptablePathLength)
+        {
+            LogError("getResultFileName - were not able to short the result file name.", folderPath, executableName,
+                     extension, dataFileNameLength);
+            return nullptr;
+        }
+    }
+
+    WCHAR* dataFileName = new WCHAR[dataFileNameLength + 1];
+    dataFileName[0]     = 0;
+    wcsncat_s(dataFileName, dataFileNameLength + 1, folderPath, folderPathLength);
+    wcsncat_s(dataFileName, dataFileNameLength + 1, DIRECTORY_SEPARATOR_STR_W, 1);
+    wcsncat_s(dataFileName, dataFileNameLength + 1, executableName, executableNameLength);
+    wcsncat_s(dataFileName, dataFileNameLength + 1, extension, extensionLength);
+    return dataFileName;
+}
index 7565122..55573ae 100644 (file)
 
 #include "methodcontext.h"
 
-extern bool breakOnDebugBreakorAV;
+bool BreakOnDebugBreakorAV();
+void SetBreakOnDebugBreakOrAV(bool value);
 
-extern void DebugBreakorAV(int val); // Global(ish) error handler
+void DebugBreakorAV(int val); // Global(ish) error handler
 
-extern char* GetEnvironmentVariableWithDefaultA(const char* envVarName, const char* defaultValue = nullptr);
+char* GetEnvironmentVariableWithDefaultA(const char* envVarName, const char* defaultValue = nullptr);
 
-extern WCHAR* GetEnvironmentVariableWithDefaultW(const WCHAR* envVarName, const WCHAR* defaultValue = nullptr);
+WCHAR* GetEnvironmentVariableWithDefaultW(const WCHAR* envVarName, const WCHAR* defaultValue = nullptr);
 
 #ifdef FEATURE_PAL
-extern LPSTR GetCommandLineA();
+LPSTR GetCommandLineA();
 #endif // FEATURE_PAL
 
+bool LoadRealJitLib(HMODULE& realJit, WCHAR* realJitPath);
+
+WCHAR* getResultFileName(const WCHAR* folderPath, WCHAR* executableName, const WCHAR* extension);
+
 #endif // !_SPMIUtil
index 6971eb3..9c2cdc3 100644 (file)
@@ -66,72 +66,14 @@ 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.
-    WCHAR* OriginalExecutableName =
-        GetCommandLineW(); // TODO-Cleanup: not cool to write to the process view of commandline....
-    size_t len            = wcslen(OriginalExecutableName);
-    WCHAR* ExecutableName = new WCHAR[len + 1];
-    wcscpy_s(ExecutableName, len + 1, OriginalExecutableName);
-    ExecutableName[len] = W('\0');
-    WCHAR* quote1       = NULL;
-
-    // if there are any quotes in filename convert them to spaces.
-    while ((quote1 = wcsstr(ExecutableName, W("\""))) != NULL)
-        *quote1 = W(' ');
-
-    // remove any illegal or annoying characters from file name by converting them to underscores
-    while ((quote1 = wcspbrk(ExecutableName, W("=<>:\"/\\|?! *.,"))) != NULL)
-        *quote1 = W('_');
-
-    const WCHAR* DataFileExtension       = W(".mc");
-    size_t       ExecutableNameLength    = wcslen(ExecutableName);
-    size_t       DataFileExtensionLength = wcslen(DataFileExtension);
-    size_t       logPathLength           = wcslen(g_logPath);
-
-    size_t dataFileNameLength = logPathLength + 1 + ExecutableNameLength + 1 + DataFileExtensionLength + 1;
-
-    const size_t MaxAcceptablePathLength =
-        MAX_PATH - 20; // subtract 20 to leave buffer, for possible random number addition
-    if (dataFileNameLength >= MaxAcceptablePathLength)
-    {
-        // The path name is too long; creating the file will fail. This can happen because we use the command line,
-        // which for ngen includes lots of environment variables, for example.
-
-        // Assume (!) the extra space is all in the ExecutableName, so shorten that.
-        ExecutableNameLength -= dataFileNameLength - MaxAcceptablePathLength;
-
-        dataFileNameLength = MaxAcceptablePathLength;
-    }
-
-// Always add a random number, just in case the above doesn't give us a unique filename.
-#ifdef FEATURE_PAL
-    unsigned __int64 randNumber       = 0;
-    const size_t     RandNumberLength = sizeof(randNumber) * 2 + 1; // 16 hex digits + null
-    WCHAR            RandNumberString[RandNumberLength];
-    PAL_Random(&randNumber, sizeof(randNumber));
-    swprintf_s(RandNumberString, RandNumberLength, W("%016llX"), randNumber);
-#else  // !FEATURE_PAL
-    unsigned int randNumber       = 0;
-    const size_t RandNumberLength = sizeof(randNumber) * 2 + 1; // 8 hex digits + null
-    WCHAR        RandNumberString[RandNumberLength];
-    rand_s(&randNumber);
-    swprintf_s(RandNumberString, RandNumberLength, W("%08X"), randNumber);
-#endif // !FEATURE_PAL
-
-    dataFileNameLength += RandNumberLength - 1;
-
-    // Construct the full pathname we're going to use.
-    g_dataFileName    = new WCHAR[dataFileNameLength];
-    g_dataFileName[0] = 0;
-    wcsncat_s(g_dataFileName, dataFileNameLength, g_logPath, logPathLength);
-    wcsncat_s(g_dataFileName, dataFileNameLength, DIRECTORY_SEPARATOR_STR_W, 1);
-    wcsncat_s(g_dataFileName, dataFileNameLength, ExecutableName, ExecutableNameLength);
-
-    if (RandNumberLength > 0)
-    {
-        wcsncat_s(g_dataFileName, dataFileNameLength, RandNumberString, RandNumberLength);
-    }
-
-    wcsncat_s(g_dataFileName, dataFileNameLength, DataFileExtension, DataFileExtensionLength);
+    const WCHAR* originalExecutableName = GetCommandLineW();
+    size_t       executableNameLength   = wcslen(originalExecutableName);
+    WCHAR*       executableName         = new WCHAR[executableNameLength + 1];
+    wcscpy_s(executableName, executableNameLength + 1, originalExecutableName);
+    executableName[executableNameLength] = W('\0');
+
+    const WCHAR* DataFileExtension = W(".mc");
+    g_dataFileName                 = getResultFileName(g_logPath, executableName, DataFileExtension);
 }
 
 // TODO: this only works for ANSI file paths...
@@ -187,15 +129,9 @@ extern "C" void __stdcall jitStartup(ICorJitHost* host)
     SetDefaultPaths();
     SetLibName();
 
-    // Load Library
-    if (g_hRealJit == 0)
+    if (!LoadRealJitLib(g_hRealJit, g_realJitPath))
     {
-        g_hRealJit = ::LoadLibraryW(g_realJitPath);
-        if (g_hRealJit == 0)
-        {
-            LogError("jitStartup() - LoadLibrary failed to load '%ws' (0x%08x)", g_realJitPath, ::GetLastError());
-            return;
-        }
+        return;
     }
 
     // Get the required entrypoint
@@ -224,15 +160,9 @@ extern "C" ICorJitCompiler* __stdcall getJit()
     SetLogPath();
     SetLogPathName();
 
-    // Load Library
-    if (g_hRealJit == 0)
+    if (!LoadRealJitLib(g_hRealJit, g_realJitPath))
     {
-        g_hRealJit = ::LoadLibraryW(g_realJitPath);
-        if (g_hRealJit == 0)
-        {
-            LogError("getJit() - LoadLibrary failed to load '%ws' (0x%08x)", g_realJitPath, ::GetLastError());
-            return nullptr;
-        }
+        return nullptr;
     }
 
     // get the required entrypoints
@@ -272,15 +202,9 @@ extern "C" void __stdcall sxsJitStartup(CoreClrCallbacks const& original_cccallb
     SetDefaultPaths();
     SetLibName();
 
-    // Load Library
-    if (g_hRealJit == 0)
+    if (!LoadRealJitLib(g_hRealJit, g_realJitPath))
     {
-        g_hRealJit = ::LoadLibraryW(g_realJitPath);
-        if (g_hRealJit == 0)
-        {
-            LogError("sxsJitStartup() - LoadLibrary failed to load '%ws' (0x%08x)", g_realJitPath, ::GetLastError());
-            return;
-        }
+        return;
     }
 
     // get entry point
index ad12ffc..ca1e191 100644 (file)
@@ -6,6 +6,7 @@
 #include "standardpch.h"
 #include "methodcallsummarizer.h"
 #include "logging.h"
+#include "spmiutil.h"
 
 MethodCallSummarizer::MethodCallSummarizer(WCHAR* logPath)
 {
@@ -13,66 +14,10 @@ MethodCallSummarizer::MethodCallSummarizer(WCHAR* logPath)
     names    = nullptr;
     counts   = nullptr;
 
-    WCHAR* ExecutableName = GetCommandLineW();
-    WCHAR* quote1         = NULL;
+    WCHAR*       executableName    = GetCommandLineW();
+    const WCHAR* dataFileExtension = W(".csv");
 
-    // if there are any quotes in filename convert them to spaces.
-    while ((quote1 = wcsstr(ExecutableName, W("\""))) != NULL)
-        *quote1 = W(' ');
-
-    // remove any illegal or annoying characters from file name by converting them to underscores
-    while ((quote1 = wcspbrk(ExecutableName, W("=<>:\"/\\|?! *.,"))) != NULL)
-        *quote1 = W('_');
-
-    const WCHAR* DataFileExtension       = W(".csv");
-    size_t       ExecutableNameLength    = wcslen(ExecutableName);
-    size_t       DataFileExtensionLength = wcslen(DataFileExtension);
-    size_t       logPathLength           = wcslen(logPath);
-
-    unsigned int randNumber = 0;
-    WCHAR        RandNumberString[9];
-    RandNumberString[0]     = L'\0';
-    size_t RandNumberLength = 0;
-
-    size_t dataFileNameLength =
-        logPathLength + 1 + ExecutableNameLength + 1 + RandNumberLength + 1 + DataFileExtensionLength + 1;
-
-    const size_t MaxAcceptablePathLength =
-        MAX_PATH - 20; // subtract 20 to leave buffer, for possible random number addition
-    if (dataFileNameLength >= MaxAcceptablePathLength)
-    {
-        // The path name is too long; creating the file will fail. This can happen because we use the command line,
-        // which for ngen includes lots of environment variables, for example.
-
-        // Assume (!) the extra space is all in the ExecutableName, so shorten that.
-        ExecutableNameLength -= dataFileNameLength - MaxAcceptablePathLength;
-
-        dataFileNameLength = MaxAcceptablePathLength;
-
-#ifdef FEATURE_PAL
-        PAL_Random(&randNumber, sizeof(randNumber));
-#else  // !FEATURE_PAL
-        rand_s(&randNumber);
-#endif // !FEATURE_PAL
-
-        RandNumberLength = 9; // 8 hex digits + null
-        swprintf_s(RandNumberString, RandNumberLength, W("%08X"), randNumber);
-
-        dataFileNameLength += RandNumberLength - 1;
-    }
-
-    dataFileName    = new WCHAR[dataFileNameLength];
-    dataFileName[0] = 0;
-    wcsncat_s(dataFileName, dataFileNameLength, logPath, logPathLength);
-    wcsncat_s(dataFileName, dataFileNameLength, W("\\\0"), 1);
-    wcsncat_s(dataFileName, dataFileNameLength, ExecutableName, ExecutableNameLength);
-
-    if (RandNumberLength > 0)
-    {
-        wcsncat_s(dataFileName, dataFileNameLength, RandNumberString, RandNumberLength);
-    }
-
-    wcsncat_s(dataFileName, dataFileNameLength, DataFileExtension, DataFileExtensionLength);
+    dataFileName = getResultFileName(logPath, executableName, dataFileExtension);
 }
 
 // lots of ways will be faster.. this happens to be decently simple and good enough for the task at hand and nicely
index b234727..51d76b4 100644 (file)
@@ -116,15 +116,9 @@ extern "C" void __stdcall jitStartup(ICorJitHost* host)
     SetDefaultPaths();
     SetLibName();
 
-    // Load Library
-    if (g_hRealJit == 0)
+    if (!LoadRealJitLib(g_hRealJit, g_realJitPath))
     {
-        g_hRealJit = ::LoadLibraryW(g_realJitPath);
-        if (g_hRealJit == 0)
-        {
-            LogError("jitStartup - LoadLibrary failed to load '%ws' (0x%08x)", g_realJitPath, ::GetLastError());
-            return;
-        }
+        return;
     }
 
     // Get the required entrypoint
@@ -160,15 +154,9 @@ extern "C" ICorJitCompiler* __stdcall getJit()
     SetDefaultPaths();
     SetLibName();
 
-    // Load Library
-    if (g_hRealJit == 0)
+    if (!LoadRealJitLib(g_hRealJit, g_realJitPath))
     {
-        g_hRealJit = ::LoadLibraryW(g_realJitPath);
-        if (g_hRealJit == 0)
-        {
-            LogError("getJit() - LoadLibrary failed to load '%ws' (0x%08x)", g_realJitPath, ::GetLastError());
-            return nullptr;
-        }
+        return nullptr;
     }
 
     // get the required entrypoints
@@ -206,15 +194,9 @@ extern "C" void __stdcall sxsJitStartup(CoreClrCallbacks const& original_cccallb
     SetDefaultPaths();
     SetLibName();
 
-    // Load Library
-    if (g_hRealJit == 0)
+    if (!LoadRealJitLib(g_hRealJit, g_realJitPath))
     {
-        g_hRealJit = ::LoadLibraryW(g_realJitPath);
-        if (g_hRealJit == 0)
-        {
-            LogError("sxsJitStartup() - LoadLibrary failed to load '%ws' (0x%08x)", g_realJitPath, ::GetLastError());
-            return;
-        }
+        return;
     }
 
     // get entry point
index f374836..1f446ca 100644 (file)
@@ -101,15 +101,9 @@ extern "C" void __stdcall jitStartup(ICorJitHost* host)
     SetDefaultPaths();
     SetLibName();
 
-    // Load Library
-    if (g_hRealJit == 0)
+    if (!LoadRealJitLib(g_hRealJit, g_realJitPath))
     {
-        g_hRealJit = ::LoadLibraryW(g_realJitPath);
-        if (g_hRealJit == 0)
-        {
-            LogError("getJit() - LoadLibrary failed to load '%ws' (0x%08x)", g_realJitPath, ::GetLastError());
-            return;
-        }
+        return;
     }
 
     // Get the required entrypoint
@@ -136,15 +130,9 @@ extern "C" ICorJitCompiler* __stdcall getJit()
     SetDefaultPaths();
     SetLibName();
 
-    // Load Library
-    if (g_hRealJit == 0)
+    if (!LoadRealJitLib(g_hRealJit, g_realJitPath))
     {
-        g_hRealJit = ::LoadLibraryW(g_realJitPath);
-        if (g_hRealJit == 0)
-        {
-            LogError("getJit() - LoadLibrary failed to load '%ws' (0x%08x)", g_realJitPath, ::GetLastError());
-            return nullptr;
-        }
+        return nullptr;
     }
 
     // get the required entrypoints
index 31be088..3eb5a31 100644 (file)
@@ -1756,7 +1756,7 @@ int MyICJI::doAssert(const char* szFile, int iLine, const char* szExpr)
     // Under "/boa", ask the user if they want to attach a debugger. If they do, the debugger will be attached,
     // then we'll call DebugBreakorAV(), which will issue a __debugbreak() and actually cause
     // us to stop in the debugger.
-    if (breakOnDebugBreakorAV)
+    if (BreakOnDebugBreakorAV())
     {
         DbgBreakCheck(szFile, iLine, szExpr);
     }
index 27f4212..7a63ed8 100644 (file)
@@ -6,9 +6,6 @@
 #ifndef _JitDebugger
 #define _JitDebugger
 
-extern bool breakOnDebugBreakorAV; // It's kind of awful that I'm making this global, but it was kind of awful that it
-                                   // was file-global already.
-
 //
 // Functions to support just-in-time debugging.
 //
index ce2205c..3333567 100644 (file)
@@ -46,17 +46,14 @@ JitInstance* JitInstance::InitJit(char*          nameOfJit,
     return jit;
 }
 
-HRESULT JitInstance::StartUp(char*          PathToJit,
-                             bool           copyJit,
-                             bool           parambreakOnDebugBreakorAV,
-                             MethodContext* firstContext)
+HRESULT JitInstance::StartUp(char* PathToJit, bool copyJit, bool breakOnDebugBreakorAV, MethodContext* firstContext)
 {
     // startup jit
     DWORD dwRetVal = 0;
     UINT  uRetVal  = 0;
     BOOL  bRetVal  = FALSE;
 
-    breakOnDebugBreakorAV = parambreakOnDebugBreakorAV;
+    SetBreakOnDebugBreakOrAV(breakOnDebugBreakorAV);
 
     char pFullPathName[MAX_PATH];
     char lpTempPathBuffer[MAX_PATH];
@@ -525,4 +522,4 @@ bool JitInstance::resetConfig(MethodContext* firstContext)
     delete static_cast<JitHost*>(jitHost);
     jitHost = newHost;
     return true;
-}
\ No newline at end of file
+}