From a4747e339cc0eef51a57331043d3d775e8c05854 Mon Sep 17 00:00:00 2001 From: Arvind Shyamsundar Date: Tue, 8 Feb 2022 18:05:18 -0800 Subject: [PATCH] Fix bug in relookup source code path (#23) When re-looking up frames to include source info, we now correctly use the starting address of the function, and offset into the function, and then compare to see which line covers that computed address, and then use the RVA for that line of code to "re-write" the frame in the module+RVA notation. The earlier implementation used the RVA of the function to add the offset to, which is incorrect in cases where there was re-ordering of chunks of machine code done by the compiler in the process of optimization. Address offset for each function is stable in these cases as well, and thereby using the mapping offered by the PDB to get back the actual (sometimes re-located) RVA based on this logic, is correct behavior. This change also streamlines the "re-lookup" code path to just re-write the frame as module+RVA, and thereby allowing other downstream processing (such as looking up inlinees, etc.) to reuse existing code. --- Engine/DiaUtil.cs | 8 ++++---- Engine/StackResolver.cs | 45 ++++++++++++++++++++--------------------- GUI/MainForm.cs | 2 +- 3 files changed, 27 insertions(+), 28 deletions(-) diff --git a/Engine/DiaUtil.cs b/Engine/DiaUtil.cs index 9a71461..51a2280 100644 --- a/Engine/DiaUtil.cs +++ b/Engine/DiaUtil.cs @@ -134,7 +134,7 @@ internal static string GetSourceInfo(IDiaEnumLineNumbers enumLineNums, bool pdbH if (enumLineNums.count > 0) { for (uint tmpOrdinal = 0; tmpOrdinal < enumLineNums.count; tmpOrdinal++) { if (tmpOrdinal > 0) { - sbOutput.Append(" -- WARN: multiple matches -- "); + sbOutput.Append(" -- WARNING: multiple matches -- "); } sbOutput.Append(string.Format(CultureInfo.CurrentCulture, @@ -147,7 +147,7 @@ internal static string GetSourceInfo(IDiaEnumLineNumbers enumLineNums, bool pdbH } else { if (pdbHasSourceInfo) { - sbOutput.Append("-- WARN: unable to find source info --"); + sbOutput.Append("-- WARNING: unable to find source info --"); } } Marshal.FinalReleaseComObject(enumLineNums); @@ -175,9 +175,9 @@ internal static string ProcessInlineFrames(string moduleName, bool useUndecorate } Marshal.ReleaseComObject(enumInlinees); } catch (COMException) { - sbInline.AppendLine(" -- WARN: Unable to process inline frames; maybe symbols are mismatched?"); + sbInline.AppendLine(" -- WARNING: Unable to process inline frames; maybe symbols are mismatched?"); } catch (System.ArgumentException) { - sbInline.AppendLine(" -- WARN: Unable to process inline frames; maybe symbols are mismatched?"); + sbInline.AppendLine(" -- WARNING: Unable to process inline frames; maybe symbols are mismatched?"); } return sbInline.ToString(); diff --git a/Engine/StackResolver.cs b/Engine/StackResolver.cs index b3f66b1..2a4e997 100644 --- a/Engine/StackResolver.cs +++ b/Engine/StackResolver.cs @@ -94,39 +94,38 @@ private string ResolveSymbols(Dictionary _diautils, string[] ca DiaUtil.LocateandLoadPDBs(_diautils, tp.symPath, tp.searchPDBsRecursively, new List() { matchedModuleName }, tp.cachePDB, modulesToIgnore); } - if (_diautils.ContainsKey(matchedModuleName)) { + if (_diautils.ContainsKey(matchedModuleName) && _diautils[matchedModuleName].HasSourceInfo) { var myDIAsession = _diautils[matchedModuleName]._IDiaSession; myDIAsession.findChildrenEx(myDIAsession.globalScope, SymTagEnum.SymTagNull, matchAlreadySymbolized.Groups["symbolizedfunc"].Value, 0, out IDiaEnumSymbols matchedSyms); + var foundMatch = false; if (matchedSyms.count > 0) { for (uint tmpOrdinal = 0; tmpOrdinal < matchedSyms.count; tmpOrdinal++) { IDiaSymbol tmpSym = matchedSyms.Item(tmpOrdinal); - var rva = tmpSym.relativeVirtualAddress; - string offsetString = matchAlreadySymbolized.Groups["offset"].Value; int numberBase = offsetString.ToUpperInvariant().StartsWith("0X", StringComparison.CurrentCulture) ? 16 : 10; - uint offset = Convert.ToUInt32(offsetString, numberBase); - rva += offset; - myDIAsession.findLinesByRVA(rva, 0, out IDiaEnumLineNumbers enumLineNums); - string tmpsourceInfo = DiaUtil.GetSourceInfo(enumLineNums, - _diautils[matchedModuleName].HasSourceInfo); - - if (tmpOrdinal > 0) { - finalCallstack.Append(" OR "); + var currAddress = (uint)(tmpSym.addressOffset + Convert.ToUInt32(offsetString, numberBase)); + + string tmpsourceInfo = String.Empty; + myDIAsession.findLinesByRVA(tmpSym.relativeVirtualAddress, (uint)tmpSym.length, out IDiaEnumLineNumbers enumAllLineNums); + if (enumAllLineNums.count > 0) { + for (uint tmpOrdinalInner = 0; tmpOrdinalInner < enumAllLineNums.count; tmpOrdinalInner++) { + // below, we search for a line of code whose address range covers the current address of interest, and if matched, we re-write the current line in the module+RVA format + 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 = true; + } + Marshal.FinalReleaseComObject(enumAllLineNums.Item(tmpOrdinalInner)); + } } - finalCallstack.AppendFormat(CultureInfo.CurrentCulture, "{0}!{1}{2}\t{3}", matchedModuleName, - matchAlreadySymbolized.Groups["symbolizedfunc"].Value, includeOffsets ? "+" + offsetString : string.Empty, tmpsourceInfo); Marshal.ReleaseComObject(tmpSym); } Marshal.ReleaseComObject(matchedSyms); } - } else { - // in the rare case that the symbol does not exist, return frame as-is - finalCallstack.Append(currentFrame); } - finalCallstack.AppendLine(); - continue; } } @@ -229,19 +228,19 @@ private string ProcessFrameModuleOffset(Dictionary _diautils, r return null; } - // try to find if we have source and line number info and include it based on the param - string sourceInfo = string.Empty; + string sourceInfo = string.Empty; // try to find if we have source and line number info and include it based on the param + string inlineFrameAndSourceInfo = string.Empty; // Process inline functions, but only if private PDBs are in use var pdbHasSourceInfo = _diautils[moduleName].HasSourceInfo; if (includeSourceInfo) { _diautils[moduleName]._IDiaSession.findLinesByRVA(rva, 0, out IDiaEnumLineNumbers enumLineNums); sourceInfo = DiaUtil.GetSourceInfo(enumLineNums, pdbHasSourceInfo); } - // Process inline functions, but only if private PDBs are in use - string inlineFrameAndSourceInfo = string.Empty; - if (showInlineFrames && pdbHasSourceInfo) { + if (showInlineFrames && pdbHasSourceInfo && !sourceInfo.Contains("-- WARNING:")) { inlineFrameAndSourceInfo = DiaUtil.ProcessInlineFrames(moduleName, useUndecorateLogic, includeOffset, includeSourceInfo, rva, mysym, pdbHasSourceInfo); } + var symbolizedFrame = DiaUtil.GetSymbolizedFrame(moduleName, mysym, useUndecorateLogic, includeOffset, displacement, false); + // make sure we cleanup COM allocations for the resolved sym Marshal.FinalReleaseComObject(mysym); result = (inlineFrameAndSourceInfo + symbolizedFrame + "\t" + sourceInfo).Trim(); diff --git a/GUI/MainForm.cs b/GUI/MainForm.cs index ac69e6d..c48c437 100644 --- a/GUI/MainForm.cs +++ b/GUI/MainForm.cs @@ -133,7 +133,7 @@ private void ResolveCallstacks_Click(object sender, EventArgs e) { finalOutput.Text = backgroundTask.Result; - if (backgroundTask.Result.Contains("-- WARN:")) { + if (backgroundTask.Result.Contains("WARNING:")) { 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",