Skip to content

Commit

Permalink
Fix bug in relookup source code path (#23)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@arvindshmicrosoft
arvindshmicrosoft authored Feb 9, 2022
1 parent ec2ea6f commit a4747e3
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 28 deletions.
8 changes: 4 additions & 4 deletions Engine/DiaUtil.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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);
Expand Down Expand Up @@ -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();
Expand Down
45 changes: 22 additions & 23 deletions Engine/StackResolver.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,39 +94,38 @@ private string ResolveSymbols(Dictionary<string, DiaUtil> _diautils, string[] ca
DiaUtil.LocateandLoadPDBs(_diautils, tp.symPath, tp.searchPDBsRecursively, new List<string>() { 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;
}
}

Expand Down Expand Up @@ -229,19 +228,19 @@ private string ProcessFrameModuleOffset(Dictionary<string, DiaUtil> _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();
Expand Down
2 changes: 1 addition & 1 deletion GUI/MainForm.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down

0 comments on commit a4747e3

Please sign in to comment.