Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[wasm][debugger] Fix debugger behavior when the type has ToString method overridden #76780

Merged
merged 11 commits into from
Oct 11, 2022
Merged

[wasm][debugger] Fix debugger behavior when the type has ToString method overridden #76780

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
62d86f2
The description of an object that has ToString implemented should be …
thaystg Oct 8, 2022
0c124b8
Fix if cannot find ToString method.
thaystg Oct 8, 2022
8dcd0d4
removing my name of the code
thaystg Oct 8, 2022
87fc1a3
Implementing support for inherited classes with ToString overriden.
thaystg Oct 9, 2022
eabbc6b
Fix test cases.
thaystg Oct 9, 2022
7d9698a
Apply suggestions from code review
thaystg Oct 10, 2022
451b4ec
Adding more tests and addressing @radical comments
thaystg Oct 10, 2022
eadebf0
Adding tests for record
thaystg Oct 10, 2022
99b2429
Moving implementation of InvokeToStringAsync to sdbhelper and reuse i…
thaystg Oct 10, 2022
d11ed8f
Apply suggestions from code review
thaystg Oct 10, 2022
794a460
Addressing @radical comments.
thaystg Oct 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 50 additions & 5 deletions src/mono/wasm/debugger/BrowserDebugProxy/JObjectValueCreator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using BrowserDebugProxy;
using Microsoft.Extensions.Logging;
using Newtonsoft.Json.Linq;
using System.Reflection;

namespace Microsoft.WebAssembly.Diagnostics;

Expand Down Expand Up @@ -265,22 +266,60 @@ async Task<string> GetNullObjectClassName()
return className;
}
}

private async Task<string> InvokeToStringAsync(List<int> typeIds, int objectId, CancellationToken token)
{
string description = "";
try {
foreach (var typeId in typeIds)
{
radical marked this conversation as resolved.
Show resolved Hide resolved
var typeInfo = await _sdbAgent.GetTypeInfo(typeId, token);
if (typeInfo == null || typeInfo.Name == "object")
continue;
{
MethodInfo methodInfo = typeInfo.Info.Methods.FirstOrDefault(m => m.Name == "ToString");
if (methodInfo != null)
{
int[] methodIds = await _sdbAgent.GetMethodIdsByName(typeId, "ToString", BindingFlags.DeclaredOnly, token);
if (methodIds != null)
{
foreach (var methodId in methodIds)
{
var methodInfoFromRuntime = await _sdbAgent.GetMethodInfo(methodId, token);
if (methodInfoFromRuntime.Info.GetParametersInfo().Length > 0)
continue;
var toString = await _sdbAgent.InvokeMethod(objectId, methodId, isValueType: false, token);
if (toString["value"]?["value"] != null)
{
description = toString["value"]?["value"].Value<string>();
}
}
}
break;
}
}
}
}
catch (Exception e)
{
_logger.LogDebug($"Error while evaluating ToString method: {e}");
}
return description;
}
private async Task<JObject> ReadAsObjectValue(MonoBinaryReader retDebuggerCmdReader, int typeIdFromAttribute, bool forDebuggerDisplayAttribute, CancellationToken token)
{
var objectId = retDebuggerCmdReader.ReadInt32();
var type_id = await _sdbAgent.GetTypeIdsForObject(objectId, false, token);
string className = await _sdbAgent.GetTypeName(type_id[0], token);
var typeIds = await _sdbAgent.GetTypeIdsForObject(objectId, withParents: true, token);
string className = await _sdbAgent.GetTypeName(typeIds[0], token);
string debuggerDisplayAttribute = null;
if (!forDebuggerDisplayAttribute)
debuggerDisplayAttribute = await _sdbAgent.GetValueFromDebuggerDisplayAttribute(
new DotnetObjectId("object", objectId), type_id[0], token);
new DotnetObjectId("object", objectId), typeIds[0], token);
var description = className.ToString();

if (debuggerDisplayAttribute != null)
description = debuggerDisplayAttribute;

thaystg marked this conversation as resolved.
Show resolved Hide resolved
if (await _sdbAgent.IsDelegate(objectId, token))
else if (await _sdbAgent.IsDelegate(objectId, token))
{
if (typeIdFromAttribute != -1)
{
Expand All @@ -293,6 +332,12 @@ private async Task<JObject> ReadAsObjectValue(MonoBinaryReader retDebuggerCmdRea
return Create(value: className, type: "symbol", description: className);
}
}
else
{
var toString = await InvokeToStringAsync(typeIds, objectId, token);
if (toString != "")
description = toString;
}
return Create<object>(value: null, type: "object", description: description, className: className, objectId: $"dotnet:object:{objectId}");
}

Expand Down
7 changes: 4 additions & 3 deletions src/mono/wasm/debugger/BrowserDebugProxy/MemberReferenceResolver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
using System.Net.WebSockets;
using BrowserDebugProxy;
using System.Globalization;
using System.Reflection;

namespace Microsoft.WebAssembly.Diagnostics
{
Expand Down Expand Up @@ -458,7 +459,7 @@ public async Task<JObject> Resolve(ElementAccessExpressionSyntax elementAccess,
return await ExpressionEvaluator.EvaluateSimpleExpression(this, eaFormatted, elementAccessStr, variableDefinitions, logger, token);
}
var typeIds = await context.SdbAgent.GetTypeIdsForObject(objectId.Value, true, token);
int[] methodIds = await context.SdbAgent.GetMethodIdsByName(typeIds[0], "ToArray", token);
int[] methodIds = await context.SdbAgent.GetMethodIdsByName(typeIds[0], "ToArray", BindingFlags.Default, token);
// ToArray should not have an overload, but if user defined it, take the default one: without params
if (methodIds == null)
throw new InvalidOperationException($"Type '{rootObject?["className"]?.Value<string>()}' cannot be indexed.");
Expand Down Expand Up @@ -560,7 +561,7 @@ public async Task<JObject> Resolve(InvocationExpressionSyntax method, Dictionary
{
typeIds = await context.SdbAgent.GetTypeIdsForObject(objectId.Value, true, token);
}
int[] methodIds = await context.SdbAgent.GetMethodIdsByName(typeIds[0], methodName, token);
int[] methodIds = await context.SdbAgent.GetMethodIdsByName(typeIds[0], methodName, BindingFlags.Default, token);
if (methodIds == null)
{
//try to search on System.Linq.Enumerable
Expand Down Expand Up @@ -666,7 +667,7 @@ async Task<int> FindMethodIdOnLinqEnumerable(IList<int> typeIds, string methodNa
}
}

int[] newMethodIds = await context.SdbAgent.GetMethodIdsByName(linqTypeId, methodName, token);
int[] newMethodIds = await context.SdbAgent.GetMethodIdsByName(linqTypeId, methodName, BindingFlags.Default, token);
if (newMethodIds == null)
return 0;

Expand Down
8 changes: 4 additions & 4 deletions src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -787,7 +787,7 @@ internal sealed class MonoSDBHelper
private DebugStore store;
private SessionId sessionId;

private readonly ILogger logger;
internal readonly ILogger logger;
private static readonly Regex regexForAsyncLocals = new (@"\<([^)]*)\>", RegexOptions.Singleline);
private static readonly Regex regexForAsyncMethodName = new (@"\<([^>]*)\>([d][_][_])([0-9]*)", RegexOptions.Compiled);
private static readonly Regex regexForGenericArgs = new (@"[`][0-9]+", RegexOptions.Compiled);
Expand Down Expand Up @@ -1737,15 +1737,15 @@ public async Task<int> GetTypeIdFromToken(int assemblyId, int typeToken, Cancell
return retDebuggerCmdReader.ReadInt32();
}

public async Task<int[]> GetMethodIdsByName(int type_id, string method_name, CancellationToken token)
public async Task<int[]> GetMethodIdsByName(int type_id, string method_name, BindingFlags extraFlags, CancellationToken token)
{
if (type_id <= 0)
throw new DebuggerAgentException($"Invalid type_id {type_id} (method_name: {method_name}");

using var commandParamsWriter = new MonoBinaryWriter();
commandParamsWriter.Write((int)type_id);
commandParamsWriter.Write(method_name);
commandParamsWriter.Write((int)(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Static));
commandParamsWriter.Write((int)(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Static | extraFlags));
commandParamsWriter.Write((int)1); //case sensitive
using var retDebuggerCmdReader = await SendDebuggerAgentCommand(CmdType.GetMethodsByNameFlags, commandParamsWriter, token);
var nMethods = retDebuggerCmdReader.ReadInt32();
Expand Down Expand Up @@ -2169,7 +2169,7 @@ private async Task<int> FindDebuggerProxyConstructorIdFor(int typeId, Cancellati
break;
cAttrTypeId = genericTypeId;
}
int[] methodIds = await GetMethodIdsByName(cAttrTypeId, ".ctor", token);
int[] methodIds = await GetMethodIdsByName(cAttrTypeId, ".ctor", BindingFlags.Default, token);
if (methodIds != null)
methodId = methodIds[0];
break;
Expand Down
47 changes: 42 additions & 5 deletions src/mono/wasm/debugger/BrowserDebugProxy/ValueTypeClass.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using System.Threading.Tasks;
using Microsoft.WebAssembly.Diagnostics;
using Newtonsoft.Json.Linq;
using Microsoft.Extensions.Logging;

namespace BrowserDebugProxy
{
Expand Down Expand Up @@ -111,16 +112,46 @@ JObject GetFieldWithMetadata(FieldTypeClass field, JObject fieldValue, bool isSt
}
}

public async Task<string> InvokeToStringAsync(MonoSDBHelper sdbAgent, CancellationToken token)
{
var typeInfo = await sdbAgent.GetTypeInfo(TypeId, token);
if (typeInfo == null)
return null;
if (typeInfo.Name == "object")
return null;
Microsoft.WebAssembly.Diagnostics.MethodInfo methodInfo = typeInfo.Info.Methods.FirstOrDefault(m => m.Name == "ToString");
if (!IsEnum && methodInfo == null)
Copy link
Member

Choose a reason for hiding this comment

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

Should this be ||?

Copy link
Member

Choose a reason for hiding this comment

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

Can this method exit early in case of IsEnum==true?

Copy link
Member Author

Choose a reason for hiding this comment

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

No should be &&, if it's a enum we want to call ToString even if we cannot find the method.

Copy link
Member

Choose a reason for hiding this comment

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

We don't get the method in case of an enum? Is that the case for other valuetypes? record struct?

Copy link
Member Author

Choose a reason for hiding this comment

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

Only for enum, I added tests for record also.

Copy link
Member

Choose a reason for hiding this comment

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

Are no methods returned for enums at all? Is that a debugger-agent thing?

Copy link
Member Author

@thaystg thaystg Oct 10, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

No methods returned for enums at all, not sure if it's a debugger-agent thing or if they are really not implemented in the enum, they are somewhere like in a "base class".

return null;
int[] methodIds = await sdbAgent.GetMethodIdsByName(TypeId, "ToString", IsEnum ? BindingFlags.Default : BindingFlags.DeclaredOnly, token);
if (methodIds == null)
return null;
try {
foreach (var methodId in methodIds)
{
var methodInfoFromRuntime = await sdbAgent.GetMethodInfo(methodId, token);
if (methodInfoFromRuntime.Info.GetParametersInfo().Length > 0)
continue;
var retMethod = await sdbAgent.InvokeMethod(Buffer, methodId, token, "methodRet");
return retMethod["value"]?["value"].Value<string>();
}
}
catch (Exception e)
{
sdbAgent.logger.LogDebug($"Error while evaluating ToString method: {e}");
}
return null;
}

public async Task<JObject> ToJObject(MonoSDBHelper sdbAgent, bool forDebuggerDisplayAttribute, CancellationToken token)
{
string description = className;
if (ShouldAutoInvokeToString(className) || IsEnum)
radical marked this conversation as resolved.
Show resolved Hide resolved
{
int[] methodIds = await sdbAgent.GetMethodIdsByName(TypeId, "ToString", token);
if (methodIds == null)
throw new InternalErrorException($"Cannot find method 'ToString' on typeId = {TypeId}");
var retMethod = await sdbAgent.InvokeMethod(Buffer, methodIds[0], token, "methodRet");
description = retMethod["value"]?["value"].Value<string>();
var toString = await InvokeToStringAsync(sdbAgent, token);
if (toString == null)
sdbAgent.logger.LogDebug($"Error while evaluating ToString method on typeId = {TypeId}");
else
description = toString;
if (className.Equals("System.Guid"))
description = description.ToUpperInvariant(); //to keep the old behavior
}
Expand All @@ -129,6 +160,12 @@ public async Task<JObject> ToJObject(MonoSDBHelper sdbAgent, bool forDebuggerDis
string displayString = await sdbAgent.GetValueFromDebuggerDisplayAttribute(Id, TypeId, token);
if (displayString != null)
description = displayString;
thaystg marked this conversation as resolved.
Show resolved Hide resolved
else
{
var toString = await InvokeToStringAsync(sdbAgent, token);
if (toString != null)
description = toString;
}
}
return JObjectValueCreator.Create(
IsEnum ? fields[0]["value"] : null,
Expand Down
32 changes: 32 additions & 0 deletions src/mono/wasm/debugger/DebuggerTestSuite/CustomViewTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,5 +106,37 @@ async Task<bool> CheckProperties(JObject pause_location)
Assert.True(task.Result);
}
}

[ConditionalFact(nameof(RunningOnChrome))]
public async Task InspectObjectOfTypeWithToStringOverriden()
{
var expression = $"{{ invoke_static_method('[debugger-test] ToStringOverriden:Run'); }}";

await EvaluateAndCheck(
"window.setTimeout(function() {" + expression + "; }, 1);",
"dotnet://debugger-test.dll/debugger-test.cs", 1505, 8,
"ToStringOverriden.Run",
wait_for_event_fn: async (pause_location) =>
{
var id = pause_location["callFrames"][0]["callFrameId"].Value<string>();
await EvaluateOnCallFrameAndCheck(id,
("a", TObject("ToStringOverriden", description:"helloToStringOverriden")),
("b", TObject("ToStringOverriden.ToStringOverridenB", description:"helloToStringOverridenA")),
("c", TObject("ToStringOverriden.ToStringOverridenD", description:"helloToStringOverridenD")),
("d", TObject("ToStringOverriden.ToStringOverridenE", description:"helloToStringOverridenE")),
("e", TObject("ToStringOverriden.ToStringOverridenB", description:"helloToStringOverridenA")),
("f", TObject("ToStringOverriden.ToStringOverridenB", description:"helloToStringOverridenA")),
("g", TObject("ToStringOverriden.ToStringOverridenG", description:"helloToStringOverridenG")),
("h", TObject("ToStringOverriden.ToStringOverridenH", description:"helloToStringOverridenH")),
("i", TObject("ToStringOverriden.ToStringOverridenI", description:"ToStringOverriden.ToStringOverridenI")),
("j", TObject("ToStringOverriden.ToStringOverridenJ", description:"helloToStringOverridenJ")),
("k", TObject("ToStringOverriden.ToStringOverridenK", description:"ToStringOverriden.ToStringOverridenK")),
("l", TObject("ToStringOverriden.ToStringOverridenL", description:"helloToStringOverridenL")),
("m", TObject("ToStringOverriden.ToStringOverridenM", description:"ToStringOverridenM { }")),
("n", TObject("ToStringOverriden.ToStringOverridenN", description:"helloToStringOverridenN"))
);
}
);
}
}
}
2 changes: 1 addition & 1 deletion src/mono/wasm/debugger/DebuggerTestSuite/MiscTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -713,7 +713,7 @@ public async Task PreviousFrameForAReflectedCall() => await CheckInspectLocalsAt

await CheckProps(frame_locals, new
{
mi = TObject("System.Reflection.RuntimeMethodInfo"), //this is what is returned when debugging desktop apps using VS
mi = TObject("System.Reflection.RuntimeMethodInfo", description: "Void SimpleStaticMethod(System.DateTime, System.String)"), //this is what is returned when debugging desktop apps using VS
dt = TDateTime(new DateTime(4210, 3, 4, 5, 6, 7)),
i = TNumber(4),
strings = TArray("string[]", "string[1]"),
Expand Down
Loading