[mono][debugger] Fix watch of local variable values

[thaystg]

Fixesdotnet/android#6161

This was a side-effect of icordbg support implementation.

In this case the index that come from debugger-libs is 0 but the correct index after reading pdb info is 1. That is why it was showing wrong variable value.

The index that comes from icordbg is already the correct one read from pdb we don't need to "fix" it on runtime side.

[ghost]

Tagging subscribers to this area: @thaystg
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Fixes dotnet/android#6161

In this case the index that come from debugger-libs is 0 but the correct index after reading pdb info is 1. That is why it was showing wrong variable value.

The index that comes from icordbg is already the correct one read from pdb we don't need to "fix" it on runtime side.

Author:	thaystg
Assignees:	-
Labels:	area-Debugger-mono
Milestone:	-

[lewing]

is it only the 0 index that is off?

[thaystg]

No, if you have more than one local variable all of them will be wrong:
like the 0 -> would take the real 0 positioned -> should take the 1 positioned.
like the 1 -> would take the real 1 positioned -> should take the 2 positioned.

It doesn't happen in all methods. I have tested in a lot of cases and couldn't reproduce, but today I found a code attached https://dev.azure.com/devdiv/DevDiv/_workitems/edit/1366070/ and could reproduce.

The code sample with variable view showing wrong value.

set
{
someValue = value;
count++;
if (count == 10)
{
#if __ANDROID__
var view = Handler.NativeView as Android.Views.View;
if (view != null)
{
} // Add your break point here and you'll see the value of view is "true"
#endif
}
SomeValue = count;
}

[lambdageek]

1. Is it possible to add a test for this?

2. Should we try to get this into RC1? or is it ok to wait for RC2?

Larry is more thorough

[thaystg]

1. I'm not sure. I'm thinking. And also I'll test if this will not break wasm debugger now that it also uses debugger-agent.
2. I think we should backport.

[lewing]

so debugger libs is sending the 0-index index into the number of locals and icordbg is sending the index in the debug info? Is it ever off by anything other than 1?

[thaystg]

Here is where we read the correct index from pdb:

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

Line 701 in 3ffd963

res->locals [lindex].index=locals_cols [MONO_LOCALVARIABLE_INDEX];

In some methods the variable 0 has index 0.
In this specific case the variable 0 has index 1.

And I don't know the reason.

[thaystg]

This PR is probably breaking webassembly that also sends the correct index.

[radical]

Here is where we read the correct index from pdb:

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

Line 701 in 3ffd963

res->locals [lindex].index=locals_cols [MONO_LOCALVARIABLE_INDEX];

In some methods the variable 0 has index 0.
In this specific case the variable 0 has index 1.

And I don't know the reason.

Not a static vs instance, thing, I guess?

[lewing]

👍

[radical]

Just for my understanding, this is a difference between debugger-libs+debugger-agent, and icordbg+debugger-agent? And wasm uses the latter, and what uses the former? Please correct me, if I have that wrong!

[lewing]

Just for my understanding, this is a difference between debugger-libs+debugger-agent, and icordbg+debugger-agent? And wasm uses the latter, and what uses the former? Please correct me, if I have that wrong!

In the older protocol we referenced the locals differently (always 0 indexed). That changed with icordbg support to match the index from the debug info but it wasn't noticed as a breaking change. This just makes that change explicit by marking it as a protocol change and handling the old behavior when appropriate.

[thaystg]

/backport to release/6.0-rc1

[github-actions]

Started backporting to release/6.0-rc1: https://github.com/dotnet/runtime/actions/runs/1148746102