[mono][debugger] Fix watch of local variable values (#57771)

* Fix xamarin-android 6161

* Fixing leak.

* Adding wasm debugger test.
Fixing using protocol version.

* only get locals in older protocol versions.

Author: thaystg

src/mono/mono/component/debugger-agent.c

@@ -8513,7 +8513,7 @@ method_commands_internal (int command, MonoMethod *method, MonoDomain *domain, g
 			return ERR_INVALID_ARGUMENT;
 		}
 
-		locals = mono_debug_lookup_locals (method, FALSE);
+		locals = mono_debug_lookup_locals (method);
 		if (!locals) {
 			if (CHECK_PROTOCOL_VERSION (2, 43)) {
 				/* Scopes */
@@ -9230,13 +9230,14 @@ frame_commands (int command, guint8 *p, guint8 *end, Buffer *buf)
 				pos = - pos - 1;
 				cmd_stack_frame_get_parameter (frame, sig, pos, buf, jit);
 			} else {
-				MonoDebugLocalsInfo *locals;
-
-				locals = mono_debug_lookup_locals (frame->de.method, TRUE);
-				if (locals) {
-					g_assert (pos < locals->num_locals);
-					pos = locals->locals [pos].index;
-					mono_debug_free_locals (locals);
+				if (!CHECK_PROTOCOL_VERSION (2, 59)) { //from newer protocol versions it's sent the pdb index
+					MonoDebugLocalsInfo *locals;
+					locals = mono_debug_lookup_locals (frame->de.method);
+					if (locals) {
+						g_assert (pos < locals->num_locals);
+						pos = locals->locals [pos].index;
+						mono_debug_free_locals (locals);
+					}
 				}
 
 				PRINT_DEBUG_MSG (4, "[dbg]   send local %d.\n", pos);
@@ -9284,13 +9285,14 @@ frame_commands (int command, guint8 *p, guint8 *end, Buffer *buf)
 				var = &jit->params [pos];
 				is_arg = TRUE;
 			} else {
-				MonoDebugLocalsInfo *locals;
-
-				locals = mono_debug_lookup_locals (frame->de.method, TRUE);
-				if (locals) {
-					g_assert (pos < locals->num_locals);
-					pos = locals->locals [pos].index;
-					mono_debug_free_locals (locals);
+				if (!CHECK_PROTOCOL_VERSION (2, 59)) { //from newer protocol versions it's sent the pdb index
+					MonoDebugLocalsInfo *locals;
+					locals = mono_debug_lookup_locals (frame->de.method);
+					if (locals) {
+						g_assert (pos < locals->num_locals);
+						pos = locals->locals [pos].index;
+						mono_debug_free_locals (locals);
+					}
 				}
 				g_assert (pos >= 0 && pos < jit->num_locals);
 

src/mono/mono/metadata/mono-debug.c

@@ -867,7 +867,7 @@ mono_debug_method_lookup_location (MonoDebugMethodInfo *minfo, int il_offset)
  * The result should be freed using mono_debug_free_locals ().
  */
 MonoDebugLocalsInfo*
-mono_debug_lookup_locals (MonoMethod *method, mono_bool ignore_pdb)
+mono_debug_lookup_locals (MonoMethod *method)
 {
 	MonoDebugMethodInfo *minfo;
 	MonoDebugLocalsInfo *res;
@@ -893,18 +893,16 @@ mono_debug_lookup_locals (MonoMethod *method, mono_bool ignore_pdb)
 		return NULL;
 	}
 
-	if (ignore_pdb)
-		res = mono_debug_symfile_lookup_locals (minfo);
-	else {
-		if (minfo->handle->ppdb) {
-			res = mono_ppdb_lookup_locals (minfo);
-		} else {
-			if (!minfo->handle->symfile || !mono_debug_symfile_is_loaded (minfo->handle->symfile))
-				res = NULL;
-			else
-				res = mono_debug_symfile_lookup_locals (minfo);
-		}
+
+	if (minfo->handle->ppdb) {
+		res = mono_ppdb_lookup_locals (minfo);
+	} else {
+		if (!minfo->handle->symfile || !mono_debug_symfile_is_loaded (minfo->handle->symfile))
+			res = NULL;
+		else
+			res = mono_debug_symfile_lookup_locals (minfo);
 	}
+
 	mono_debugger_unlock ();
 
 	return res;

src/mono/mono/metadata/mono-debug.h

@@ -189,7 +189,7 @@ MONO_API void
 mono_debug_add_delegate_trampoline (void* code, int size);
 
 MONO_API MonoDebugLocalsInfo*
-mono_debug_lookup_locals (MonoMethod *method, mono_bool ignore_pdb);
+mono_debug_lookup_locals (MonoMethod *method);
 
 MONO_API MonoDebugMethodAsyncInfo*
 mono_debug_lookup_method_async_debug_info (MonoMethod *method);

src/mono/mono/mini/dwarfwriter.c

@@ -1899,7 +1899,7 @@ mono_dwarf_writer_emit_method (MonoDwarfWriter *w, MonoCompile *cfg, MonoMethod
 	g_free (names);
 
 	/* Locals */
-	locals_info = mono_debug_lookup_locals (method, FALSE);
+	locals_info = mono_debug_lookup_locals (method);
 
 	for (i = 0; i < header->num_locals; ++i) {
 		MonoInst *ins = locals [i];

src/mono/wasm/debugger/DebuggerTestSuite/Tests.cs

@@ -845,6 +845,21 @@ public async Task InspectTaskAtLocals() => await CheckInspectLocalsAtBreakpointS
                     }, "t_props", num_fields: 53);
             });
 
+
+        [Fact]
+        public async Task InspectLocalsWithIndexAndPositionWithDifferentValues() //https://github.com/xamarin/xamarin-android/issues/6161
+        {
+            await EvaluateAndCheck(
+                "window.setTimeout(function() { invoke_static_method('[debugger-test] MainPage:CallSetValue'); }, 1);",
+                "dotnet://debugger-test.dll/debugger-test.cs", 758, 16,
+                "set_SomeValue",
+                locals_fn: (locals) =>
+                {
+                    CheckNumber(locals, "view", 150);
+                }
+            );
+        }
+
         //TODO add tests covering basic stepping behavior as step in/out/over
     }
 }

src/mono/wasm/debugger/tests/debugger-test/debugger-test.cs

@@ -725,5 +725,48 @@ public async System.Threading.Tasks.Task<bool> AsyncMethod()
         Console.WriteLine($"time for await");
         return true;
     }
+
+}
+
+public class MainPage
+{
+    public MainPage()
+    {
+    }
+
+    int count = 0;
+    private int someValue;
+
+    public int SomeValue
+    {
+        get
+        {
+            return someValue;
+        }
+        set
+        {
+            someValue = value;
+            count++;
+
+            if (count == 10)
+            {
+                var view = 150;
+
+                if (view != 50)
+                {
+
+                }
+                System.Diagnostics.Debugger.Break();
+            }
+
+            SomeValue = count;
+        }
+    }
+
+    public static void CallSetValue()
+    {
+        var mainPage = new MainPage();
+        mainPage.SomeValue = 10;
+    }
 }