Uh oh!
There was an error while loading. Please reload this page.
[debugger] Support step into a tail call - #110334
Conversation
mikem8361
left a comment
There was a problem hiding this comment.
I'm not an expert on this code but it looks good to me.
| case DB_IPCE_AFTER_GARBAGE_COLLECTION: | ||
| case DB_IPCE_DISABLE_OPTS_RESULT: | ||
| case DB_IPCE_CATCH_HANDLER_FOUND_RESULT: | ||
| case DB_IPCE_SET_ENABLE_CUSTOM_NOTIFICATION_RESULT: |
There was a problem hiding this comment.
Is this intentionally part of this PR?
There was a problem hiding this comment.
This was causing a boring assertion in the debug build, I can open a separate PR with this fix but it can be here together.
There was a problem hiding this comment.
I don't have a strong preference. Would we want to include that changes as part of a backport to .NET servicing?
There was a problem hiding this comment.
Ah, okay, I will remove it, it will be better for backporting only the change.
| TraceDestination trace; | ||
| if (!g_pEEInterface->TraceStub(ip, &trace) || !g_pEEInterface->FollowTrace(&trace)) | ||
| { | ||
| return false; | ||
| } | ||
| MethodDesc* pTargetMD = | ||
| trace.GetTraceType() == TRACE_UNJITTED_METHOD | ||
| ? trace.GetMethodDesc() | ||
| : g_pEEInterface->GetNativeCodeMethodDesc(trace.GetAddress()); |
There was a problem hiding this comment.
Very minor nit: this looks like a copy of the code in IsTailCallThatReturns, can we consolidate this into a shared function?
runtime/src/coreclr/debug/ee/controller.cpp
Lines 5642 to 5651 in ecefb5e
tommcdon
left a comment
There was a problem hiding this comment.
This looks good to me! Left a couple comments for your review.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
thaystg
commented
Dec 5, 2024
/backport to release/9.0-staging |
Started backporting to release/9.0-staging: https://github.com/dotnet/runtime/actions/runs/12181390920 |
thaystg
commented
Dec 5, 2024
/backport to release/8.0-staging |
Started backporting to release/8.0-staging: https://github.com/dotnet/runtime/actions/runs/12181421329 |
@thaystg backporting to release/8.0-staging failed, the patch most likely resulted in conflicts: $ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch
Applying: Support step into a tail call
.git/rebase-apply/patch:54: trailing whitespace.
case DB_IPCE_SET_ENABLE_CUSTOM_NOTIFICATION_RESULT: warning: 1 line adds whitespace errors.
Using index info to reconstruct a base tree...
M src/coreclr/debug/ee/controller.cpp
M src/coreclr/debug/shared/dbgtransportsession.cpp
Falling back to patching base and 3-way merge...
Auto-merging src/coreclr/debug/shared/dbgtransportsession.cpp
CONFLICT (content): Merge conflict in src/coreclr/debug/shared/dbgtransportsession.cpp
Auto-merging src/coreclr/debug/ee/controller.cpp
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Patch failed at 0001 Support step into a tail call
Error: The process '/usr/bin/git' failed with exit code 128Please backport manually! |
Addressing Tom's and Mikelle's comments Removing unrelated change and adding enum Changing the comment.
68a5b88 to
12f0cf5Comparethaystg
commented
Dec 5, 2024
/backport to release/8.0-staging |
thaystg
commented
Dec 5, 2024
/backport to release/9.0-staging |
Started backporting to release/8.0-staging: https://github.com/dotnet/runtime/actions/runs/12182141863 |
Started backporting to release/9.0-staging: https://github.com/dotnet/runtime/actions/runs/12182142995 |
I got this test case:
https://github.com/dotnet/runtime/blob/80d8c46f071d9e3d8ede2d98cb2c918d9b0d7d31/src/tests/JIT/Regression/JitBlue/Runtime_39581/Runtime_39581.il
Added a breakpoint on line 93 and tried to step into to Runtime_39581.Program::Callee and it behaves like a step over.
With this change the step into works correctly.
Fixes#110441