Removing unused branches corrupts local scopes in the debugger information

[vitek-karas]

Repro:
Unzip PreTrim.zip which contains the pre-trim S.P.CoreLib.
Run linker with this command line linker.rsp.txt (adapt the paths to local ones).

The problem is in method System.Globalization.CharUnicodeInfo.GetNumericValueNoBoundsCheck. The method has two scopes the scope for the entire method and then a nested scope for the last branch (everything after the if).

With the embedded substitutions in the repro dll the last branch is going to be removed (as IsLittleEndian is set to true always). The RemoveUnreachableBlocksStep will remove the instructions at the end of the body, but it will not remove the associated nested scope. This causes problems with the PDB because it now contains scope which is outside of the method body (the scope starts at index 41, but the method body length is 40).

In the end Cecil will write the scope into the PDB with start offset 41 and length -1 -> the -1 is very problematic and will cause "crashes" in subsequent tools.

[vitek-karas]

Cleaner way to fix this would be in Cecil itself. InstructionCollection.OnRemove already implements cleanup for sequence points, it should also implement cleanup/fixups for local scopes.

Somewhat more interesting question is what should happen in InstructionCollection.OnAdd if the insertion is on a local scope boundary. This is not needed for linker (yet?), but if the fix is done in Cecil should probably be done as part of the change.

[marek-safar]

Fixed in master and 5.0 release