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.

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

WASM Debugging: Problem with inherited class in QuickWatch while debugging in Blazor pages. #41406

Closed
KevinValmo opened this issue Aug 6, 2020 · 5 comments · Fixed by mono/mono#20318 or #41480
Assignees
Labels
arch-wasm WebAssembly architecture area-Debugger-mono
Milestone

Comments

@KevinValmo
Copy link

There is an issue when i'm trying to QuickWatch an instance of an Object inherited by other class (even if i hover with mouse).
I'm working with Blazor Wasm.

Here there are some image.

Here I am watching an object named a of Type HubLed that inherit from BaseHubComponent
image

Here there is a partial class HubLed where we can notice that we can access the public property ConnectionId
image

Here there is the Razor page in where i specify the inheritance.
image

Here we can see some property in BaseHubComponent
image

We can notice that property BaseHubComponent.ConnectionId is public, and i can access this property from our class HubLed but in debug mode i can not see it.

So the property inherited can't be seen in debug mode.

However, i'm sorry for my english...

Hope there is necessary infos.

Infos

(_public abstract_) **BaseComponent**.cs : ComponentBase
(_public abstract_) _BaseHubComponent_.cs : **BaseComponent**
(_public partial_) HubLed.razor : _BaseHubComponent_
(_public partial_) HubLed.razor.cs

Further technical details

  • Target Framework: .NET standard 2.1 for Blazor project.
  • The IDE is Visual Studio 16.8.0 Preview 1 (but i see this issue in previuos versions)
@KevinValmo
Copy link
Author

Anyone has faced this problem in Debug mode?

@pranavkm pranavkm transferred this issue from dotnet/aspnetcore Aug 26, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-Diagnostics-coreclr untriaged New issue has not been triaged by the area owner labels Aug 26, 2020
@ghost
Copy link

ghost commented Aug 26, 2020

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

@pranavkm pranavkm added arch-wasm WebAssembly architecture and removed area-Diagnostics-coreclr untriaged New issue has not been triaged by the area owner labels Aug 26, 2020
@pranavkm pranavkm changed the title Problem with inherited class in QuickWatch while debugging in Blazor pages. WASM Debugging: Problem with inherited class in QuickWatch while debugging in Blazor pages. Aug 26, 2020
@ghost
Copy link

ghost commented Aug 26, 2020

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

@marek-safar marek-safar added this to the 5.0.0 milestone Aug 26, 2020
@marek-safar
Copy link
Contributor

/cc @radical

@radical
Copy link
Member

radical commented Sep 2, 2020

Closing as duplicate of #40245 .

@radical radical closed this as completed Sep 2, 2020
monojenkins pushed a commit to monojenkins/mono that referenced this issue Sep 3, 2020
- surface inherited fields, and properties
- we try to support `Runtime.getProperties`'s two arguments:
    - `ownProperties`, and `accessorsOnly`

    - `ownProperties`: for JS, this means return only the object's own
    members (not inherited ones)
    - `accessorsOnly`: for JS, this means return all the getters

Actual implementation:

- In practice, VSCode, and Chrome debugger seem to only send
`{ ownProperties: true, accessorsOnly: false }`,
and `{ ownProperties: false, accessorsOnly: true }`. The combination of
which means - that we don't return any inherited fields!

- But we want to show inherited fields too, so to get that behavior we
essentially *ignore* `ownProperties`. IOW,

    - `ownProperties`: we return all fields, and properties
    - `accessorsOnly`: we return only the getters, including the
    inherited ones

- Another thing to note is the case for auto-properties
    - these have a backing field
    - and we usually return the backing field's value, instead of
    returning a getter
    - To continue with that, auto-properties are *not* returned for
    `accessorsOnly`

- The code in `mini-wasm-debugger.c` does handle these two arguments,
but that is currently disabled by not passing the args to debugger.c at
all
    - Instead, we get the *full* list of members, and try to filter it
    in `library_mono.js`
    - which includes handling property overrides, or shadowing by new
    properties/fields in derived classes

Fixes dotnet/runtime#40245
Fixes dotnet/runtime#41406 .
akoeplinger pushed a commit to mono/mono that referenced this issue Sep 4, 2020
- surface inherited fields, and properties
- we try to support `Runtime.getProperties`'s two arguments:
    - `ownProperties`, and `accessorsOnly`

    - `ownProperties`: for JS, this means return only the object's own
    members (not inherited ones)
    - `accessorsOnly`: for JS, this means return all the getters

Actual implementation:

- In practice, VSCode, and Chrome debugger seem to only send
`{ ownProperties: true, accessorsOnly: false }`,
and `{ ownProperties: false, accessorsOnly: true }`. The combination of
which means - that we don't return any inherited fields!

- But we want to show inherited fields too, so to get that behavior we
essentially *ignore* `ownProperties`. IOW,

    - `ownProperties`: we return all fields, and properties
    - `accessorsOnly`: we return only the getters, including the
    inherited ones

- Another thing to note is the case for auto-properties
    - these have a backing field
    - and we usually return the backing field's value, instead of
    returning a getter
    - To continue with that, auto-properties are *not* returned for
    `accessorsOnly`

- The code in `mini-wasm-debugger.c` does handle these two arguments,
but that is currently disabled by not passing the args to debugger.c at
all
    - Instead, we get the *full* list of members, and try to filter it
    in `library_mono.js`
    - which includes handling property overrides, or shadowing by new
    properties/fields in derived classes

Fixes dotnet/runtime#40245
Fixes dotnet/runtime#41406 .

Co-authored-by: radical <[email protected]>
radical added a commit that referenced this issue Sep 15, 2020
)

This fixes variable inspection and expression evaluation in the WASM debugger. It markedly improves developer experience in the debugger around watch expressions, locals, and object properties. It fixes the issues listed below, improves error handling and adds extensive tests.

Issues fixed:

#41818
#41744
#41846
mono/mono#20310
#41406
#40245
#41447
#41276
#41990

---

* [wasm][debugger]  Support deep member accesses for EvaluteOnCallFrame (#40836)

* [wasm][debugger] Support deep member accesses for EvaluteOnCallFrame

Eg. `obj.Property.X`, or `obj.Y + obj.Z.p`

Each of the member access expressions (like `a.b.c`) must be of only
primitive types. Though if the expression is a single member access (and
nothing else, like `"a.b.c"`), then that can be a non-primitive type.

This works by sending the expression to the browser, where it gets
resolved by `library_mono.js`. And that takes an easy route for doing
this, essentially just fetching the list of locals/properties, and using
that.

There are better ways to do this, that will be explored later.

* [wasm][debugger][tests] Remove some debug spew

* [wasm][debugger] fix formatting with dotnet-format

(cherry picked from commit 907f7da)

* [wasm][debugger] Fix expression evaluation when it is a reference (#41135)

* [wasm][debugger] Fix expression evaluation when it is a reference

.. preceded by spaces. Eg: `"  foo.dateTime", or `"  foo.count"`

* Explanation of the fix:

- these particular expressions end up in the code path where we get a
SimpleMemberAccessExpression, and only one member access (like `a.b.c`)
was found.

- that code path had
    `return memberAccessValues[0]?["value"]?.Value<JObject>();`

  - which is incorrect, and we need to return `memberAccessValues[0]`,
  which is the value itself.

(cherry picked from commit ec59f65)

* [wasm][debugger] Correctly handle empty, and whitespace-only strings (#41424)

There are two cases being fixed here:

1. str=='', or str=' '
	- We check `str_value == 0`, and for the above cases JS returns
	true, due to type coercion.
	- So, we show the result as a null string.

2. str==null
	- debugger.c adds the value for this with `mono_wasm_add_typed_value ("string", NULL, 0)`
	- the second argument is converted to a string with
	  `Module.UTF8ToString(..)`, but when it's `0`/NULL, we get an
	  empty string. And that becomes a null string, because of (1).

Fixing this by using `===` operator to avoid type coercion.

Fixes #41276

(cherry picked from commit 0795094)

* [wasm] Disable an extraneous debug message (#41468)

Recent debugger test runs were showing lot of
`CWL: Failed to lookup sequence point` messages.

These are being shown for cases like:

`CWL: list_frames: Failed to lookup sequence point. method: runtime_invoke_direct_void, native_offset: 56`

This is a warning, and doesn't need to be emitted by default.

(cherry picked from commit c4841c5)

* [wasm][debugger] Breakpoint stopping after it's removed (#41479)

* Remove unnecessary WriteLine.
Fix #41447.

* Creating unit test about remove breakpoint.

* Implementing @radical suggestions!

(cherry picked from commit 513ade6)

* [wasm][debugger] Add support for surfacing inherited members (#41480)

* [wasm][debugger][tests] Update to use `TDateTime`

- this ensures that we check the datetime, and some property getters on
it, wherever we have a datetime.

* [wasm][debugger][tests] Add labels to more checks

* [wasm][debugger] Add support for surfacing inherited members

- surface inherited fields, and properties
- we try to support `Runtime.getProperties`'s two arguments:
    - `ownProperties`, and `accessorsOnly`

    - `ownProperties`: for JS, this means return only the object's own
    members (not inherited ones)
    - `accessorsOnly`: for JS, this means return all the getters

Actual implementation:

- In practice, VSCode, and Chrome debugger seem to only send
`{ ownProperties: true, accessorsOnly: false }`,
and `{ ownProperties: false, accessorsOnly: true }`. The combination of
which means - that we don't return any inherited fields!

- But we want to show inherited fields too, so to get that behavior we
essentially *ignore* `ownProperties`. IOW,

    - `ownProperties`: we return all fields, and properties
    - `accessorsOnly`: we return only the getters, including the
    inherited ones

- Another thing to note is the case for auto-properties
    - these have a backing field
    - and we usually return the backing field's value, instead of
    returning a getter
    - To continue with that, auto-properties are *not* returned for
    `accessorsOnly`

- The code in `mini-wasm-debugger.c` does handle these two arguments,
but that is currently disabled by not passing the args to debugger.c at
all
    - Instead, we get the *full* list of members, and try to filter it
    in `library_mono.js`
    - which includes handling property overrides, or shadowing by new
    properties/fields in derived classes

* [wasm][debugger][tests] Fix simple warnings

* [wasm][debugger][tests] Fix warnings introduced in this PR

* [wasm][debugger][tests] Fix indentation

* [wasm][debugger] Correctly handle local structs in async methods

- When we have a struct local in an async instance method, it doesn't
get expanded, since we have a containerId (the async object), and we can
expand/access it later.

- When the IDE asks us to expand it with `{accessorPropertiesOnly: true}`:
    - we get the expanded json, but `_filter_automatic_properties` tries
    to return just the accessors, but that doesn't handle the expanded
    members of nested structs!
    - That is done in `extract_and_cache_value_types`, which is run *after*
    `_filter_automatic_properties`, but by that time we have already
    lost the expanded members!

    - So, `_get_vt_properties` fails with `Unknown valuetype id`,
    because it doesn't have anything to return at that point.

- This is being solved by ignoring the getProperties args in case of
expanding valuetypes.
    - that means that we can correctly extract, and cache the whole
    object.
    - And after that, we can return accessors/others, based on the args.

* [wasm][debugger] Fix warnings in debugger-test-app, and turn on warnAsError

* For some cases, debugger seems to give the actual method name instead of MoveNext for async methods

(cherry picked from commit b25b2bc)

* [wasm][debugger] Add support for Nullable<T>  (#41559)

* [wasm][debugger] Add support for Nullable<T>

Return the value, or null.

Fixes mono/mono#20310

* Address review feedback - merge functions

* [wasm][debugger] run dotnet-format on the debugger test app

* [wasm][debugger] simplify function sig, based on usage

- addresses review feedback from @lewing

* [wasm][debugger] Simplify the function further, based on @lewing's

.. excellent suggestion!

(cherry picked from commit 66f4b4b)

* [wasm][debugger] Show actual data for boxed values (#41562)

* [wasm][debugger] Add support for Nullable<T>

Return the value, or null.

Fixes mono/mono#20310

* Address review feedback - merge functions

* [wasm][debugger] run dotnet-format on the debugger test app

* [wasm][debugger] simplify function sig, based on usage

- addresses review feedback from @lewing

* [wasm][debugger] Simplify the function further, based on @lewing's

.. excellent suggestion!

* [wasm][debugger] Show actual data for boxed values

Eg. `object o = "foobar"`

This will show the string `"foobar"`, instead of an object, in the
debugger.

(cherry picked from commit 4fd87bc)

* [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: #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

(cherry picked from commit 2e4e75b)

* Fix wasm sample after that was broken after this PR: #40478 (#41277)

(cherry picked from commit f384168)

* [wasm][debugger] Avoid infinite loop when we have a boxed `new object` (#42059)

* [wasm][debugger] Avoid infinite loop when we have a boxed `new object`

Eg. `object o = new object(); object o1 = o;`
- Avoid infinitely looping for `o1`

* [wasm][debugger] Handle valuetypes boxed in classes that are not

.. type `object`.

Prompted by @lambdageek's comment - #42059 (comment)

(cherry picked from commit b58eba3)

Co-authored-by: Thays Grazia <[email protected]>
@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.