Skip to content

Commit

Permalink
Fix trimming of DebuggerDisplay with Name (dotnet#92191)
Browse files Browse the repository at this point in the history 
The `Name` and `Type` property of the `DebuggerDisplay` attribute accepts the
same format string as its `Value` property, but does not prevent
trimming members it references. Thanks to this fix, members referenced by
any of these two properties are not trimmed and can be displayed by a
debugger.
  • Loading branch information
@arturek
arturek committed Sep 20, 2023
1 parent 521e1e6 commit 0dc5903
Show file tree
Hide file tree
Showing 3 changed files with 104 additions and 52 deletions.
110 changes: 60 additions & 50 deletions src/tools/illink/src/linker/Linker.Steps/MarkStep.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2265,70 +2265,80 @@ void MarkXmlSchemaProvider (TypeDefinition type, CustomAttribute attribute)
void MarkTypeWithDebuggerDisplayAttribute (TypeDefinition type, CustomAttribute attribute)
{
if (Context.KeepMembersForDebugger) {

// Members referenced by the DebuggerDisplayAttribute are kept even if the attribute may not be.
// Record a logical dependency on the attribute so that we can blame it for the kept members below.
Tracer.AddDirectDependency (attribute, new DependencyInfo (DependencyKind.CustomAttribute, type), marked: false);

string displayString = (string) attribute.ConstructorArguments[0].Value;
if (string.IsNullOrEmpty (displayString))
return;
MarkTypeWithDebuggerDisplayAttributeValue(type, attribute, (string) attribute.ConstructorArguments[0].Value);
if (attribute.HasProperties) {
foreach (var property in attribute.Properties) {
if (property.Name is "Name" or "Type") {
MarkTypeWithDebuggerDisplayAttributeValue (type, attribute, (string) property.Argument.Value);
}
}
}
}
}

foreach (Match match in DebuggerDisplayAttributeValueRegex ().Matches (displayString)) {
// Remove '{' and '}'
string realMatch = match.Value.Substring (1, match.Value.Length - 2);
void MarkTypeWithDebuggerDisplayAttributeValue (TypeDefinition type, CustomAttribute attribute, string? displayString)
{
if (string.IsNullOrEmpty (displayString))
return;

// Remove ",nq" suffix if present
// (it asks the expression evaluator to remove the quotes when displaying the final value)
if (ContainsNqSuffixRegex ().IsMatch (realMatch)) {
realMatch = realMatch.Substring (0, realMatch.LastIndexOf (','));
}
foreach (Match match in DebuggerDisplayAttributeValueRegex ().Matches (displayString)) {
// Remove '{' and '}'
string realMatch = match.Value.Substring (1, match.Value.Length - 2);

if (realMatch.EndsWith ("()")) {
string methodName = realMatch.Substring (0, realMatch.Length - 2);
// Remove ",nq" suffix if present
// (it asks the expression evaluator to remove the quotes when displaying the final value)
if (ContainsNqSuffixRegex ().IsMatch (realMatch)) {
realMatch = realMatch.Substring (0, realMatch.LastIndexOf (','));
}

// It's a call to a method on some member. Handling this scenario robustly would be complicated and a decent bit of work.
//
// We could implement support for this at some point, but for now it's important to make sure at least we don't crash trying to find some
// method on the current type when it exists on some other type
if (methodName.Contains ('.'))
continue;
if (realMatch.EndsWith ("()")) {
string methodName = realMatch.Substring (0, realMatch.Length - 2);

MethodDefinition? method = GetMethodWithNoParameters (type, methodName);
if (method != null) {
MarkMethod (method, new DependencyInfo (DependencyKind.ReferencedBySpecialAttribute, attribute), ScopeStack.CurrentScope.Origin);
continue;
}
} else {
FieldDefinition? field = GetField (type, realMatch);
if (field != null) {
MarkField (field, new DependencyInfo (DependencyKind.ReferencedBySpecialAttribute, attribute), ScopeStack.CurrentScope.Origin);
continue;
}
// It's a call to a method on some member. Handling this scenario robustly would be complicated and a decent bit of work.
//
// We could implement support for this at some point, but for now it's important to make sure at least we don't crash trying to find some
// method on the current type when it exists on some other type
if (methodName.Contains ('.'))
continue;

PropertyDefinition? property = GetProperty (type, realMatch);
if (property != null) {
if (property.GetMethod != null) {
MarkMethod (property.GetMethod, new DependencyInfo (DependencyKind.ReferencedBySpecialAttribute, attribute), ScopeStack.CurrentScope.Origin);
}
if (property.SetMethod != null) {
MarkMethod (property.SetMethod, new DependencyInfo (DependencyKind.ReferencedBySpecialAttribute, attribute), ScopeStack.CurrentScope.Origin);
}
continue;
}
MethodDefinition? method = GetMethodWithNoParameters (type, methodName);
if (method != null) {
MarkMethod (method, new DependencyInfo (DependencyKind.ReferencedBySpecialAttribute, attribute), ScopeStack.CurrentScope.Origin);
continue;
}
} else {
FieldDefinition? field = GetField (type, realMatch);
if (field != null) {
MarkField (field, new DependencyInfo (DependencyKind.ReferencedBySpecialAttribute, attribute), ScopeStack.CurrentScope.Origin);
continue;
}

while (true) {
// Currently if we don't understand the DebuggerDisplayAttribute we mark everything on the type
// This can be improved: dotnet/linker/issues/1873
MarkMethods (type, new DependencyInfo (DependencyKind.KeptForSpecialAttribute, attribute));
MarkFields (type, includeStatic: true, new DependencyInfo (DependencyKind.ReferencedBySpecialAttribute, attribute));
if (Context.TryResolve (type.BaseType) is not TypeDefinition baseType)
break;
type = baseType;
PropertyDefinition? property = GetProperty (type, realMatch);
if (property != null) {
if (property.GetMethod != null) {
MarkMethod (property.GetMethod, new DependencyInfo (DependencyKind.ReferencedBySpecialAttribute, attribute), ScopeStack.CurrentScope.Origin);
}
if (property.SetMethod != null) {
MarkMethod (property.SetMethod, new DependencyInfo (DependencyKind.ReferencedBySpecialAttribute, attribute), ScopeStack.CurrentScope.Origin);
}
continue;
}
return;
}

while (true) {
// Currently if we don't understand the DebuggerDisplayAttribute we mark everything on the type
// This can be improved: dotnet/linker/issues/1873
MarkMethods (type, new DependencyInfo (DependencyKind.KeptForSpecialAttribute, attribute));
MarkFields (type, includeStatic: true, new DependencyInfo (DependencyKind.ReferencedBySpecialAttribute, attribute));
if (Context.TryResolve (type.BaseType) is not TypeDefinition baseType)
break;
type = baseType;
}
return;
}
}

Expand Down
20 changes: 19 additions & 1 deletion ...illink/test/Mono.Linker.Tests.Cases/Attributes.Debugger/DebuggerDisplayAttributeOnType.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ public static void Main ()
var foo = new Foo ();
var bar = new Bar ();
var baz = new Baz ();
var fooBaz = new FooBaz ();
var fooBar = new FooBar ();
}

[Kept]
Expand Down Expand Up @@ -39,5 +41,21 @@ public int Method ()
class Baz
{
}

[Kept]
[KeptMember (".ctor()")]
[DebuggerDisplay ("_", Name="{Property}")]
class FooBaz
{
public int Property { get; set; }
}

[Kept]
[KeptMember (".ctor()")]
[DebuggerDisplay ("_", Type="{Property}")]
class FooBar
{
public int Property { get; set; }
}
}
}
}
26 changes: 25 additions & 1 deletion ...Linker.Tests.Cases/Attributes.Debugger/KeepDebugMembers/DebuggerDisplayAttributeOnType.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ public static void Main ()
{
var foo = new Foo ();
var bar = new Bar ();
var fooBaz = new FooBaz ();
var fooBar = new FooBar ();
}

[Kept]
Expand All @@ -38,5 +40,27 @@ public int Method ()
return 1;
}
}

[Kept]
[KeptMember (".ctor()")]
[KeptAttributeAttribute (typeof (DebuggerDisplayAttribute))]
[DebuggerDisplay ("_", Name="{Property}")]
class FooBaz
{
[Kept]
[KeptBackingField]
public int Property { [Kept] get; [Kept] set; }
}

[Kept]
[KeptMember (".ctor()")]
[KeptAttributeAttribute (typeof (DebuggerDisplayAttribute))]
[DebuggerDisplay ("_", Type="{Property}")]
class FooBar
{
[Kept]
[KeptBackingField]
public int Property { [Kept] get; [Kept] set; }
}
}
}
}

0 comments on commit 0dc5903

Please sign in to comment.