Check for and report corrupt PDBs (#103)

* Check for and report corrupt PDBs
* Also refactor some string literals into constants
* Bump latest release to 2022.11.05

Author: arvindshmicrosoft

Engine/DiaUtil.cs

@@ -48,12 +48,10 @@ private void ReleaseDiaObjects() {
         }
 
         /// This function builds up the PDB map, by searching for matched PDBs (based on name) and constructing the DIA session for each
-        internal static bool LocateandLoadPDBs(Dictionary<string, DiaUtil> _diautils, string userSuppliedSymPath, string symSrvSymPath, bool recurse, Dictionary<string, string> moduleNamesMap, bool cachePDB, List<string> modulesToIgnore) {
+        internal static bool LocateandLoadPDBs(string currentModule, string pdbFileName, Dictionary<string, DiaUtil> _diautils, string userSuppliedSymPath, string symSrvSymPath, bool recurse, bool cachePDB, List<string> modulesToIgnore, out string errorDetails) {
             var completeSymPath = $"{symSrvSymPath};{userSuppliedSymPath}";
-            List<string> moduleNames;
-            lock (moduleNamesMap) moduleNames = moduleNamesMap.Keys.ToList();
             // loop through each module, trying to find matched PDB files
-            foreach (string currentModule in moduleNames.Where(m => !modulesToIgnore.Contains(m) && !_diautils.ContainsKey(m))) {
+            if (!modulesToIgnore.Contains(currentModule) && !_diautils.ContainsKey(currentModule)) {
                 // we only need to search for the PDB if it does not already exist in our map
                 var cachedPDBFile = Path.Combine(Path.GetTempPath(), "SymCache", currentModule + ".pdb");
                 lock (_syncRoot) {  // the lock is needed to ensure that we do not make multiple copies of PDBs when cachePDB is true
@@ -75,8 +73,6 @@ internal static bool LocateandLoadPDBs(Dictionary<string, DiaUtil> _diautils, st
                         }
 
                         // if needed, make a last attempt looking for the original module name - but only amongst user-supplied symbol path folder(s)
-                        string pdbFileName;
-                        lock (moduleNamesMap) pdbFileName = moduleNamesMap[currentModule] + ".pdb";
                         if (!foundFiles.Any()) foreach (var currPath in userSuppliedSymPath.Split(';').Where(p => Directory.Exists(p) && !p.EndsWith(currentModule))) {
                                 foundFiles = Directory.EnumerateFiles(currPath, pdbFileName, recurse ? SearchOption.AllDirectories : SearchOption.TopDirectoryOnly);
                             }
@@ -92,10 +88,12 @@ internal static bool LocateandLoadPDBs(Dictionary<string, DiaUtil> _diautils, st
                     try {
                         _diautils.Add(currentModule, new DiaUtil(cachedPDBFile));
                     } catch (COMException) {
+                        errorDetails = cachedPDBFile;
                         return false;
                     }
                 } else if (!modulesToIgnore.Contains(currentModule)) modulesToIgnore.Add(currentModule);
             }
+            errorDetails = string.Empty;
             return true;
         }
 
@@ -127,7 +125,7 @@ internal static string GetSourceInfo(IDiaEnumLineNumbers enumLineNums, bool pdbH
             // only if we found line number information should we append to output 
             if (enumLineNums.count > 0) {
                 for (uint tmpOrdinal = 0; tmpOrdinal < enumLineNums.count; tmpOrdinal++) {
-                    if (tmpOrdinal > 0) sbOutput.Append(" -- WARNING: multiple matches -- ");
+                    if (tmpOrdinal > 0) sbOutput.Append($" {StackResolver.WARNING_PREFIX} multiple matches -- ");
                     sbOutput.Append(string.Format(CultureInfo.CurrentCulture,
                         "({0}:{1})", enumLineNums.Item(tmpOrdinal).sourceFile.fileName,
                         enumLineNums.Item(tmpOrdinal).lineNumber));
@@ -136,7 +134,7 @@ internal static string GetSourceInfo(IDiaEnumLineNumbers enumLineNums, bool pdbH
                     Marshal.FinalReleaseComObject(enumLineNums.Item(tmpOrdinal));
                 }
             }
-            else if (pdbHasSourceInfo) sbOutput.Append("-- WARNING: unable to find source info --");
+            else if (pdbHasSourceInfo) sbOutput.Append($"{StackResolver.WARNING_PREFIX} unable to find source info --");
             Marshal.FinalReleaseComObject(enumLineNums);
             return sbOutput.ToString();
         }
@@ -163,9 +161,9 @@ internal static string ProcessInlineFrames(string moduleName, bool useUndecorate
                 }
                 Marshal.FinalReleaseComObject(enumInlinees);
             } catch (COMException) {
-                sbInline.AppendLine(" -- WARNING: Unable to process inline frames; maybe symbols are mismatched?");
+                sbInline.AppendLine($" {StackResolver.WARNING_PREFIX} Unable to process inline frames; maybe symbols are mismatched?");
             } catch (System.ArgumentException) {
-                sbInline.AppendLine(" -- WARNING: Unable to process inline frames; maybe symbols are mismatched?");
+                sbInline.AppendLine($" {StackResolver.WARNING_PREFIX} Unable to process inline frames; maybe symbols are mismatched?");
             }
 
             return sbInline.ToString();

Engine/StackResolver.cs

@@ -5,6 +5,7 @@ public class StackResolver : IDisposable {
         public const string OperationCanceled = "Operation cancelled.";
         public const int OperationWaitIntervalMilliseconds = 300;
         public const int Operation100Percent = 100;
+        public const string WARNING_PREFIX = "-- WARNING:";
 
         /// This is used to store module name and start / end virtual address ranges
         /// Only populated if the user provides a tab-separated string corresponding to the output of the following SQL query:
@@ -87,7 +88,9 @@ private string ResolveSymbols(Dictionary<string, DiaUtil> _diautils, Dictionary<
                     var matchAlreadySymbolized = rgxAlreadySymbolizedFrame.Match(currentFrame);
                     if (matchAlreadySymbolized.Success) {
                         var matchedModuleName = matchAlreadySymbolized.Groups["module"].Value;
-                        if (!_diautils.ContainsKey(matchedModuleName)) DiaUtil.LocateandLoadPDBs(_diautils, userSuppliedSymPath, symSrvSymPath, searchPDBsRecursively, new Dictionary<string, string>() { { matchedModuleName, matchedModuleName } }, cachePDB, modulesToIgnore);
+                        if (!_diautils.ContainsKey(matchedModuleName) && !DiaUtil.LocateandLoadPDBs(matchedModuleName, $"{matchedModuleName}.pdb", _diautils, userSuppliedSymPath, symSrvSymPath, searchPDBsRecursively, cachePDB, modulesToIgnore, out string errorDetails)) {
+                            currentFrame += $" {WARNING_PREFIX} could not load symbol file {errorDetails}. The file may possibly be corrupt.";
+                        }
                         if (_diautils.TryGetValue(matchedModuleName, out var existingEntry) && _diautils[matchedModuleName].HasSourceInfo) {
                             var myDIAsession = existingEntry._IDiaSession;
                             myDIAsession.findChildrenEx(myDIAsession.globalScope, SymTagEnum.SymTagNull, matchAlreadySymbolized.Groups["symbolizedfunc"].Value, 0, out IDiaEnumSymbols matchedSyms);
@@ -107,7 +110,7 @@ private string ResolveSymbols(Dictionary<string, DiaUtil> _diautils, Dictionary<
                                             if (enumAllLineNums.Item(tmpOrdinalInner).addressOffset <= currAddress
                                                 && currAddress < enumAllLineNums.Item(tmpOrdinalInner).addressOffset + enumAllLineNums.Item(tmpOrdinalInner).length) {
                                                 currentFrame = $"{matchedModuleName}+{currAddress - enumAllLineNums.Item(tmpOrdinalInner).addressOffset + enumAllLineNums.Item(tmpOrdinalInner).relativeVirtualAddress:X}" 
-                                                    + (foundMatch ? " -- WARNING: ambiguous symbol; relookup might be incorrect -- " : String.Empty);
+                                                    + (foundMatch ? $" {WARNING_PREFIX}: ambiguous symbol; relookup might be incorrect -- " : String.Empty);
                                                 foundMatch = true;
                                             }
                                             Marshal.FinalReleaseComObject(enumAllLineNums.Item(tmpOrdinalInner));
@@ -125,8 +128,14 @@ private string ResolveSymbols(Dictionary<string, DiaUtil> _diautils, Dictionary<
                 var match = rgxModuleName.Match(currentFrame);
                 if (match.Success) {
                     var matchedModuleName = match.Groups["module"].Value;
-                    lock (moduleNamesMap) if (!moduleNamesMap.ContainsKey(matchedModuleName)) moduleNamesMap.Add(matchedModuleName, matchedModuleName);
-                    if (!_diautils.ContainsKey(matchedModuleName)) DiaUtil.LocateandLoadPDBs(_diautils, userSuppliedSymPath, symSrvSymPath, searchPDBsRecursively, moduleNamesMap, cachePDB, modulesToIgnore);
+                    string pdbFileName;
+                    lock (moduleNamesMap) {
+                        if (!moduleNamesMap.ContainsKey(matchedModuleName)) moduleNamesMap.Add(matchedModuleName, matchedModuleName);
+                        pdbFileName = $"{moduleNamesMap[matchedModuleName]}.pdb";
+                    }
+                    if (!_diautils.ContainsKey(matchedModuleName) && !DiaUtil.LocateandLoadPDBs(matchedModuleName, pdbFileName, _diautils, userSuppliedSymPath, symSrvSymPath, searchPDBsRecursively, cachePDB, modulesToIgnore, out string errorDetails)) {
+                        currentFrame += $" {WARNING_PREFIX} could not load symbol file {errorDetails}. The file may possibly be corrupt.";
+                    }
                     int frameNumFromInput = string.IsNullOrWhiteSpace(match.Groups["framenum"].Value) ? int.MinValue : Convert.ToInt32(match.Groups["framenum"].Value, 16);
                     if (frameNumFromInput != int.MinValue && runningFrameNum == int.MinValue) runningFrameNum = frameNumFromInput;
                     if (_diautils.ContainsKey(matchedModuleName)) {
@@ -216,7 +225,7 @@ private string ProcessFrameModuleOffset(Dictionary<string, DiaUtil> _diautils, D
                     Marshal.FinalReleaseComObject(enumLineNums);
                 }
                 var originalModuleName = moduleNamesMap.TryGetValue(moduleName, out string existingModule) ? existingModule : moduleName;
-                if (showInlineFrames && pdbHasSourceInfo && !sourceInfo.Contains("-- WARNING:")) {
+                if (showInlineFrames && pdbHasSourceInfo && !sourceInfo.Contains(WARNING_PREFIX)) {
                     inlineFrameAndSourceInfo = DiaUtil.ProcessInlineFrames(originalModuleName, useUndecorateLogic, includeOffset, includeSourceInfo, rva, mysym, pdbHasSourceInfo);
                 }
 

Engine/XELHelper.cs

@@ -67,7 +67,7 @@ await xeStream.ReadEventStream(evt => {
                         parent.PercentComplete = (int)((double)parent.globalCounter / callstackRaw.Count * 100.0);
                     }
 
-                    if (hasOverflow) xmlEquivalent.AppendLine("<!-- WARNING: output was truncated due to size limits -->");
+                    if (hasOverflow) xmlEquivalent.AppendLine($"<!{StackResolver.WARNING_PREFIX} output was truncated due to size limits -->");
                     xmlEquivalent.AppendLine("</Events>");
                     finalEventCount = callstackRaw.Count;
                 }

GUI/MainForm.cs

@@ -97,7 +97,7 @@ private void ResolveCallstacks_Click(object sender, EventArgs e) {
                     finalOutput.Text = resolverTask.Result;
                 }
 
-            if (finalOutput.Text.Contains("WARNING:")) {
+            if (finalOutput.Text.Contains(StackResolver.WARNING_PREFIX)) {
                 MessageBox.Show(this,
                     "One or more potential issues exist in the output. This is sometimes due to mismatched symbols, so please double-check symbol paths and re-run if needed.",
                     "Potential issues with the output", MessageBoxButtons.OK, MessageBoxIcon.Warning);

Tests/TestCases/downloadsyms.ps1

@@ -57,6 +57,14 @@ if (-not (test-path $localpath)) {
     dir "./TestOrdinal"
 }
 
+### CorruptPDB
+mkdir -Force "./CorruptPDB" -ErrorAction Ignore
+$localpath = "./CorruptPDB/sqldk.pdb"
+if (-not (test-path $localpath)) {
+    (Get-Content "./TestOrdinal/sqldk.pdb" -encoding byte -TotalCount 1000) | Set-Content $localpath -encoding byte
+    dir $localpath
+}
+
 ### ImportXEL
 mkdir -Force "./ImportXEL" -ErrorAction Ignore
 $localpath = "./ImportXEL/XESpins_0_131627061603030000.xel"

Tests/Tests.cs

@@ -75,6 +75,14 @@ public class Tests {
             Assert.AreEqual(expectedSymbol + Environment.NewLine + expectedSymbol, ret.Trim());
         }
 
+        /// A corrupt PDB should be reported as a warning
+        [TestMethod][TestCategory("Unit")] public async Task CorruptPDBWarning() {
+            using var csr = new StackResolver();
+            using var cts = new CancellationTokenSource();
+            var ret = await csr.ResolveCallstacksAsync(await csr.GetListofCallStacksAsync("sqldk+40609", false, cts), @"..\..\..\Tests\TestCases\CorruptPDB", false, null, false, false, false, true, false, false, null, cts);
+            Assert.IsTrue(ret.StartsWith($"sqldk+40609 {StackResolver.WARNING_PREFIX}"));
+        }
+
         /// Test the resolution of a "regular" symbol with virtual address as input.
         [TestMethod][TestCategory("Unit")] public async Task RegularSymbolVirtualAddress() {
             using var csr = new StackResolver();

latestrelease.txt

@@ -1 +1 @@
-2022-10-29 00:00
+2022-11-05 00:00