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] Support indexing of valuetype scheme #92637

Merged
merged 14 commits into from
Oct 12, 2023
Merged

[wasm][debugger] Support indexing of valuetype scheme #92637

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
6d988af
Prepare test cases for signle dim.
ilonatommy Sep 26, 2023
8c99fd3
Fixed for constant indices.
ilonatommy Sep 26, 2023
116200b
More tests + fix.
ilonatommy Sep 26, 2023
ec15434
Unblock, working after rebuild.
ilonatommy Sep 26, 2023
c5e8792
Merge branch 'main' into index-a-struct
ilonatommy Sep 27, 2023
4b3e6c1
Initial inlined arrays cleanup.
ilonatommy Sep 27, 2023
21841f7
Lines are shifted again.
ilonatommy Sep 27, 2023
a8a2ee4
@thaystg's fix for inline arrays
ilonatommy Sep 28, 2023
52dd1f8
Added test for method on inline array.
ilonatommy Sep 28, 2023
6781416
Multi-dim works as well.
ilonatommy Sep 28, 2023
c4595c4
fix
ilonatommy Sep 28, 2023
39ace3a
Tentative fix from feedback.
ilonatommy Sep 29, 2023
98c6db3
0th element is decoded before the loop.
ilonatommy Oct 3, 2023
5d41e6a
Feedback part 1.
ilonatommy Oct 5, 2023
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
20 changes: 18 additions & 2 deletions src/mono/mono/component/debugger-agent.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5353,14 +5353,15 @@ decode_vtype (MonoType *t, MonoDomain *domain, gpointer void_addr, gpointer void
gpointer iter = NULL;
MonoDomain *d;
ErrorCode err;
int inlineArraySize = -1;

/* is_enum, ignored */
decode_byte (buf, &buf, limit);
if (CHECK_PROTOCOL_VERSION(2, 61))
decode_byte (buf, &buf, limit);
klass = decode_typeid (buf, &buf, limit, &d, &err);
if (CHECK_PROTOCOL_VERSION(2, 65))
decode_int (buf, &buf, limit); //ignore inline array
inlineArraySize = decode_int (buf, &buf, limit);
if (err != ERR_NONE)
return err;

Expand All @@ -5385,6 +5386,12 @@ decode_vtype (MonoType *t, MonoDomain *domain, gpointer void_addr, gpointer void
if (err != ERR_NONE)
return err;
nfields --;
if (CHECK_PROTOCOL_VERSION(2, 66) && inlineArraySize > 0)
{
int element_size = mono_class_instance_size (mono_class_from_mono_type_internal (f->type)) - MONO_ABI_SIZEOF (MonoObject);
for (int i = 1; i < inlineArraySize; i++)
decode_value (f->type, domain, ((char*)mono_vtype_get_field_addr (addr, f)) + (i*element_size), buf, &buf, limit, check_field_datatype, extra_space, members_in_extra_space);
}
}
g_assert (nfields == 0);

Expand Down Expand Up @@ -5459,14 +5466,15 @@ decode_vtype_compute_size (MonoType *t, MonoDomain *domain, gpointer void_buf, g
gpointer iter = NULL;
MonoDomain *d;
ErrorCode err;
int inlineArraySize = -1;

/* is_enum, ignored */
decode_byte (buf, &buf, limit);
if (CHECK_PROTOCOL_VERSION(2, 61))
decode_byte (buf, &buf, limit);
klass = decode_typeid (buf, &buf, limit, &d, &err);
if (CHECK_PROTOCOL_VERSION(2, 65))
decode_int (buf, &buf, limit); //ignore inline array
inlineArraySize = decode_int (buf, &buf, limit);
if (err != ERR_NONE)
goto end;

Expand All @@ -5489,6 +5497,14 @@ decode_vtype_compute_size (MonoType *t, MonoDomain *domain, gpointer void_buf, g
if (err != ERR_NONE)
return err;
nfields --;
if (CHECK_PROTOCOL_VERSION(2, 66) && inlineArraySize > 0)
{
for (int i = 1; i < inlineArraySize; i++) {
field_size = decode_value_compute_size (f->type, 0, domain, buf, &buf, limit, members_in_extra_space);
if (members_in_extra_space)
ret += field_size;
}
}
}
g_assert (nfields == 0);

Expand Down
115 changes: 82 additions & 33 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 @@ -408,6 +408,12 @@ public async Task<JObject> Resolve(
if (!context.SdbAgent.ValueCreator.TryGetValueTypeById(objectId.Value, out ValueTypeClass valueType))
throw new InvalidOperationException($"Cannot apply indexing with [] to an expression of scheme '{objectId.Scheme}'");
var typeInfo = await context.SdbAgent.GetTypeInfo(valueType.TypeId, token);
if (valueType.InlineArray == null)
{
JObject vtResult = await InvokeGetItemOnJObject(rootObject, valueType.TypeId, objectId, elementIdxInfo, token);
if (vtResult != null)
return vtResult;
}
if (int.TryParse(elementIdxInfo.ElementIdxStr, out elementIdx) && elementIdx >= 0 && elementIdx < valueType.InlineArray.Count)
return (JObject)valueType.InlineArray[elementIdx]["value"];
throw new InvalidOperationException($"Index is outside the bounds of the inline array");
Expand Down Expand Up @@ -436,34 +442,11 @@ public async Task<JObject> Resolve(
if (elementIdxInfo.Indexers is null || elementIdxInfo.Indexers.Count == 0)
throw new InternalErrorException($"Unable to write index parameter to invoke the method in the runtime.");

var typeIds = await context.SdbAgent.GetTypeIdsForObject(objectId.Value, true, token);
int[] methodIds = await context.SdbAgent.GetMethodIdsByName(typeIds[0], "get_Item", BindingFlags.Default, token);
if (methodIds == null || methodIds.Length == 0)
throw new InvalidOperationException($"Type '{rootObject?["className"]?.Value<string>()}' cannot be indexed.");

// ToDo: optimize the loop by choosing the right method at once without trying out them all
for (int i = 0; i < methodIds.Length; i++)
{
MethodInfoWithDebugInformation methodInfo = await context.SdbAgent.GetMethodInfo(methodIds[i], token);
ParameterInfo[] paramInfo = methodInfo.GetParametersInfo();
if (paramInfo.Length == elementIdxInfo.DimensionsCount)
{
try
{
if (!CheckParametersCompatibility(paramInfo, elementIdxInfo.Indexers))
continue;
ArraySegment<byte> buffer = await WriteIndexObjectAsIndices(objectId, elementIdxInfo.Indexers, paramInfo);
JObject getItemRetObj = await context.SdbAgent.InvokeMethod(buffer, methodIds[i], token);
return (JObject)getItemRetObj["value"];
}
catch (Exception ex)
{
logger.LogDebug($"Attempt number {i + 1} out of {methodIds.Length} of invoking method {methodInfo.Name} with parameter named {paramInfo[0].Name} on type {type} failed. Method Id = {methodIds[i]}.\nInner exception: {ex}.");
continue;
}
}
}
throw new InvalidOperationException($"Cannot apply indexing with [] to an object of type '{rootObject?["className"]?.Value<string>()}'");
List<int> typeIds = await context.SdbAgent.GetTypeIdsForObject(objectId.Value, true, token);
JObject objResult = await InvokeGetItemOnJObject(rootObject, typeIds[0], objectId, elementIdxInfo, token);
if (objResult == null)
throw new InvalidOperationException($"Cannot apply indexing with [] to an object of type '{rootObject?["className"]?.Value<string>()}'");
return objResult;
default:
throw new InvalidOperationException($"Cannot apply indexing with [] to an expression of scheme '{objectId.Scheme}'");
}
Expand Down Expand Up @@ -543,6 +526,42 @@ async Task<ElementIndexInfo> GetElementIndexInfo(List<JObject> nestedIndexers)
ElementIdxStr: elementIdxStr.ToString(),
Indexers: indexers);
}
}

private async Task<JObject> InvokeGetItemOnJObject(
JObject rootObject,
int typeId,
DotnetObjectId objectId,
ElementIndexInfo elementIdxInfo,
CancellationToken token)
{
int[] methodIds = await context.SdbAgent.GetMethodIdsByName(typeId, "get_Item", BindingFlags.Default, token);
if (methodIds == null || methodIds.Length == 0)
throw new InvalidOperationException($"Type '{rootObject?["className"]?.Value<string>()}' cannot be indexed.");
var type = rootObject?["type"]?.Value<string>();

// ToDo: optimize the loop by choosing the right method at once without trying out them all
Copy link
Member

Choose a reason for hiding this comment

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

What is needed, or missing to do this?

Copy link
Member

Choose a reason for hiding this comment

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

Your idea here is the better approach.

Copy link
Member Author

@ilonatommy ilonatommy Oct 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Reason why it might be not possible:

  1. If the parameter is required (as opposed to default) then we don't have information about its type. Even though we know what types were passed to the method, we cannot tell if this method will work well with them till we ask the runtime to CMD_VM_INVOKE_METHOD and it will throw / return a result.
  2. Sometimes we even are not sure what type was passed to the method, see [wasm][debugger] Evaluate methods with short arguments #93057.
    It would be good to try if something changed but please, not in this PR. I will see if I can make it in a separate one.
    Edit:
    Trying to create the new PR I found a good example for situation 1).
void foo(Class1 c);
void foo(Class2 c);

When we ask for GetMethodIdsByName for "foo" we will get both methods' IDs. We can check both method parameter types by refection but the information we will get is ElementType.Class which does not tell us enough.

for (int i = 0; i < methodIds.Length; i++)
{
MethodInfoWithDebugInformation methodInfo = await context.SdbAgent.GetMethodInfo(methodIds[i], token);
ParameterInfo[] paramInfo = methodInfo.GetParametersInfo();
if (paramInfo.Length != elementIdxInfo.DimensionsCount)
continue;
try
{
if (!CheckParametersCompatibility(paramInfo, elementIdxInfo.Indexers))
continue;
ArraySegment<byte> buffer = await WriteIndexObjectAsIndices(objectId, elementIdxInfo.Indexers, paramInfo);
JObject getItemRetObj = await context.SdbAgent.InvokeMethod(buffer, methodIds[i], token);
return (JObject)getItemRetObj["value"];
}
catch (Exception ex)
{
logger.LogDebug($"Attempt number {i + 1} out of {methodIds.Length} of invoking method {methodInfo.Name} with parameter named {paramInfo[0].Name} on type {type} failed. Method Id = {methodIds[i]}.\nInner exception: {ex}.");
continue;
}
}
return null;

async Task<ArraySegment<byte>> WriteIndexObjectAsIndices(DotnetObjectId rootObjId, List<object> indexObjects, ParameterInfo[] paramInfo)
{
Expand Down Expand Up @@ -578,19 +597,47 @@ private static bool CheckParametersCompatibility(ParameterInfo[] paramInfos, Lis
return false;
foreach ((ParameterInfo paramInfo, object indexObj) in paramInfos.Zip(indexObjects))
{
// shouldn't we check LiteralExpressionSyntax for compatibility as well?
if (indexObj is JObject indexJObj && !CheckParameterCompatibility(paramInfo.TypeCode, indexJObj))
string argumentType = "", argumentClassName = "";
if (indexObj is JObject indexJObj)
{
argumentType = indexJObj["type"]?.Value<string>();
argumentClassName = indexJObj["className"]?.Value<string>();
}
else if (indexObj is LiteralExpressionSyntax literal)
{
// any primitive literal is an object
if (paramInfo.TypeCode.Value == ElementType.Object)
continue;
switch (literal.Kind())
{
case SyntaxKind.NumericLiteralExpression:
argumentType = "number";
break;
case SyntaxKind.StringLiteralExpression:
argumentType = "string";
break;
case SyntaxKind.TrueLiteralExpression:
case SyntaxKind.FalseLiteralExpression:
argumentType = "boolean";
break;
case SyntaxKind.CharacterLiteralExpression:
argumentType = "symbol";
break;
case SyntaxKind.NullLiteralExpression:
// do not check
continue;
}
}
if (!CheckParameterCompatibility(paramInfo.TypeCode, argumentType, argumentClassName))
return false;
}
return true;
}

private static bool CheckParameterCompatibility(ElementType? paramTypeCode, JObject value)
private static bool CheckParameterCompatibility(ElementType? paramTypeCode, string argumentType, string argumentClassName="")
{
if (!paramTypeCode.HasValue)
return true;
var argumentType = value["type"]?.Value<string>();
var argumentClassName = value["className"]?.Value<string>();

switch (paramTypeCode.Value)
{
Expand Down Expand Up @@ -624,6 +671,8 @@ private static bool CheckParameterCompatibility(ElementType? paramTypeCode, JObj
return false;
if (argumentType == "object")
return false;
if (argumentType == "string" || argumentType == "symbol")
return false;
break;
case ElementType.String:
if (argumentType != "string")
Expand Down
2 changes: 1 addition & 1 deletion src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -858,7 +858,7 @@ internal sealed partial class MonoSDBHelper

private static int debuggerObjectId;
private static int cmdId = 1; //cmdId == 0 is used by events which come from runtime
private const int MINOR_VERSION = 65;
private const int MINOR_VERSION = 66;
private const int MAJOR_VERSION = 2;

private int VmMinorVersion { get; set; }
Expand Down
8 changes: 5 additions & 3 deletions src/mono/wasm/debugger/DebuggerTestSuite/ArrayTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -684,7 +684,7 @@ public async Task InspectInlineArray(string varName)
$"'[debugger-test] DebuggerTests.InlineArray:run'" +
"); }, 1);";

var pause_location = await EvaluateAndCheck(eval_expr, debugger_test_loc, 441, 12, "DebuggerTests.InlineArray.run");
var pause_location = await EvaluateAndCheck(eval_expr, debugger_test_loc, 442, 12, "DebuggerTests.InlineArray.run");
var id = pause_location["callFrames"][0]["callFrameId"].Value<string>();
await EvaluateOnCallFrameAndCheck(id,
($"{varName}[0]", TObject("DebuggerTests.InlineArray.E")),
Expand Down Expand Up @@ -714,6 +714,7 @@ await EvaluateOnCallFrameAndCheck(id,
o = TObject("DebuggerTests.InlineArray.Two")
}, "s_one_props#1");
}

[ConditionalFact(nameof(RunningOnChrome))]
public async Task InspectInlineArray2()
{
Expand All @@ -723,13 +724,14 @@ public async Task InspectInlineArray2()
$"'[debugger-test] DebuggerTests.InlineArray:run2'" +
"); }, 1);";

var pause_location = await EvaluateAndCheck(eval_expr, debugger_test_loc, 459, 12, "DebuggerTests.InlineArray.run2");
var pause_location = await EvaluateAndCheck(eval_expr, debugger_test_loc, 460, 12, "DebuggerTests.InlineArray.run2");
var id = pause_location["callFrames"][0]["callFrameId"].Value<string>();
await EvaluateOnCallFrameAndCheck(id,
($"a2[0]", TNumber(1)),
($"a3[0]", TNumber(2)),
($"a4[0]", TObject("DebuggerTests.InlineArray.E")),
($"Arr4.myStaticField", TNumber(50))
($"Arr4.myStaticField", TNumber(50)),
($"a2.InlineMethod(1)", TNumber(101))
);

var (_, res) = await EvaluateOnCallFrame(id,$"a3[1]", expect_ok: false);
Expand Down
33 changes: 26 additions & 7 deletions src/mono/wasm/debugger/DebuggerTestSuite/EvaluateOnCallFrame2Tests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -755,13 +755,32 @@ public async Task EvaluateObjectIndexingMultidimensional() => await CheckInspect
var id = pause_location["callFrames"][0]["callFrameId"].Value<string>();

await EvaluateOnCallFrameAndCheck(id,
("f[j, aDouble]", TNumber("3.34")), //only IdentifierNameSyntaxes
("f[1, aDouble]", TNumber("3.34")), //IdentifierNameSyntax with LiteralExpressionSyntax
("f[aChar, \"&\", longString]", TString("9-&-longString")),
("f[f.numArray[j], aDouble]", TNumber("4.34")), //ElementAccessExpressionSyntax
("f[f.numArray[j], f.numArray[0]]", TNumber("3")), //multiple ElementAccessExpressionSyntaxes
("f[f.numArray[f.numList[0]], f.numArray[i]]", TNumber("3")),
("f[f.numArray[f.numList[0]], f.numArray[f.numArray[i]]]", TNumber("4"))
("c[j, aDouble]", TNumber("3.34")), //only IdentifierNameSyntaxes
("c[1, aDouble]", TNumber("3.34")), //IdentifierNameSyntax with LiteralExpressionSyntax
("c[aChar, \"&\", longString]", TString("9-&-longString")),
("c[cc.numArray[j], aDouble]", TNumber("4.34")), //ElementAccessExpressionSyntax
("c[cc.numArray[j], cc.numArray[0]]", TNumber("3")), //multiple ElementAccessExpressionSyntaxes
("c[cc.numArray[cc.numList[0]], cc.numArray[i]]", TNumber("3")),
("c[cc.numArray[cc.numList[0]], cc.numArray[cc.numArray[i]]]", TNumber("4"))
);
});

[Fact]
public async Task EvaluateValueTypeIndexingMultidimensional() => await CheckInspectLocalsAtBreakpointSite(
"DebuggerTests.EvaluateLocalsWithIndexingTests", "EvaluateLocals", 12, "DebuggerTests.EvaluateLocalsWithIndexingTests.EvaluateLocals",
"window.setTimeout(function() { invoke_static_method ('[debugger-test] DebuggerTests.EvaluateLocalsWithIndexingTests:EvaluateLocals'); })",
wait_for_event_fn: async (pause_location) =>
{
var id = pause_location["callFrames"][0]["callFrameId"].Value<string>();

await EvaluateOnCallFrameAndCheck(id,
("s[j, aDouble]", TNumber("3.34")), //only IdentifierNameSyntaxes
("s[1, aDouble]", TNumber("3.34")), //IdentifierNameSyntax with LiteralExpressionSyntax
("s[aChar, \"&\", longString]", TString("9-&-longString")),
("s[cc.numArray[j], aDouble]", TNumber("4.34")), //ElementAccessExpressionSyntax
("s[cc.numArray[j], cc.numArray[0]]", TNumber("3")), //multiple ElementAccessExpressionSyntaxes
("s[cc.numArray[cc.numList[0]], cc.numArray[i]]", TNumber("3")),
("s[cc.numArray[cc.numList[0]], cc.numArray[cc.numArray[i]]]", TNumber("4"))
);
});
}
Expand Down
Loading
Loading