Fix misc SOS bugs (#2600)

Fix misc SOS bugs

* Fix bug in the AddFilesFromDirectoryToTpaList issue: #2596

* Added some logging to GetLineByOffset

* Improve SymbolService.ParseSymbolPath support for Watson. Issue #2512. Can
now handle the various symbol paths that Watson can throw at us. Doesn't support actually calling the symbol server
dll like in the symsrv*symaudit.dll*\\server\share syntax. The dll is ignored.

* Minor doc updates

* Better loadsymbols error message when no server is set

Author: mikem8361

documentation/FAQ.md

@@ -1,6 +1,8 @@
 Frequently Asked Questions
 ==========================
 
+* `dotnet-dump analyze` running on Windows doesn't support MacOS .NET 5.0 and 6.0 core dumps. `dotnet-dump` running on MacOS does support .NET 5.0 but not 6.0 core dumps (which will be fixed in a future dotnet-dump release). MacOS .NET 6.0 core dumps generated by the runtime via [createdump](https://github.com/dotnet/runtime/blob/main/docs/design/coreclr/botr/xplat-minidump-generation.md#os-x) are supported by lldb/SOS running on MacOS.
+
 * If SOS or dotnet-dump analyze commands display "UNKNOWN" for types or functions names, your core dump may not have all the managed state. Dumps created with gdb or gcore have this problem. Linux system generated core dumps need the `coredump_filter` for the process to be set to at least 0x3f. See [core](http://man7.org/linux/man-pages/man5/core.5.html) for more information.
 
 * If dump collection (`dotnet-dump collect` or `createdump`) doesn't work in a docker container, try adding the SYS\_TRACE capability with --cap-add=SYS\_PTRACE or --privileged.

documentation/debugging-coredump.md

@@ -72,6 +72,8 @@ Even if the core dump was not generated on this machine, the native and managed
 
 Follow the rest of the above Linux steps to set the symbol server and load native symbols.
 
+NOTE: The following issue has been fixed with .NET 6.0 core dumps generated by the runtime (see [createdump](https://github.com/dotnet/runtime/blob/main/docs/design/coreclr/botr/xplat-minidump-generation.md#os-x)) with a recent version of SOS.
+
 The MacOS lldb has a bug that prevents SOS clrstack from properly working. Because of this bug SOS can't properly match the lldb native with with the managed thread OSID displayed by `clrthreads`. The `setsostid` command is a work around for this lldb bug. This command maps the OSID from this command:
 
 ```

documentation/lldb/linux-instructions.md

@@ -88,15 +88,9 @@ To install the lldb packages:
 
 This installs lldb version 10.0.0.
 
-#### Alpine 3.9 ####
+#### Alpine 3.9 to 3.12 ####
 
-Currently there is no lldb that works on Alpine.
-
-Issue https://github.com/dotnet/diagnostics/issues/73
-
-#### Alpine 3.12 ####
-
-lldb 10.0 is available for this Apline version.
+lldb 10.0 is available and works for these Apline versions.
 
 #### CentOS 6 ####
 

dotnet.sh

@@ -8,6 +8,7 @@
 using Microsoft.SymbolStore.SymbolStores;
 using SOS;
 using System;
+using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Diagnostics;
 using System.IO;
@@ -27,8 +28,8 @@ public class SymbolService : ISymbolService
         /// <summary>
         /// Symbol server URLs
         /// </summary>
-        const string MsdlSymbolServer = "http://msdl.microsoft.com/download/symbols/";
-        const string SymwebSymbolServer = "http://symweb.corp.microsoft.com/";
+        public const string MsdlSymbolServer = "http://msdl.microsoft.com/download/symbols/";
+        public const string SymwebSymbolServer = "http://symweb.corp.microsoft.com/";
 
         private readonly IHost _host;
         private string _defaultSymbolCache;
@@ -96,77 +97,103 @@ public bool ParseSymbolPath(string symbolPath)
 
             foreach (string path in paths.Reverse())
             {
-                string[] parts = path.Split(new char[] { '*' }, StringSplitOptions.RemoveEmptyEntries);
-
-                // UNC or directory paths are ignored (paths not prefixed with srv* or cache*).
+                string[] parts = path.Split(new char[] { '*' }, StringSplitOptions.None);
                 if (parts.Length > 0)
                 {
-                    string symbolServerPath = null;
-                    string symbolCachePath = null;
+                    List<string> symbolCachePaths = new();
                     string symbolDirectoryPath = null;
-                    bool msdl = false;
+                    string symbolServerPath = null;
+
+                    void ParseServer(int start)
+                    {
+                        symbolServerPath = MsdlSymbolServer;
+                        for (int i = start; i < parts.Length; i++)
+                        {
+                            if (string.IsNullOrEmpty(parts[i]))
+                            {
+                                // srv** means use default cache
+                                if (i != (parts.Length - 1))
+                                {
+                                    symbolCachePaths.Add(DefaultSymbolCache);
+                                }
+                            }
+                            else if (i < (parts.Length - 1))
+                            {
+                                symbolCachePaths.Add(parts[i]);
+                            }
+                            else
+                            {
+                                symbolServerPath = parts[i];
+                            }
+                        }
+                    }
 
                     switch (parts[0].ToLowerInvariant())
                     {
+                        case "symsrv":
+                            if (parts.Length <= 2)
+                            {
+                                return false;
+                            }
+                            // ignore symsrv.dll or other server dlls in parts[2]
+                            ParseServer(2);
+                            break;
+
                         case "srv":
-                            switch (parts.Length)
-                            { 
-                                case 1:
-                                    msdl = true;
-                                    symbolCachePath = DefaultSymbolCache;
-                                    break;
-                                case 2:
-                                    symbolServerPath = parts[1];
-                                    break;
-                                case 3:
-                                    symbolCachePath = parts[1];
-                                    symbolServerPath = parts[2];
-                                    break;
-                                default:
-                                    return false;
+                            if (parts.Length <= 1)
+                            {
+                                return false;
                             }
+                            ParseServer(1);
                             break;
 
                         case "cache":
-                            switch (parts.Length)
-                            { 
-                                case 1:
-                                    symbolCachePath = DefaultSymbolCache;
-                                    break;
-                                case 2:
-                                    symbolCachePath = parts[1];
-                                    break;
-                                default:
-                                    return false;
+                            if (parts.Length <= 1)
+                            {
+                                return false;
+                            }
+                            else
+                            {
+                                for (int i = 1; i < parts.Length; i++)
+                                {
+                                    if (string.IsNullOrEmpty(parts[i]))
+                                    {
+                                        if (i == 1)
+                                        {
+                                            symbolCachePaths.Add(DefaultSymbolCache);
+                                        }
+                                    }
+                                    else 
+                                    {
+                                        symbolCachePaths.Add(parts[i]);
+                                    }
+                                }
                             }
                             break;
 
                         default:
                             // Directory path search
-                            switch (parts.Length)
+                            if (parts.Length != 1)
                             {
-                                case 1:
-                                    symbolDirectoryPath = parts[0];
-                                    break;
-                                default:
-                                    return false;
+                                return false;
                             }
+                            symbolDirectoryPath = parts[0];
                             break;
                     }
-                    if (msdl || symbolServerPath != null)
+                    if (symbolServerPath != null)
                     {
-                        if (!AddSymbolServer(msdl, symweb: false, symbolServerPath, authToken: null, timeoutInMinutes: 0))
+                        if (!AddSymbolServer(msdl: false, symweb: false, symbolServerPath.Trim(), authToken: null, timeoutInMinutes: 0))
                         {
                             return false;
                         }
                     }
-                    if (symbolCachePath != null)
+                    foreach (string symbolCachePath in symbolCachePaths.Reverse<string>())
                     {
-                        AddCachePath(symbolCachePath);
+                        AddCachePath(symbolCachePath.Trim());
                     }
                     if (symbolDirectoryPath != null)
                     {
-                        AddDirectoryPath(symbolDirectoryPath);
+                        AddDirectoryPath(symbolDirectoryPath.Trim());
                     }
                 }
             }

src/Microsoft.Diagnostics.DebugServices.Implementation/SymbolService.cs

@@ -236,7 +236,7 @@ private HResult RegisterDebuggerServices(
             hr = DebuggerServices.GetSymbolPath(out string symbolPath);
             if (hr == HResult.S_OK)
             {
-                if (!_symbolService.ParseSymbolPath(symbolPath))
+                if (!_symbolService.ParseSymbolPathFixDefault(symbolPath))
                 {
                     Trace.TraceError("ParseSymbolPath FAILED: {0}", symbolPath);
                 }

src/SOS/SOS.Extensions/HostServices.cs

@@ -21,6 +21,24 @@ public static class SymbolServiceExtensions
         // HRESULT_FROM_WIN32(ERROR_INSUFFICIENT_BUFFER) 
         const int E_INSUFFICIENT_BUFFER = unchecked((int)0x8007007a);
 
+        /// <summary>
+        /// Set the windows symbol path converting the default "srv*" to the cached public symbol server URL.
+        /// </summary>
+        /// <param name="symbolPath">The windows symbol path to translate and set</param>
+        /// <returns>if false, error parsing symbol path</returns>
+        public static bool ParseSymbolPathFixDefault(
+            this ISymbolService symbolService,
+            string symbolPath)
+        {
+            // Translate dbgeng's default .sympath to what the public version actually does. Normally "srv*" 
+            // means no caching and the server path depends on whether dbgeng is internal or public.
+            if (symbolPath.ToLowerInvariant() == "srv*")
+            {
+                symbolPath = "cache*;SRV*https://msdl.microsoft.com/download/symbols";
+            }
+            return symbolService.ParseSymbolPath(symbolPath);
+        }
+
         /// <summary>
         /// Metadata locator helper for the DAC.
         /// </summary>

src/SOS/SOS.Hosting/SymbolServiceExtensions.cs

@@ -157,16 +157,16 @@ private bool InitializeSymbolStore(
         /// <summary>
         /// Parse the Windows sympath format
         /// </summary>
-        /// <param name="windowsSymbolPath">windows symbol path</param>
+        /// <param name="symbolPath">windows symbol path</param>
         /// <returns>if false, failure</returns>
         private bool ParseSymbolPath(
             IntPtr self,
-            string windowsSymbolPath)
+            string symbolPath)
         {
-            if (windowsSymbolPath == null) {
+            if (string.IsNullOrWhiteSpace(symbolPath)) {
                 return false;
             }
-            return _symbolService.ParseSymbolPath(windowsSymbolPath);
+            return _symbolService.ParseSymbolPathFixDefault(symbolPath);
         }
 
         /// <summary>

src/SOS/SOS.Hosting/SymbolServiceWrapper.cs

@@ -16749,6 +16749,10 @@ DECLARE_API(SetSymbolServer)
     else if (loadNative)
     {
         Status = LoadNativeSymbols();
+        if (FAILED(Status))
+        {
+            ExtErr("Symbol server not set\n");
+        }
     }
 #endif
     else

src/SOS/Strike/strike.cpp

@@ -173,12 +173,11 @@ static void LoadNativeSymbolsCallback(void* param, const char* moduleFilePath, U
 \**********************************************************************/
 HRESULT LoadNativeSymbols(bool runtimeOnly)
 {
-    HRESULT hr = S_OK;
     if (g_symbolStoreInitialized)
     {
-        hr = g_ExtServices2->LoadNativeSymbols(runtimeOnly, LoadNativeSymbolsCallback);
+        return g_ExtServices2->LoadNativeSymbols(runtimeOnly, LoadNativeSymbolsCallback);
     }
-    return hr;
+    return E_FAIL;
 }
 
 #endif

src/SOS/Strike/symbols.cpp

@@ -5109,23 +5109,30 @@ GetLineByOffset(
     ___in ULONG cchFileName,
     ___in BOOL bAdjustOffsetForLineNumber /* = FALSE */)
 {
-    HRESULT Status = S_OK;
+    HRESULT status = S_OK;
     ULONG32 methodToken;
     ULONG32 methodOffs;
 
     // Find the image, method token and IL offset that correspond to "nativeOffset"
     ToRelease<IXCLRDataModule> pModule(NULL);
-    IfFailRet(ConvertNativeToIlOffset(nativeOffset, bAdjustOffsetForLineNumber, &pModule, &methodToken, &methodOffs));
-
+    status = ConvertNativeToIlOffset(nativeOffset, bAdjustOffsetForLineNumber, &pModule, &methodToken, &methodOffs);
+    if (FAILED(status))
+    {
+        ExtDbgOut("GetLineByOffset(%p): ConvertNativeToIlOffset FAILED %08x\n", nativeOffset, status);
+        return status;
+    }
     ToRelease<IMetaDataImport> pMDImport(NULL);
-    Status = pModule->QueryInterface(IID_IMetaDataImport, (LPVOID *) &pMDImport);
-    if (FAILED(Status))
+    status = pModule->QueryInterface(IID_IMetaDataImport, (LPVOID *) &pMDImport);
+    if (FAILED(status))
     {
-        ExtDbgOut("GetLineByOffset(%p): QueryInterface(IID_IMetaDataImport) FAILED %08x\n", nativeOffset, Status);
+        ExtDbgOut("GetLineByOffset(%p): QueryInterface(IID_IMetaDataImport) FAILED %08x\n", nativeOffset, status);
     }
     SymbolReader symbolReader;
-    IfFailRet(symbolReader.LoadSymbols(pMDImport, pModule));
-
+    status = symbolReader.LoadSymbols(pMDImport, pModule);
+    if (FAILED(status))
+    {
+        return status;
+    }
     return symbolReader.GetLineByILOffset(methodToken, methodOffs, pLinenum, pwszFileName, cchFileName);
 }
 

src/SOS/Strike/util.cpp

@@ -220,19 +220,23 @@ static void AddFilesFromDirectoryToTpaList(const char* directory, std::string& t
                 const char extension[] = ".dll";
                 std::string filename(find.FileName());
                 size_t extLen = sizeof(extension) - 1;
-                size_t extPos = filename.length() - extLen;
-
-                // Check if the extension matches the one we are looking for
-                if ((extPos > 0) && (filename.compare(extPos, extLen, extension) == 0))
+                size_t extPos = filename.length();
+                if (extPos > extLen)
                 {
-                    std::string filenameWithoutExt(filename.substr(0, extPos));
+                    extPos -= extLen;
 
-                    // Make sure if we have an assembly with multiple extensions present,
-                    // we insert only one version of it.
-                    if (addedAssemblies.find(filenameWithoutExt) == addedAssemblies.end())
+                    // Check if the extension matches the one we are looking for
+                    if (filename.compare(extPos, extLen, extension) == 0)
                     {
-                        addedAssemblies.insert(filenameWithoutExt);
-                        AddFileToTpaList(directory, filename.c_str(), tpaList);
+                        std::string filenameWithoutExt(filename.substr(0, extPos));
+
+                        // Make sure if we have an assembly with multiple extensions present,
+                        // we insert only one version of it.
+                        if (addedAssemblies.find(filenameWithoutExt) == addedAssemblies.end())
+                        {
+                            addedAssemblies.insert(filenameWithoutExt);
+                            AddFileToTpaList(directory, filename.c_str(), tpaList);
+                        }
                     }
                 }
             }