Skip to content

Implement natvis 'na' modifier and $T substitution#1612

Open
SachinM123 wants to merge 1 commit into
microsoft:mainfrom
SachinM123:natvisFormatSpecifierFix
Open

Implement natvis 'na' modifier and $T substitution#1612
SachinM123 wants to merge 1 commit into
microsoft:mainfrom
SachinM123:natvisFormatSpecifierFix

Conversation

@SachinM123

Copy link
Copy Markdown
Collaborator

Add support for the natvis 'na' (no-address) modifier to strip MI's leading address prefix from string values. This requires tracking the format specifier through variable initialization and applying cleanup at value retrieval points.

Also implement template parameter substitution ($T1, $T2, etc.) within natvis brace expressions before format specifier extraction.

Add support for the natvis 'na' (no-address) modifier to strip MI's leading address prefix from string values. This requires tracking the format specifier through variable initialization and applying cleanup at value retrieval points.

Also implement template parameter substitution ($T1, $T2, etc.) within natvis brace expressions before format specifier extraction.
TypeName = results.TryFindString("type");
Value = results.TryFindString("value");
// Diagnostic: log raw tuple child value before any cleanup
_debuggedProcess.Logger?.WriteLine(LogLevel.Verbose, FormattableString.Invariant($"TupleCtor: name={Name}, exp={results.TryFindString("exp")}, type={TypeName}, format={_format}, formatHasNa={_formatHasNa}, rawValue={results.TryFindString("value")}"));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should already be in the MI log. Is there value in keeping them?

_debuggedProcess.Logger?.WriteLine(LogLevel.Verbose, FormattableString.Invariant($"TupleCtor: name={Name}, exp={results.TryFindString("exp")}, type={TypeName}, format={_format}, formatHasNa={_formatHasNa}, rawValue={results.TryFindString("value")}"));
// Only strip the leading MI address prefix ("0x... \"\"") when the natvis
// format included the 'na' modifier.
if (_formatHasNa && !string.IsNullOrEmpty(Value) && Regex.IsMatch(Value, "^0x[0-9a-fA-F]+\\s+"))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"

You want a non-space character after the spaces

_debuggedProcess.Logger?.WriteLine(LogLevel.Verbose, FormattableString.Invariant($"TupleCtor: name={Name}, exp={results.TryFindString("exp")}, type={TypeName}, format={_format}, formatHasNa={_formatHasNa}, rawValue={results.TryFindString("value")}"));
// Only strip the leading MI address prefix ("0x... \"\"") when the natvis
// format included the 'na' modifier.
if (_formatHasNa && !string.IsNullOrEmpty(Value) && Regex.IsMatch(Value, "^0x[0-9a-fA-F]+\\s+"))

@gregg-miskelly gregg-miskelly Jul 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"

Since we will be using the Regex often, probably better to move it to a static variable. See s_addressPattern for an example, though I don't think you need to bother making this a Lazy

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR extends MIEngine’s NatVis evaluator to (1) support the na (no-address) modifier by stripping MI’s leading 0x... address prefix from certain evaluated values, and (2) perform $T1, $T2, … template-parameter substitution inside NatVis brace expressions before parsing format specifiers.

Changes:

  • Add $Tn macro substitution over full {...} brace expressions in Natvis display-string formatting.
  • Detect and apply na address-prefix stripping during NatVis display-string formatting and during variable evaluation/initialization by tracking an _formatHasNa flag.
  • Update NatVis string “cleanup” helpers and related comments around address-prefix stripping behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
src/MIDebugEngine/Natvis.Impl/Natvis.cs Adds $Tn substitution in brace expressions and introduces na detection/cleanup in display-string formatting; updates format-specifier helpers and string cleanup docs.
src/MIDebugEngine/Engine.Impl/Variables.cs Tracks na in parsed format specifiers and attempts to strip MI’s address prefix at variable construction/evaluation time.

Comment on lines +1297 to 1299
bool hasNa = HasNaModifier(rawExpr);
string spec = ExtractFormatSpecifier(rawExpr);
string exprValue = GetExpressionValue(rawExpr, variable, scopedNames, intrinsics);
Comment on lines 1539 to 1545
internal static string CleanUtf16StringValue(string value)
{
if (string.IsNullOrEmpty(value)) return value;
// Strip leading "0x<hex> " address prefix emitted by GDB/LLDB.
value = s_addressPrefix.Replace(value, "");
// Strip surrounding u"..." or U"..." quotes.
if (value.Length >= 3 &&
(value.StartsWith("u\"", StringComparison.Ordinal) || value.StartsWith("U\"", StringComparison.Ordinal)))
{
value = value.EndsWith("\"", StringComparison.Ordinal)
? value.Substring(2, value.Length - 3)
: value.Substring(2);
}
return value;
}
Comment on lines +1552 to +1554
/// This method strips the leading address prefix that GDB/LLDB emits ("0x... ").
/// It does NOT remove surrounding quotes; callers should decide whether quotes
/// should be removed based on the caller's context.
Comment on lines 274 to +286
TypeName = results.TryFindString("type");
Value = results.TryFindString("value");
// Diagnostic: log raw tuple child value before any cleanup
_debuggedProcess.Logger?.WriteLine(LogLevel.Verbose, FormattableString.Invariant($"TupleCtor: name={Name}, exp={results.TryFindString("exp")}, type={TypeName}, format={_format}, formatHasNa={_formatHasNa}, rawValue={results.TryFindString("value")}"));
// Only strip the leading MI address prefix ("0x... \"\"") when the natvis
// format included the 'na' modifier.
if (_formatHasNa && !string.IsNullOrEmpty(Value) && Regex.IsMatch(Value, "^0x[0-9a-fA-F]+\\s+"))
{
// Recognize typical GDB prefix: 0x<hex> <string>
string before = Value;
Value = Regex.Replace(Value, "^0x[0-9a-fA-F]+\\s+", "");
_debuggedProcess.Logger?.WriteLine(LogLevel.Verbose, FormattableString.Invariant($"TupleCtor: stripped address prefix: before={before}, after={Value}"));
}
// Recognize typical GDB prefix: 0x<hex> <string>
string before = Value;
Value = Regex.Replace(Value, "^0x[0-9a-fA-F]+\\s+", "");
_debuggedProcess.Logger?.WriteLine(LogLevel.Verbose, FormattableString.Invariant($"TupleCtor: stripped address prefix: before={before}, after={Value}"));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_debuggedProcess

Remove

string expFS = exp.Substring(lastComma + 1).Trim();
// Detect whether the natvis 'na' modifier is present in the original format specifier.
// We must detect this before we strip modifiers below.
_formatHasNa = expFS.IndexOf("na", StringComparison.Ordinal) >= 0;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_formatHasNa

The other outputs from this method are return as out params/return value, so I you want this as an out bool formatNa to go along them them.

}
Value = results.TryFindString("value");
// If natvis requested 'na', strip MI's leading address prefix
if (_formatHasNa && !string.IsNullOrEmpty(Value) && Regex.IsMatch(Value, "^0x[0-9a-fA-F]+\\s+"))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"^0x[0-9a-fA-F]+\s+"

Same suggestions on the other usage of the Regex

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, better yet: can you extract this into a helper method so you don't repeat it three times?

// expression (this covers both the expression and any trailing format specifier)
if (scopedNames != null)
{
rawExpr = Regex.Replace(rawExpr, "\\$T\\d+", (Match mt) =>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"\$T\d+"

nit: this would be easier to read as @"\$T\d+"

return mt.Value;
});
}
bool hasNa = HasNaModifier(rawExpr);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bool hasNa = HasNaModifier(rawExpr);

Instead of duplicating the work of ExtractFormatSpecifier, have ExtractFormatSpecifier return this as an out param

if (spec == "sub" || spec == "su")
exprValue = CleanUtf16StringValue(exprValue);
else if (spec == "sb")
exprValue = CleanAsciiStringValue(exprValue);

@gregg-miskelly gregg-miskelly Jul 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean to loose this sub/su/sb code?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, is su needed? Variabel.cs line 477 already handles su correctly.

value = value.EndsWith("\"", StringComparison.Ordinal)
? value.Substring(2, value.Length - 3)
: value.Substring(2);
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

,sub is supposed to remove the quotes/prefix

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though this changes looks like it is correct for ,su

return replacement;
return mt.Value;
});
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I am reading the code correctly, this is already at least partially done by ReplaceNamesInExpression, which is called by GetExpressionValue, so I don't think you want to do this over the entire expression.

Can you trace through why ReplaceNamesInExpression isn't doing this already? (lines 1713-1717).

@gregg-miskelly

Copy link
Copy Markdown
Member

Can you add a test for your changes? You may also need to adjust a test baseline for su since the code was doing the wrong thing there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants