Skip to content
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.

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

Fix JIT debugger notification location #53590

Merged
2 commits merged into from
Jun 2, 2021
Merged

Conversation

janvorli
Copy link
Member

@janvorli janvorli commented Jun 2, 2021

My recent change to enable jitting into a scratch buffer has broken
debugging due to the notification to debugger being sent too early. This
change moves the notification after the call to WriteCode and also
removes the CallCompileMethodWithSEHWrapper completely, since its only
purpose was to call the notification in PAL_FINALLY and only when the
JITting succeeded.

Close #53508

My recent change to enable jitting into a scratch buffer has broken
debugging due to the notification to debugger being sent too early. This
change moves the notification after the call to WriteCode and also
removes the CallCompileMethodWithSEHWrapper completely, since its only
purpose was to call the notification in PAL_FINALLY and only when the
JITting succeeded.
@janvorli janvorli added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jun 2, 2021
@janvorli janvorli added this to the 6.0.0 milestone Jun 2, 2021
@janvorli janvorli requested review from jkotas and hoyosjs June 2, 2021 10:40
@janvorli janvorli self-assigned this Jun 2, 2021
@janvorli
Copy link
Member Author

janvorli commented Jun 2, 2021

cc: @tommcdon

@stephentoub
Copy link
Member

Thanks for the quick fix.

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

Thanks

Co-authored-by: Jan Kotas <jkotas@microsoft.com>
@ghost
Copy link

ghost commented Jun 2, 2021

Hello @janvorli!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit ad6a697 into dotnet:main Jun 2, 2021
@eerhardt
Copy link
Member

eerhardt commented Jun 3, 2021

@janvorli - any chance this also fixes #53448? I see the error codes are the same - 0x80131506

@hoyosjs
Copy link
Member

hoyosjs commented Jun 3, 2021

@janvorli - any chance this also fixes #53448? I see the error codes are the same - 0x80131506

This would affect all apps, trimmed or not, if a method gets jitted while the debugger is attached. So yeah, it's likely this.

@ghost ghost locked as resolved and limited conversation to collaborators Jul 3, 2021
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

COR_E_EXECUTIONENGINE when debugging any managed code
5 participants