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

New issue

Closed

opened 2020-06-16 06:16:29 -05:00 by vitek-karas · 0 comments

vitek-karas commented

2020-06-16 06:16:29 -05:00

(Migrated from github.com)

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 https://github.com/mono/linker/issues/1267

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

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`](https://github.com/jbevain/cecil/blob/c1a3c199c31b76e3064316a5f24e176ca780b0af/Mono.Cecil.Cil/MethodBody.cs#L232) 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 https://github.com/mono/linker/issues/1267 I plan to work on a PR for this unless somebody wants to pick it up first.