Skip to content

Test GH Code review suggestions #4

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

Closed
ilonatommy wants to merge 2 commits into from
Closed

Test GH Code review suggestions #4

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8cdd754
TMP
ilonatommy Sep 20, 2022
91fdb96
Update src/mono/mono/component/debugger-agent.c
ilonatommy Feb 28, 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
Binary file added out.txt
View file
Open in desktop
Binary file not shown.
31 changes: 24 additions & 7 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 @@ -5335,8 +5335,9 @@ decode_vtype (MonoType *t, MonoDomain *domain, gpointer void_addr, gpointer void

if (t && klass != mono_class_from_mono_type_internal (t)) {
char *name = mono_type_full_name (t);
char *name2 = mono_type_full_name (m_class_get_byval_arg (klass));
PRINT_DEBUG_MSG (1, "[%p] Expected value of type %s, got %s.\n", (gpointer) (gsize) mono_native_thread_id_get (), name, name2);
char *name3 = mono_type_full_name (m_class_get_byval_arg (klass));
HA! my edition
PRINT_DEBUG_MSG (1, "[%p] 1 Expected value of type %s, got %s.\n", (gpointer) (gsize) mono_native_thread_id_get (), name, name2);
Copy link
Owner Author

@ilonatommy ilonatommy Feb 28, 2023

Choose a reason for hiding this comment

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

Suggested change
PRINT_DEBUG_MSG (1, "[%p] 1 Expected value of type %s, got %s.\n", (gpointer) (gsize) mono_native_thread_id_get (), name, name2);
PRINT_DEBUG_MSG (1, "[%p] 2 Expected value of type %s, got %s.\n", (gpointer) (gsize) mono_native_thread_id_get (), name, name2);

g_free (name);
g_free (name2);
return ERR_INVALID_ARGUMENT;
Expand Down Expand Up @@ -5416,6 +5417,7 @@ static ErrorCode
decode_value_internal (MonoType *t, int type, MonoDomain *domain, guint8 *addr, guint8 *buf, guint8 **endbuf, guint8 *limit, gboolean check_field_datatype)
{
ErrorCode err;
PRINT_DEBUG_MSG (1, "[ILONA] decode_value_internal t->type = %d, type = %d\n", t->type, type);

if (type != t->type && !MONO_TYPE_IS_REFERENCE (t) &&
!(t->type == MONO_TYPE_I && type == MONO_TYPE_VALUETYPE) &&
Expand All @@ -5426,7 +5428,7 @@ decode_value_internal (MonoType *t, int type, MonoDomain *domain, guint8 *addr,
!(t->type == MONO_TYPE_GENERICINST && type == MONO_TYPE_VALUETYPE) &&
!(t->type == MONO_TYPE_VALUETYPE && type == MONO_TYPE_OBJECT)) {
char *name = mono_type_full_name (t);
PRINT_DEBUG_MSG (1, "[%p] Expected value of type %s, got 0x%0x.\n", (gpointer) (gsize) mono_native_thread_id_get (), name, type);
PRINT_DEBUG_MSG (1, "[%p] 2 Expected value of type %s, got 0x%0x.\n", (gpointer) (gsize) mono_native_thread_id_get (), name, type);
g_free (name);
return ERR_INVALID_ARGUMENT;
}
Expand Down Expand Up @@ -5581,7 +5583,7 @@ decode_value_internal (MonoType *t, int type, MonoDomain *domain, guint8 *addr,
g_free (vtype_buf);
} else {
char *name = mono_type_full_name (t);
PRINT_DEBUG_MSG (1, "[%p] Expected value of type %s, got 0x%0x.\n", (gpointer) (gsize) mono_native_thread_id_get (), name, type);
PRINT_DEBUG_MSG (1, "[%p] 3 Expected value of type %s, got 0x%0x. %s, %s\n", (gpointer) (gsize) mono_native_thread_id_get (), name, type, name, type);
g_free (name);
return ERR_INVALID_ARGUMENT;
}
Expand Down Expand Up @@ -5612,8 +5614,10 @@ decode_value (MonoType *t, MonoDomain *domain, gpointer void_addr, gpointer void
ERROR_DECL (error);
ErrorCode err;
int type = decode_byte (buf, &buf, limit);
PRINT_DEBUG_MSG (1, "[ILONA] decode_value type = %d, t->type = %d\n", type, t->type);

if (t->type == MONO_TYPE_GENERICINST && mono_class_is_nullable (mono_class_from_mono_type_internal (t))) {
PRINT_DEBUG_MSG (1, "[ILONA] decode_value type in if 1");
MonoType *targ = t->data.generic_class->context.class_inst->type_argv [0];
guint8 *nullable_buf;

Expand All @@ -5623,11 +5627,13 @@ decode_value (MonoType *t, MonoDomain *domain, gpointer void_addr, gpointer void
err = decode_value_internal (t, type, domain, addr, buf, endbuf, limit, check_field_datatype);
if (err == ERR_NONE)
return err;
PRINT_DEBUG_MSG (1, "[ILONA] decode_value type in if 2");

/*
* Then try decoding as a primitive value or null.
*/
if (targ->type == type) {
PRINT_DEBUG_MSG (1, "[ILONA] decode_value type in if 3");
nullable_buf = (guint8 *)g_malloc (mono_class_instance_size (mono_class_from_mono_type_internal (targ)));
err = decode_value_internal (targ, type, domain, nullable_buf, buf, endbuf, limit, check_field_datatype);
if (err != ERR_NONE) {
Expand Down Expand Up @@ -6037,14 +6043,14 @@ mono_do_invoke_method (DebuggerTlsData *tls, Buffer *buf, InvokeData *invoke, gu
* Invoke this method directly, currently only Environment.Exit () is supported.
*/
this_arg = NULL;
PRINT_DEBUG_MSG (1, "[%p] Invoking method '%s' on receiver '%s'.\n", (gpointer) (gsize) mono_native_thread_id_get (), mono_method_full_name (invoke->method, TRUE), this_arg ? m_class_get_name (this_arg->vtable->klass) : "<null>");
PRINT_DEBUG_MSG (1, "[%p] 1 Invoking method '%s' on receiver '%s'.\n", (gpointer) (gsize) mono_native_thread_id_get (), mono_method_full_name (invoke->method, TRUE), this_arg ? m_class_get_name (this_arg->vtable->klass) : "<null>");

mono_runtime_try_invoke (invoke->method, NULL, invoke->args, &exc, error);
mono_error_assert_ok (error);
g_assert_not_reached ();
}

m = decode_methodid (p, &p, end, &domain, &err);
m = decode_methodid (p, &p, end, &domain, &err); // exception after it
if (err != ERR_NONE)
return err;
sig = mono_method_signature_internal (m);
Expand Down Expand Up @@ -6116,7 +6122,7 @@ mono_do_invoke_method (DebuggerTlsData *tls, Buffer *buf, InvokeData *invoke, gu
}
}

PRINT_DEBUG_MSG (1, "[%p] Invoking method '%s' on receiver '%s'.\n", (gpointer) (gsize) mono_native_thread_id_get (), mono_method_full_name (m, TRUE), this_arg ? m_class_get_name (this_arg->vtable->klass) : "<null>");
PRINT_DEBUG_MSG (1, "[%p] 2 Invoking method '%s' on receiver '%s'.\n", (gpointer) (gsize) mono_native_thread_id_get (), mono_method_full_name (m, TRUE), this_arg ? m_class_get_name (this_arg->vtable->klass) : "<null>");

if (this_arg && this_arg->vtable->domain != domain)
NOT_IMPLEMENTED;
Expand All @@ -6141,39 +6147,50 @@ mono_do_invoke_method (DebuggerTlsData *tls, Buffer *buf, InvokeData *invoke, gu
return ERR_INVALID_ARGUMENT;

nargs = decode_int (p, &p, end);
PRINT_DEBUG_MSG (1, "[ILONA] mono_do_invoke_method 1, nargs = %d, paramCnt = %d\n", nargs, sig->param_count);
if (nargs != sig->param_count)
return ERR_INVALID_ARGUMENT;
/* Use alloca to get gc tracking */
arg_buf = (guint8 **)g_alloca (nargs * sizeof (gpointer));
memset (arg_buf, 0, nargs * sizeof (gpointer));
args = (gpointer *)g_alloca (nargs * sizeof (gpointer));
for (i = 0; i < nargs; ++i) {
PRINT_DEBUG_MSG (1, "[ILONA] mono_do_invoke_method in the loop i = %d\n", i);
if (MONO_TYPE_IS_REFERENCE (sig->params [i])) {
PRINT_DEBUG_MSG (1, "[ILONA] mono_do_invoke_method 2\n");
err = decode_value (sig->params [i], domain, (guint8*)&args [i], p, &p, end, TRUE);
PRINT_DEBUG_MSG (1, "[ILONA] mono_do_invoke_method 3, err = %d\n", err);
if (err != ERR_NONE)
break;
if (args [i] && ((MonoObject*)args [i])->vtable->domain != domain)
NOT_IMPLEMENTED;
PRINT_DEBUG_MSG (1, "[ILONA] mono_do_invoke_method 4\n");

if (m_type_is_byref (sig->params [i])) {
PRINT_DEBUG_MSG (1, "[ILONA] mono_do_invoke_method 5\n");
arg_buf [i] = g_newa (guint8, sizeof (gpointer));
*(gpointer*)arg_buf [i] = args [i];
args [i] = arg_buf [i];
}
} else {
PRINT_DEBUG_MSG (1, "[ILONA] mono_do_invoke_method 6\n");
MonoClass *arg_class = mono_class_from_mono_type_internal (sig->params [i]);
arg_buf [i] = (guint8 *)g_alloca (mono_class_instance_size (arg_class));
err = decode_value (sig->params [i], domain, arg_buf [i], p, &p, end, TRUE);
PRINT_DEBUG_MSG (1, "[ILONA] mono_do_invoke_method 7, err = %d\n", err);
if (err != ERR_NONE)
break;
if (mono_class_is_nullable (arg_class)) {
PRINT_DEBUG_MSG (1, "[ILONA] mono_do_invoke_method 8\n");
args [i] = mono_nullable_box (arg_buf [i], arg_class, error);
mono_error_assert_ok (error);
} else {
PRINT_DEBUG_MSG (1, "[ILONA] mono_do_invoke_method 9\n");
args [i] = arg_buf [i];
}
}
}
PRINT_DEBUG_MSG (1, "[ILONA] mono_do_invoke_method 10, i = %d\n", i);

if (i < nargs)
return err;
Expand Down
12 changes: 10 additions & 2 deletions src/mono/mono/component/mini-wasm-debugger.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ handle_multiple_ss_requests (void) {
static void
mono_wasm_enable_debugging_internal (int debug_level)
{
log_level = debug_level;
log_level = 10;
if (debug_level != 0) {
wasm_debugger_log(1, "DEBUGGING ENABLED\n");
debugger_enabled = TRUE;
Expand Down Expand Up @@ -373,6 +373,7 @@ extern void mono_wasm_add_dbg_command_received(mono_bool res_ok, int id, void* b
EMSCRIPTEN_KEEPALIVE gboolean
mono_wasm_send_dbg_command_with_parms (int id, MdbgProtCommandSet command_set, int command, guint8* data, unsigned int size, int valtype, char* newvalue)
{
PRINT_DEBUG_MSG (1, "[ILONA] mono_wasm_send_dbg_command_with_parms 1\n");
gboolean result = FALSE;
MONO_ENTER_GC_UNSAFE;
if (!debugger_enabled) {
Expand All @@ -389,7 +390,9 @@ mono_wasm_send_dbg_command_with_parms (int id, MdbgProtCommandSet command_set, i
result = TRUE;
goto done;
}
PRINT_DEBUG_MSG (1, "[ILONA] mono_wasm_send_dbg_command_with_parms 2\n");
mono_wasm_send_dbg_command(id, command_set, command, bufWithParms.buf, m_dbgprot_buffer_len(&bufWithParms));
PRINT_DEBUG_MSG (1, "[ILONA] mono_wasm_send_dbg_command_with_parms 3\n");
buffer_free (&bufWithParms);
result = TRUE;
done:
Expand All @@ -400,6 +403,7 @@ mono_wasm_send_dbg_command_with_parms (int id, MdbgProtCommandSet command_set, i
EMSCRIPTEN_KEEPALIVE gboolean
mono_wasm_send_dbg_command (int id, MdbgProtCommandSet command_set, int command, guint8* data, unsigned int size)
{
PRINT_DEBUG_MSG (1, "[ILONA] mono_wasm_send_dbg_command 1\n");
gboolean result = FALSE;
MONO_ENTER_GC_UNSAFE;
if (!debugger_enabled) {
Expand All @@ -408,18 +412,22 @@ mono_wasm_send_dbg_command (int id, MdbgProtCommandSet command_set, int command,
result = TRUE;
goto done;
}
PRINT_DEBUG_MSG (1, "[ILONA] mono_wasm_send_dbg_command 2\n");
ss_calculate_framecount (NULL, NULL, TRUE, NULL, NULL);
MdbgProtBuffer buf;
buffer_init (&buf, 128);
gboolean no_reply;
MdbgProtErrorCode error = 0;
if (command_set == MDBGPROT_CMD_SET_VM && command == MDBGPROT_CMD_VM_INVOKE_METHOD )
{
PRINT_DEBUG_MSG (1, "[ILONA] mono_wasm_send_dbg_command 3; command == MDBGPROT_CMD_VM_INVOKE_METHOD\n");
DebuggerTlsData* tls = mono_wasm_get_tls ();
InvokeData invoke_data;
memset (&invoke_data, 0, sizeof (InvokeData));
invoke_data.endp = data + size;
PRINT_DEBUG_MSG (1, "[ILONA] mono_wasm_send_dbg_command 4\n");
error = mono_do_invoke_method (tls, &buf, &invoke_data, data, &data);
PRINT_DEBUG_MSG (1, "[ILONA] mono_wasm_send_dbg_command 5, error=%d\n", error);
}
else if (command_set == MDBGPROT_CMD_SET_VM && (command == MDBGPROT_CMD_GET_ASSEMBLY_BYTES))
{
Expand Down Expand Up @@ -488,4 +496,4 @@ mini_wasm_debugger_add_function_pointers (MonoComponentDebugger* fn_table)
fn_table->mono_wasm_single_step_hit = mono_wasm_single_step_hit;
}

#endif
#endif
47 changes: 35 additions & 12 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 @@ -458,19 +458,27 @@ 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);
// 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.");

// int[] methodIds = await context.SdbAgent.GetMethodIdsByName(typeIds[0], "ToArray", token);
// int allowedParamCnt = 0;
// if (methodIds == null)
// {
int allowedParamCnt = 1;
int[] methodIds = await context.SdbAgent.GetMethodIdsByName(typeIds[0], "get_Item", token);
if (methodIds == null)
throw new InvalidOperationException($"Type '{rootObject?["className"]?.Value<string>()}' cannot be indexed.");
// }

// ToArray / get_Item should not have an overload, but if user defined it, take the default one: without params / with one param: key
int toArrayId = methodIds[0];
if (methodIds.Length > 1)
{
Console.WriteLine($"methodIds.Length > 1");
foreach (var methodId in methodIds)
{
MethodInfoWithDebugInformation methodInfo = await context.SdbAgent.GetMethodInfo(methodId, token);
ParameterInfo[] paramInfo = methodInfo.GetParametersInfo();
if (paramInfo.Length == 0)
Console.WriteLine($"name = {methodInfo.Name}; paramInfo = {paramInfo.Length}");
if (paramInfo.Length == allowedParamCnt)
{
toArrayId = methodId;
break;
Expand All @@ -479,14 +487,29 @@ public async Task<JObject> Resolve(ElementAccessExpressionSyntax elementAccess,
}
try
{
var toArrayRetMethod = await context.SdbAgent.InvokeMethod(objectId.Value, toArrayId, isValueType: false, token);
rootObject = await GetValueFromObject(toArrayRetMethod, token);
DotnetObjectId.TryParse(rootObject?["objectId"]?.Value<string>(), out DotnetObjectId arrayObjectId);
rootObject["value"] = await context.SdbAgent.GetArrayValues(arrayObjectId.Value, token);
return (JObject)rootObject["value"][elementIdx]["value"];
Console.WriteLine($"methodIds.Length == 1");
Console.WriteLine($"rootObject = {rootObject}, elementIdxStr = {elementIdxStr}");
if (allowedParamCnt == 0) // ToArray result
{
JObject toArrayRetObj = await context.SdbAgent.InvokeMethod(objectId.Value, toArrayId, isValueType: false, token);
rootObject = await GetValueFromObject(toArrayRetObj, token);
DotnetObjectId.TryParse(rootObject?["objectId"]?.Value<string>(), out DotnetObjectId arrayObjectId);
rootObject["value"] = await context.SdbAgent.GetArrayValues(arrayObjectId.Value, token);
return (JObject)rootObject["value"][elementIdx]["value"];
}
// get_Item result
using var ctorArgsWriter = new MonoBinaryWriter();
ctorArgsWriter.WriteObj(objectId, context.SdbAgent);
ctorArgsWriter.Write(1); // number of method args
// if we learn how to check properly the indexing obj type and pass elementIdx / elementIdxStr when we should, then ToArray won't be needed
if (!await ctorArgsWriter.WriteConst(ElementType.I4, elementIdx, context.SdbAgent, token)) // string? how do you know?
throw new InternalErrorException($"Unable to write index parameter to invoke the method in the runtime.");
JObject getItemRetObj = await context.SdbAgent.InvokeMethod(ctorArgsWriter.GetParameterBuffer(), toArrayId, token);
return (JObject)getItemRetObj["value"];
}
catch
catch (Exception ex)
{
Console.WriteLine($"ex = {ex}");
throw new InvalidOperationException($"Cannot apply indexing with [] to an object of type '{rootObject?["className"]?.Value<string>()}'");
}
default:
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 @@ -778,7 +778,7 @@ internal sealed class MonoSDBHelper
private int VmMinorVersion { get; set; }
private int VmMajorVersion { get; set; }

private Dictionary<int, MethodInfoWithDebugInformation> methods;
public Dictionary<int, MethodInfoWithDebugInformation> methods;
private Dictionary<int, AssemblyInfo> assemblies;
private Dictionary<int, TypeInfoWithDebugInformation> types;

Expand Down
44 changes: 38 additions & 6 deletions src/mono/wasm/debugger/DebuggerTestSuite/EvaluateOnCallFrameTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -627,19 +627,51 @@ await EvaluateOnCallFrameAndCheck(id,

});

[Fact] //FIXME: Type 'DebuggerTests.EvaluateLocalsWithIndexingTests.TestEvaluate' cannot be indexed.
public async Task EvaluateIndexingOfObjectsThatDoNotHaveToArrayMethod() => await CheckInspectLocalsAtBreakpointSite(
"DebuggerTests.EvaluateLocalsWithIndexingTests", "EvaluateLocals", 5, "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,
("f[\"longstring\"]", TBool(true))
// ("f[\"-\"]", TBool(false))
// ("f.arrIndexedByStr[\"longstring\"]", TBool(true)),
// ("f.arrIndexedByStr[\"-\"]", TBool(false))
);
});

[Fact]
public async Task EvaluateIndexingByNonNumberType() => await CheckInspectLocalsAtBreakpointSite(
"DebuggerTests.EvaluateLocalsWithIndexingTests", "EvaluateLocals", 5, "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,
("f.indexedByStr[\"11\"]", TBool(true)),
("f.indexedByStr[\"111\"]", TBool(false)),
("f.indexedByBool[true]", TString("TRUE")),
("f.indexedByStr[false]", TString("FALSE"))
);
});


[Fact]
public async Task EvaluateIndexingByExpression() => await CheckInspectLocalsAtBreakpointSite(
public async Task EvaluateIndexingByExpression2() => await CheckInspectLocalsAtBreakpointSite(
"DebuggerTests.EvaluateLocalsWithIndexingTests", "EvaluateLocals", 5, "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,
("f.numList[i + 1]", TNumber(2)),
("f.textList[(2 * j) - 1]", TString("2")),
("f.textList[j - 1]", TString("1")),
//("f[\"longstring\"]", TBool(true)), FIXME: Broken case
("f.numArray[f.numList[j - 1]]", TNumber(2))
// ("f.numList[i + 1]", TNumber(2)),
// ("f.textList[(2 * j) - 1]", TString("2")),
// ("f.textList[j - 1]", TString("1")),
("f.indexedByStr[\"1\" + \"1\"]", TBool(true)),
("f.indexedByStr[\'1\' + \"11\"]", TBool(false))
// ("f.numArray[f.numList[j - 1]]", TNumber(2))
);
});

Expand Down
Loading