-
Notifications
You must be signed in to change notification settings - Fork 5.1k
[release/8.0-staging] Do not generate broken debug info for non-methods #94757
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
[release/8.0-staging] Do not generate broken debug info for non-methods #94757
Conversation
The related methods (`EmitDebugInfo`, `EmitEHClauseInfo`) already do this - if the node is not a method node, don't emit anything. It is not clear what purpose the "if this is not a method, emit broken debug information" serves. I traced it all the way back to https://github.com/dotnet/corert/blob/d78cf62480331f63b26eb08b86b838ffa355ff0d/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/ObjectWriter.cs#L427-L447 - it was surrounded by TODOs so maybe it didn't fully stand out but it doesn't look right there already.
Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas Issue DetailsBackport of #94693 to release/8.0-staging Customer ImpactTestingRiskIMPORTANT: If this backport is for a servicing release, please verify that:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
approved. we will take for consideration in 8.0.x
@MichalStrehovsky @jeffschwMSFT did this get Tactics approval? Today's code complete. |
@carlossanlop for January? |
Code complete for December: https://dev.azure.com/devdiv/DevDiv/_wiki/wikis/DevDiv.wiki/38691/December-2023 |
We're targeting January with this. |
@MichalStrehovsky can you take a look at the CI failures? ready to merge? |
The |
The build machine went offline. It is fine to merge. |
Thanks for merging! |
Backport of #94693 to release/8.0-staging
Fix an issue where the compiler was generating broken debug information for data symbols.
/cc @MichalStrehovsky
Customer Impact
Customer noticed setting wildcard breakpoints breaks the app under the debugger. This is because the debugger erroneously overwrote data with debugbreak instructions because it thought data is code thanks to wrong debug information about the data.
Testing
Tested with a debugger locally.
Risk
Low - we're stopping the emission of a wrong debug record. This only affects debug info generation.
IMPORTANT: If this backport is for a servicing release, please verify that:
The PR target branch is
release/X.0-staging
, notrelease/X.0
.If the change touches code that ships in a NuGet package, you have added the necessary package authoring and gotten it explicitly reviewed.