Skip to content

Testing a graceful failure of fetching codeInfo to see how much this mitigates test failures. #124089

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.

Sign up for GitHub

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

Open
rcj1 wants to merge 3 commits into
base: main
Choose a base branch
from
+13 −8
Open

Testing a graceful failure of fetching codeInfo to see how much this mitigates test failures. #124089

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6ab1a4b
testing
rcj1 Feb 6, 2026
baebee5
Update debugdebugger.cpp
rcj1 Feb 6, 2026
b278311
Add temporary comment for testing in debugdebugger.cpp
rcj1 Feb 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 13 additions & 8 deletions src/coreclr/vm/debugdebugger.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,14 +298,19 @@ extern "C" void QCALLTYPE AsyncHelpers_AddContinuationToExInternal(
_ASSERTE(pException != NULL);
// populate exception with information from the continuation object
EECodeInfo codeInfo((PCODE)diagnosticIP);
_ASSERTE(codeInfo.IsValid());
MethodDesc* methodDesc = codeInfo.GetMethodDesc();
StackTraceInfo::AppendElement(
pException,
(UINT_PTR)diagnosticIP,
0,
methodDesc,
NULL);
// Interpreter diagnostic IP is not recognized by codeInfo, so this does not work with interpreted code.
// This is a temporary measure to enable testing and once the issue is fixed this condition should be replaced by an assert.
Copy link
Member

@jkotas jkotas Feb 6, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

What happens if we get a random pointer here that happens to be valid and maps to some completely unrelated method? I doubt we are robust against mapping debug info onto a completely unrelated method. I think this is just going to make it crash less often, it is not a reliable workaround for the bug.

I think stable CI is more important than having a bit more of async testing enabled ASAP. I would take the other PR to disable the runtime async with interpreter instead.

Copy link
Contributor Author

@rcj1 rcj1 Feb 6, 2026

Choose a reason for hiding this comment

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

Another approach would be to omit the append if the interpreter is enabled.

// Tracking issue: https://github.com/dotnet/runtime/issues/124044
if (codeInfo.IsValid())
{
MethodDesc* methodDesc = codeInfo.GetMethodDesc();
StackTraceInfo::AppendElement(
pException,
(UINT_PTR)diagnosticIP,
0,
methodDesc,
NULL);
}

END_QCALL;
}
Expand Down
Loading