[mono][debugger] Support debug after hot-reload #55220

thaystg · 2021-07-06T18:16:19Z

Implemented the runtime side.
And the client side for wasm.
Added tests for wasm.

To do in another PR, implement support on debugger-libs for Android.

* origin/main: (27 commits) [mono][llvm] Only emit 'LLVM failed' messages on verbosity > 0. (dotnet#55060) Http2Stream throws a wrapped Http2ConnectionException on GO_AWAY (dotnet#54625) [main] Update dependencies from dnceng/internal/dotnet-optimization dotnet/arcade dotnet/xharness dotnet/hotreload-utils (dotnet#55007) disable a failing test. (dotnet#55063) [mono][wasm] Disable some tests which crash on AOT. (dotnet#55054) Fix fix_allocation_context for regions (dotnet#54931) Delete stale references to System.IO.FileSystem.Primitives (dotnet#55041) Add binplaced analyzers to ASP.NET transport package (dotnet#55042) [mono] Enable many HardwareIntrinsic tests on wasm Delete `compQuirkForPPP`. (dotnet#55050) [Mono] Condition Workload AOT import to be osx only (dotnet#55040) package native quic library (dotnet#54992) Make GlobalizationMode code consistent (dotnet#55039) Expand PerfMap format to support metadata for symbol indexation (dotnet#53792) [debugger]Componentize debugger (dotnet#54887) [Mono] Include loaded interpreter methods as EventPipe session rundown method events. (dotnet#54953) Delete stale ActiveIssue from HttpHeadersTest (dotnet#55027) Poison address-exposed user variables in debug (dotnet#54685) Recategorize emsdk dependency (dotnet#55028) Remove the the wasm AOT specific test project exclusions (dotnet#54988) ...

…ide for wasm.

ghost · 2021-07-06T18:16:27Z

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

Issue Details

Implemented the runtime side.
And the client side for wasm.

To do in another PR, implement support on debugger-libs for Android.

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

ghost · 2021-07-06T18:17:14Z

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

Issue Details

Implemented the runtime side.
And the client side for wasm.

To do in another PR, implement support on debugger-libs for Android.

Author:	thaystg
Assignees:	-
Labels:	`arch-wasm`, `area-Debugger-mono`
Milestone:	-

lambdageek · 2021-07-06T19:49:03Z

src/mono/mono/component/debugger-agent.c

+static void 
+send_enc_delta (MonoImage *image, gconstpointer dmeta_bytes, int32_t dmeta_len, gconstpointer dpdb_bytes, int32_t dpdb_len)
+{	
+	//TODO: if it came from debugger we don't need to pass the parameters back, they are already on debugger client side.


This is related to issue #55228 - we can assume that if the debugger is attached, all updates are coming through the debugger command, not through the managed icall.

I cannot change this right now for WASM, am I right?
Because for wasm the updates are not coming through debugger.

That's right. I think for wasm the updates are always through dotnet watch, and not through the debugger.

lambdageek · 2021-07-06T20:01:55Z

src/mono/mono/component/hot_reload.c

+			if (!cols [MONO_METHODBODY_SEQ_POINTS])
+				return NULL;
+
+			const char *ptr = mono_metadata_blob_heap (image_dppdb, cols [MONO_METHODBODY_SEQ_POINTS]);


I'm a little bit surprised this is a heap offset in the delta pdb. For the metadata, the heap offset is in the combined baseline+delta heaps. (ie if originally the blob heap was 1000 bytes and the delta metadata has 500 more bytes, then the metadata tables in the delta will have offsets like 1001-1500, not 1-500.)

But these formats aren't very consistent. So I assume you saw in a testcase that it's always offsets in the delta pdb heap only.

src/mono/mono/metadata/icall.c

lambdageek · 2021-07-06T20:11:20Z

src/mono/mono/mini/interp/transform.c

@@ -9775,8 +9775,7 @@ mono_interp_transform_method (InterpMethod *imethod, ThreadContext *context, Mon

 	MonoJitMemoryManager *jit_mm = get_default_jit_mm ();
 	jit_mm_lock (jit_mm);
-	if (!g_hash_table_lookup (jit_mm->seq_points, imethod->method))
-		g_hash_table_insert (jit_mm->seq_points, imethod->method, imethod->jinfo->seq_points);
+	g_hash_table_replace (jit_mm->seq_points, imethod->method, imethod->jinfo->seq_points);


I think we also need to do something in copy_imethod_for_frame:

In hot reload, the existing version of a method that is already running will finish with the old body, and new calls will use the new method body. So it is possible to hit a breakpoint in an old version of the method. So I think we might need to make a copy of the old seq points in copy_imethod_for_frame. What do you think?

I'm not sure, this seq_points would only be used to set breakpoint. So it would be possible to be debugging this method with old body?

Like: Thread A is running MethodA and it's stopped in a breakpoint.
In debugger we change something in MethodA so it will replace the MethodA body.
On Thread A we set another breakpoint in another line using the old body and then resume.

Would it be possible to do? While it is stopped in a breakpoint change something and send the deltas to runtime?

Like: Thread A is running MethodA and it's stopped in a breakpoint.
In debugger we change something in MethodA so it will replace the MethodA body.
On Thread A we set another breakpoint in another line using the old body and then resume.

I think this is possible. @tmat - VS can show the old and the new body of MethodA? Is that something that is already supported by VS, or something we are considering to do in the future?

src/mono/wasm/debugger/tests/ApplyUpdateReferencedAssembly/MethodBody1_v2.cs

Co-authored-by: Aleksey Kliger (λgeek) <[email protected]>

lewing · 2021-07-10T01:19:52Z

/azp run

azure-pipelines · 2021-07-10T01:19:57Z

You have several pipelines (over 10) configured to build pull requests in this repository. Specify which pipelines you would like to run by using /azp run [pipelines] command. You can specify multiple pipelines using a comma separated list.

lewing · 2021-07-10T01:23:00Z

/azp run runtime

azure-pipelines · 2021-07-10T01:23:25Z

Azure Pipelines successfully started running 1 pipeline(s).

lambdageek

🎉

thaystg added 7 commits July 1, 2021 17:58

Add test case for debug after hot reload.

45aa206

Implementing debug after hot reload, on runtime side, and on client s…

5164999

…ide for wasm.

Reuse some code.

e8bbbfc

Remove unrelated comment.

79e00bf

Reuse code.

84de82a

Refactor new code.

38712ad

thaystg requested review from lewing and lambdageek July 6, 2021 18:16

dotnet-issue-labeler bot added the area-Debugger-mono label Jul 6, 2021

thaystg changed the title ~~[mono][debugger] Support debug after hotreload~~ [mono][debugger] Support debug after hot-reload Jul 6, 2021

thaystg added the arch-wasm WebAssembly architecture label Jul 6, 2021

thaystg requested a review from radical July 6, 2021 18:21

Fix log.

f13a500

lambdageek reviewed Jul 6, 2021

View reviewed changes

lambdageek mentioned this pull request Jul 6, 2021

Mono Method Body Replacement #44806

Closed

51 tasks

lambdageek reviewed Jul 6, 2021

View reviewed changes

src/mono/mono/metadata/icall.c Outdated Show resolved Hide resolved

lambdageek reviewed Jul 6, 2021

View reviewed changes

src/mono/wasm/debugger/tests/ApplyUpdateReferencedAssembly/MethodBody1_v2.cs Outdated Show resolved Hide resolved

thaystg and others added 3 commits July 6, 2021 22:35

Apply suggestions from code review

781969b

Co-authored-by: Aleksey Kliger (λgeek) <[email protected]>

Fixing hot reload without debug information.

e337da5

replacing spaces with tabs.

48d285d

thaystg marked this pull request as ready for review July 7, 2021 18:19

thaystg requested review from BrzVlad, marek-safar and vargaz as code owners July 7, 2021 18:19

Fixing CI.

fb9a4f1

lambdageek approved these changes Jul 12, 2021

View reviewed changes

Merge branch 'main' into thays_hot_reload_debug

0736c69

thaystg merged commit c786e4f into dotnet:main Jul 13, 2021

ManickaP mentioned this pull request Jul 20, 2021

[QUIC] Remove AppContext switch from S.N.Quic #56027

Merged

ghost locked as resolved and limited conversation to collaborators Aug 12, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[mono][debugger] Support debug after hot-reload #55220

[mono][debugger] Support debug after hot-reload #55220

thaystg commented Jul 6, 2021 •

edited

Loading

ghost commented Jul 6, 2021

ghost commented Jul 6, 2021

lambdageek Jul 6, 2021

thaystg Jul 7, 2021

lambdageek Jul 7, 2021

lambdageek Jul 6, 2021 •

edited

Loading

lambdageek Jul 6, 2021

thaystg Jul 7, 2021 •

edited

Loading

lambdageek Jul 7, 2021

lewing commented Jul 10, 2021

azure-pipelines bot commented Jul 10, 2021

lewing commented Jul 10, 2021

azure-pipelines bot commented Jul 10, 2021

lambdageek left a comment

[mono][debugger] Support debug after hot-reload #55220

[mono][debugger] Support debug after hot-reload #55220

Conversation

thaystg commented Jul 6, 2021 • edited Loading

ghost commented Jul 6, 2021

ghost commented Jul 6, 2021

lambdageek Jul 6, 2021

Choose a reason for hiding this comment

thaystg Jul 7, 2021

Choose a reason for hiding this comment

lambdageek Jul 7, 2021

Choose a reason for hiding this comment

lambdageek Jul 6, 2021 • edited Loading

Choose a reason for hiding this comment

lambdageek Jul 6, 2021

Choose a reason for hiding this comment

thaystg Jul 7, 2021 • edited Loading

Choose a reason for hiding this comment

lambdageek Jul 7, 2021

Choose a reason for hiding this comment

lewing commented Jul 10, 2021

azure-pipelines bot commented Jul 10, 2021

lewing commented Jul 10, 2021

azure-pipelines bot commented Jul 10, 2021

lambdageek left a comment

Choose a reason for hiding this comment

thaystg commented Jul 6, 2021 •

edited

Loading

lambdageek Jul 6, 2021 •

edited

Loading

thaystg Jul 7, 2021 •

edited

Loading