[wasm][debugger] Reuse debugger-agent on wasm debugger #52300
Conversation
…ot of code that does the same thing on mini-wasm-debugger.
|
Tagging subscribers to this area: @thaystg Issue DetailsPROTOTYPE!!!
|
make -C src/mono/wasm/ run-debugger-tests TEST_FILTER=DebuggerTests.SteppingTests.InspectValueTypeMethodArgsWhileStepping is working without use_cfo.
Failed: 271, Passed: 220
| for (i = strlen (lookup_name) - 1; i >= 0; --i) { | ||
| if (lookup_name [i] == '.') { | ||
| lookup_name [i] = 0; | ||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
I copied this code from mini-wasm-debugger. Probably yes.
src/mono/mono/mini/debugger-agent.c
Outdated
| buffer_add_int(buf, -1); | ||
| break; | ||
| } | ||
| mono_assembly_name_free_internal (aname); |
There was a problem hiding this comment.
this should be done in the error cases too.
| MonoTypeNameFormat format = MONO_TYPE_NAME_FORMAT_FULL_NAME; | ||
| if (CHECK_PROTOCOL_VERSION(2, 61)) | ||
| format = (MonoTypeNameFormat) decode_int (p, &p, end); |
There was a problem hiding this comment.
Would it make sense to make this a macro, or function, so the protocol version stuff doesn't have to be here?
src/mono/mono/mini/debugger-agent.c
Outdated
| @@ -9897,6 +9939,62 @@ wait_for_attach (void) | |||
| return TRUE; | |||
| } | |||
|
|
|||
| ErrorCode | |||
| mono_process_dbg_packet (int id, CommandSet command_set, int command, gboolean *no_reply, guint8 *p, guint8 *end, Buffer *buf) | |||
There was a problem hiding this comment.
Maybe rename p to something descriptive
| /* Enums are sent as a normal vtype */ | ||
| if (is_enum) | ||
| return ERR_NOT_IMPLEMENTED; | ||
| if (CHECK_PROTOCOL_VERSION(2, 61)) | ||
| decode_byte (buf, &buf, limit); |
There was a problem hiding this comment.
What happens in versions other than 2.61 now?
Implementing handling error on debugger-agent.
Failed: 248, Passed: 243
Failed: 243, Passed: 248
Failed: 226, Passed: 265
Failed: 192, Passed: 299
Failed: 184, Passed: 307
…oints tests are passing. Failed: 172, Passed: 319
Failed: 156, Passed: 335
Failed: 148, Passed: 343
Failed: 99, Passed: 392
Failed: 53, Passed: 438
Failed: 31, Passed: 460
Failed: 30, Passed: 461
thaystg
requested review from
lambdageek,
marek-safar,
SamMonoRT and
vargaz
as code owners
src/mono/wasm/debugger/BrowserDebugProxy/MemberReferenceResolver.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Larry Ewing <[email protected]>
Co-authored-by: Larry Ewing <[email protected]>
Co-authored-by: Larry Ewing <[email protected]>
|
The amount of this error in the Helix logs is worrying: |
|
Thanks @filipnavara I'll take a look! :) |
There was a problem hiding this comment.
Let's do a lot of manual testing now.
cc @radical @pavelsavara @radekdoulik @kg @Daniel-Genkin-MS
|
I just did some testing on Windows and found the following behaviours. Although I'm not sure if they are directly applicable to this PR. Although other than that, this works really well and is really cool!!
|
|
@Daniel-Genkin thank you for the testing
This needs to have a test added and needs to be fixed. I'm not sure I consider it blocking for landing this pr, especially if it only diagnostics and doesn't surface to the UI.
This sounds like #54040 |
This also happens in the version without my changes, we need to investigate if debugger proxy is returning this error or the browser is returning this error. |
This one I think that always happens without my changes, but probably we should also try to fix. |
|
@Daniel-Genkin thanks for testing!!! :) |
|
Please, if anyone find anything, please comment here and I will fix and change what is suggested in another PR. |
…nitial_config * origin/main: (362 commits) [wasm][debugger] Reuse debugger-agent on wasm debugger (dotnet#52300) Put Crossgen2 in sync with dotnet#54235 (dotnet#54438) Move System.Object serialization to ObjectConverter (dotnet#54436) Move setting fHasVirtualStaticMethods out of sanity check section (dotnet#54574) [wasm] Compile .bc->.o in parallel, before passing to the linker (dotnet#54053) Change PathInternal.IsCaseSensitive to a constant (dotnet#54340) Make mono_polling_required a public symbol (dotnet#54592) Respect EventSource::IsSupported setting in more codepaths (dotnet#51977) Root ComActivator for hosting (dotnet#54524) Add ILLink annotations to S.D.Common related to DbConnectionStringBuilder (dotnet#54280) Fix finalizer issue with regions (dotnet#54550) Add support for multi-arch install locations (dotnet#53763) Update library testing docs page to reduce confusion (dotnet#54324) [FileStream] handle UNC and device paths (dotnet#54483) Update NetAnalyzers version (dotnet#54511) Added runtime dependency to fix the intermittent test failures (dotnet#54587) Disable failing System.Reflection.Tests.ModuleTests.GetMethods (dotnet#54564) [wasm] Move AOT builds from `runtime-staging` to `runtime` (dotnet#54577) Keep obj node for ArrayIndex. (dotnet#54584) Disable another failing MemoryCache test (dotnet#54578) ...
…bugger2 * origin/main: (107 commits) Disable MacCatalyst arm64 PR test runs on staging pipeline (dotnet#54678) [WASM] Fix async/await in config loading (dotnet#54652) Fix for heap_use_after_free flagged by sanitizer (dotnet#54679) [wasm] Bump emscripten to 2.0.23 (dotnet#53603) Fix compiler references when building inside VS (dotnet#54614) process more TLS frames at one when available (dotnet#50815) Add PeriodicTimer (dotnet#53899) UdpClient with span support (dotnet#53429) exclude fragile tests (dotnet#54671) get last error before calling a method that might fail as well (dotnet#54667) [FileStream] add tests for device and UNC paths (dotnet#54545) Fix sporadic double fd close (dotnet#54660) Remove Version.Clone from AssemblyName.Clone (dotnet#54621) [wasm] Enable fixed libraries tests (dotnet#54641) [wasm] Fix blazor/aot builds (dotnet#54651) [mono][wasm] Fix compilation error on wasm (dotnet#54659) Fix telemetry for Socket connects to Dns endpoints (dotnet#54071) [wasm] Build static components; include hot_reload in runtime (dotnet#54568) [wasm][debugger] Reuse debugger-agent on wasm debugger (dotnet#52300) Put Crossgen2 in sync with dotnet#54235 (dotnet#54438) ...
ghost
locked as resolved and limited conversation to collaborators
This will remove a lot of code that does the same thing on mini-wasm-debugger.
The code for debugger before this PR was shared between mini-wasm-debugger, library_mono.js and the BrowserDebugProxy. With this new approach the important code for the debugger are mainly on debugger-agent and BrowserDebugProxy.
This PR affects the size of dotnet.wasm and dotnet.js as is described below.