Skip to content

[debugger][wasm] Support DebuggerProxyAttribute - #56872

Merged
thaystg merged 15 commits into
dotnet:mainfrom
thaystg:thays_implement_debugger_proxy_attribute
Aug 16, 2021
Merged

[debugger][wasm] Support DebuggerProxyAttribute#56872
thaystg merged 15 commits into
dotnet:mainfrom
thaystg:thays_implement_debugger_proxy_attribute

Conversation

@thaystg

@thaystgthaystg commented Aug 4, 2021

Copy link
Copy Markdown
Member
  • Implement support to DebuggerProxyAttribute
  • Implement support to call ToList()
  • Added test to ToList()

image

Fixes#49912

…ger_proxy_attribute
* origin/main: (340 commits)
add RID for Debian 11 (dotnet#56789)
[wasm] [debugger] Skip thread static field (dotnet#56749)
Fix timeouts in coreroot_determinism test in GC stress mode (dotnet#56770)
Use File.OpenHandle in Socket.SendFile directly (dotnet#56777)
accept empty realm for digest auth (dotnet#56369) (dotnet#56455)
[wasm][debugger] Create test Inherited Properties (dotnet#56754)
Mark new test as incompatible with GC Mark4781_1GcStressIncompatible (dotnet#56739)
Ensure MetadataEnumResult is sufficiently updated by MetaDataImport::Enum (dotnet#56756)
[mono] Remove gdb xdebug and binary writer support, it hasn't worked in a while. (dotnet#56759)
Update windows-requirements.md (dotnet#56476)
Update doc and generic parameter name for JsonValue.GetValue (dotnet#56639)
[wasm][debugger] Inspect static class (dotnet#56740)
Fix stack overflow handling issue in GC stress (dotnet#56733)
Use ReflectionOnly as serialization mode in case dynamic code runtime feature is not supported (dotnet#56604)
Move Windows Compat pack to NuGet pack task (dotnet#56686)
Fix build error when building some packages (dotnet#56767)
Simplify JIT shutdown logic in crossgen2 (dotnet#56687)
Fix race in crossdac publishing with PGO (dotnet#56762)
Add DictionaryKeyPolicy support for EnumConverter [dotnet#47765] (dotnet#54429)
Use ComWrappers in some Marshal unit-tests and update platform metadata (dotnet#56595)
...
…ger_proxy_attribute
* origin/main:
disable token info in traces. (dotnet#56780)
[debugger] Fix debugger.break behavior (dotnet#56788)
[mono][wasm] Allow setting env variables with '=' characters in the test runner. (dotnet#56802)
Ecma edit for `conv.ovf.<to type>.un`. (dotnet#56450)
Mark HandleProcessCorruptedStateExceptionsAttribute as obsolete (dotnet#56664)
Enable SxS install of previews on Mac OS (dotnet#56797)
CoreCLR runtime tests + Mono on the x64 iOS simulator (dotnet#43954)
[main] Update dependencies from mono/linker (dotnet#56593)
STJ: Fix deserialization of UInt16 properties (dotnet#56793)
@ghost

ghost commented Aug 4, 2021

Copy link
Copy Markdown

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

Issue Details

image

Fixes #49912

Author:thaystg
Assignees:-
Labels:

area-Debugger-mono

Milestone:-

@thaystgthaystg added the arch-wasm WebAssembly architecture label Aug 4, 2021
@ghost

ghost commented Aug 4, 2021

Copy link
Copy Markdown

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

image

Fixes #49912

Author:thaystg
Assignees:-
Labels:

arch-wasm, area-Debugger-mono

Milestone:-

Comment threadsrc/mono/wasm/debugger/BrowserDebugProxy/MemberReferenceResolver.cs Outdated
Comment threadsrc/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs Outdated
@radical

Copy link
Copy Markdown
Member

How is this related to #49912 ?

radical
radical previously requested changes Aug 11, 2021

@radicalradical left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please excuse my lack of knowledge about the debugger agent/sdb etc bits :)
First read through.

Comment threadsrc/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs Outdated
Comment threadsrc/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs Outdated
Comment threadsrc/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs Outdated
Comment threadsrc/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs
Comment threadsrc/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs
Comment threadsrc/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs Outdated
Comment threadsrc/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs Outdated
Comment threadsrc/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs Outdated
Comment threadsrc/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs Outdated
Comment threadsrc/mono/wasm/debugger/DebuggerTestSuite/CustomViewTests.cs Outdated

@thaystgthaystg left a comment

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

How is this related to #49912 ?

I also implemented now the ToList(), I thought it was working, that is why I didn't test. And with this implementation after calling ToList() it will show a beautiful list as it's in the image in the PR description.

@lewing

Copy link
Copy Markdown
Member

/azp run runtime

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@lewing
lewing requested a review from radicalAugust 15, 2021 21:00

@radicalradical left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Most of the suggestions don't have to be in this PR itself. So, reviewing this with comments. More tests would be useful too, like for DebuggerProxy stuff (eg. proxy type in different assembly).

var className = await proxy.SdbHelper.GetTypeNameOriginal(sessionId, typeIds[0], token);
if (methodId == 0) //try to search on System.Linq.Enumerable
{
var linqTypeId = await proxy.SdbHelper.GetTypeByName(sessionId, "System.Linq.Enumerable", token);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is a static class, so can we cache it?

{
var linqTypeId = await proxy.SdbHelper.GetTypeByName(sessionId, "System.Linq.Enumerable", token);
var linqInitialize = await proxy.SdbHelper.TypeInitialize(sessionId, linqTypeId, token);
Console.WriteLine($"linqInitialize - {linqInitialize}");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can be dropped

Comment threadsrc/mono/wasm/debugger/BrowserDebugProxy/MemberReferenceResolver.cs Outdated
return retDebuggerCmdReader.ReadString();
}

public async Task<string> GetAssemblyNameOriginal(SessionId sessionId, int assembly_id, CancellationToken token)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

GetAssemblyNameOriginal, and GetAssemblyNameFull seem slightly misnamed.
Maybe, GetAssemblyNameOriginal -> GetFullAssemblyName or GetAssemblyNameFull
and GetAssemblyNameFull -> GetAssemblyFileNameFromId

}

[Flags]
internal enum GetObjectCommandType

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe rename to GetObjectCommandOptions

}

public async Task<string> GetTypeName(SessionId sessionId, int type_id, CancellationToken token)
public async Task<string> GetDebuggerDisplayAttribute(SessionId sessionId, int objectId, int typeId, CancellationToken token)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

rename to GetValueFromDebuggerDisplayAttribute

return retDebuggerCmdReader.ReadInt32();
}

public async Task<JArray> GetDebuggerProxyAttribute(SessionId sessionId, int objectId, int typeId, CancellationToken token)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

rename to GetValuesFromDebuggerProxyAttribute

return value?.Value<bool>();
case "object":
return null;
return variable;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Tests needed for this

Comment on lines +1120 to 1133
//reading buffer only to advance the reader to the next cattr
var parmCount = retDebuggerCmdReader.ReadInt32();
for (int j = 0; j < parmCount; j++)
{
//to typed_args
await CreateJObjectForVariableValue(sessionId, retDebuggerCmdReader, "varName", false, -1, token);
}
else

parmCount = retDebuggerCmdReader.ReadInt32();
for (int j = 0; j < parmCount; j++)
{
var parmCount = retDebuggerCmdReader.ReadInt32();
for (int j = 0; j < parmCount; j++)
{
//to read parameters
await CreateJObjectForVariableValue(sessionId, retDebuggerCmdReader, "varName", false, -1, token);
}
//to named_args
await CreateJObjectForVariableValue(sessionId, retDebuggerCmdReader, "varName", false, -1, token);
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: do it in a loop that runs twice? or a local method that gets called twice?

@radical
radical self-requested a review August 16, 2021 05:39

@radicalradical left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't see any blockers here. The other suggestions can be taken up in other PRs too. Thanks for addressing the feedback! 👍

@lewing

Copy link
Copy Markdown
Member

failures are #57501

@thaystg
thaystg merged commit 91673de into dotnet:mainAug 16, 2021
thaystg added a commit to thaystg/runtime that referenced this pull request Aug 16, 2021
…information
# By dotnet-maestro[bot] (4) and others
# Via GitHub
* origin/main: (58 commits)
Localized file check-in by OneLocBuild Task (dotnet#57384)
[debugger][wasm] Support DebuggerProxyAttribute (dotnet#56872)
Account for type mismatch of `FIELD_LIST` members in LSRA (dotnet#57450)
Qualify `sorted_table` allocation with `nothrow` (dotnet#57467)
Rename transport packages to follow convention (dotnet#57504)
Generate proper DWARF reg num for ARM32 (dotnet#57443)
Enable System.Linq.Queryable and disable dotnet#50712 (dotnet#57464)
Mark individual tests for 51211 (dotnet#57463)
Fix Length for ReadOnlySequence created out of sliced Memory owned by MemoryManager (dotnet#57479)
Add JsonConverter.Write/ReadAsPropertyName APIs (dotnet#57302)
Remove workaround for dotnet/sdk#19482 (dotnet#57453)
Do not drain HttpContentReadStream if the connection is disposed (dotnet#57287)
[mono] Fix a few corner case overflow operations (dotnet#57407)
make use of ports in SPN optional (dotnet#57159)
Fixed H/3 stress server after the last Kestrel change (dotnet#57356)
disable a failing stress test. (dotnet#57473)
Eliminate temporary byte array allocations in the static constructor of `IPAddress`. (dotnet#57397)
Update dependencies from https://github.com/dotnet/emsdk build 20210815.1 (dotnet#57447)
[main] Update dependencies from mono/linker (dotnet#57344)
Improve serializer performance (dotnet#57327)
...
# Conflicts:
#	src/mono/wasm/debugger/BrowserDebugProxy/MemberReferenceResolver.cs
#	src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs
#	src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs
@ghostghost locked as resolved and limited conversation to collaborators Sep 15, 2021
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

arch-wasmWebAssembly architecturearea-Debugger-mono

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Blazor WebAssembly debugger doesn't load values properly

3 participants

@thaystg@radical@lewing