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

Removing instruction from method body can corrupt local scopes debug info #675

Closed
vitek-karas opened this issue Jun 16, 2020 · 0 comments · Fixed by #677
Closed

Removing instruction from method body can corrupt local scopes debug info #675

vitek-karas opened this issue Jun 16, 2020 · 0 comments · Fixed by #677

Comments

@vitek-karas
Copy link
Contributor

Removing an instruction from method body currently doesn't update the attached debug information's local scope ranges. This can lead to incorrect or even corrupt debug information.
An example is removing instructions from the end of the method eventually removing enough so that the last local scope in the method should be removed. When such method is written the associated PDB still has the last local scope in it, but now its start offset points after the method body end. Depending on the reader this can lead to either confusion or plain crashes.

For example the PDB2PDB tool (which uses System.Reflection.Metadata reader for the portable PDB) will either fail to read or fail to process such PDB.

InstructionCollection.OnRemove already contains logic to update sequence points in the debug info accordingly. This can be updated to handle local scopes as well.

More complex problem is how to handle OnAdd and OnInsert. If the instruction is being inserted on a local scope boundary it's unclear to which scope it should fall into. The implemented behavior should works such that ILProcessor.Replace which internally inserts after and removes ends up updating the local scopes correctly (so that the replaced instruction ends up in the same local scope and the start/end of local scopes don't move around).

Originally found in dotnet/linker#1267

I plan to work on a PR for this unless somebody wants to pick it up first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant