[wasm][debugger] Small improvements to fail gracefully (#41713)
authorAnkit Jain <radical@gmail.com>
Sat, 5 Sep 2020 04:55:31 +0000 (00:55 -0400)
committerGitHub <noreply@github.com>
Sat, 5 Sep 2020 04:55:31 +0000 (00:55 -0400)
commit2e4e75bdf7a4d5168f79bdad1d3a3acff43f538c
tree095520159f57ac15758fd8b483dc78ac9e4cbf65
parent4fd87bc4ce9f90bcaf82de3fc5fed18486d54ea6
[wasm][debugger] Small improvements to fail gracefully (#41713)

* [wasm][debugger] Instead of failing completely, skip the problematic

.. property. Some times we might not get a `value`, or `name`, or it
might instead have a `get`. Handle those cases correctly when combining
the name/value/get objects.

This showed up in case of a `MulticastDelegate`, where we didn't have a
`value`, and ended up incorrectly combining the name/value objects, thus
returning incorrect locals.

* [wasm][debugger] Handle MulticastDelegate, and events

- Essentially we just surface these as a symbol showing the type name

* [wasm][debugger] Fail gracefully when vt could not be expanded

* [wasm][debugger] Handle invalid scope ids

scope<0, or scope too high

- This does get filtered at the proxy level, but app side should be able
to handle it too

* [wasm][debugger] Handle invalid/missing/failed value descriptions

- Eg. missing because of invalid param/local id, or value description
failed because of some as yet unsupported type

* [wasm][debugger] Fix frame indexing

- `collect_frames`, `list_frames` - both iterate over stack frames. They
skip some frames. This debug proxy assigns a simple index to each of the
received(filtered) frames.

    - so, if we had `[ frame0, (skipped frame1), frame2 ]`, the proxy will
    have `[ frame0(index:0), frame2(index:1) ]`

- `describe_variables_on_frame` also iterates over stack frames, and
tries to find a given frame by an index. And this index is what the
proxy had assigned.
    - because some frames were skipped, this function also needs to skip
    the *same* ones, so that it can find the intended frame.

    - Instead of trying to keep these in sync, here the indexing is
    changed to be the real index found as we iterate over the frames.
    - And the proxy just uses that.
    - So, we will have `[ frame0(index:0), frame2(index:2) ]`

This showed up in a trace in aspnetcore, which was called via
reflection. And that frame didn't get added to the list because it was
not `MONO_WRAPPER_NONE`, which caused the indexing to get out of sync.

Fixes: https://github.com/dotnet/runtime/issues/41818

* fix warning: remove unused var

* rebase on master, fix errors

* Make frame indices returned from debugger.c, 0-based

- Earlier this 1-based, and it was being adjusted in `MonoProxy`.
- Based on @lewing's suggestion, changing this to be 0-based in
debugger.c, itself, thus removing the need to "fixup" in `MonoProxy`.

* dotnet-format fixes
src/mono/mono/mini/mini-wasm-debugger.c
src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs
src/mono/wasm/debugger/DebuggerTestSuite/MonoJsTests.cs [new file with mode: 0644]
src/mono/wasm/debugger/DebuggerTestSuite/Support.cs
src/mono/wasm/debugger/DebuggerTestSuite/Tests.cs
src/mono/wasm/debugger/tests/debugger-get-properties-test.cs
src/mono/wasm/debugger/tests/debugger-test.cs
src/mono/wasm/runtime/library_mono.js