-
-
Notifications
You must be signed in to change notification settings - Fork 644
Better way to update local scopes when method bodies are edited #687
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
Conversation
This change will make sure that all local scopes are resolved (they refer to instructions directly and don't rely on instruction offset alone) then fixup local scopes by simply updating instruction references. Added better tests to validate that the behavior is correct across PDB write/read and some variations of the resolved/unresolved scopes behavior.
This should hopefully fix #685 - but I can't validate as there's no repro available |
I validated that this also fixes the issues mentioned in dotnet/linker#1267 (validated on latest dotnet/runtime master build of CoreLib with enabled substitutions on x64). |
@SimonCropp any chance you could give this a try? |
sorry for the delay. will try to get to this tomorrow |
this fixed the previous issue. but cause another one
|
seems
|
sorry. i debuged into it. scope is null |
adding a null check made all the fody smoke tests build and pass
|
Thanks a lot @SimonCropp - I applied the fix and added a test for this case. |
Thanks for the investigation and for the fix @vitek-karas & @SimonCropp! |
When eidting IL with ILProcessor various pieces of debug information have references to the origin IL instructions. These references can be either resolved (point to Instruction instance), in which case the editting mostly works, or unresolved (store IL offset only) in which case they need to be resolved before the editting can occur (after the edit the original IL offsets are invalid and unresolvable). This is effectively a continuation of jbevain#687 which implemented this for local scopes. This change extends this to async method stepping info and state machine scopes. The change refactors the code to make it easier to reuse the same logic between the various debug infos being processed. Updated the existing tests from jbevain#687 to include async and state machine debug info (completely made up) and validate that it gets updated correctly.
When eidting IL with ILProcessor various pieces of debug information have references to the origin IL instructions. These references can be either resolved (point to Instruction instance), in which case the editting mostly works, or unresolved (store IL offset only) in which case they need to be resolved before the editting can occur (after the edit the original IL offsets are invalid and unresolvable). This is effectively a continuation of jbevain#687 which implemented this for local scopes. This change extends this to async method stepping info and state machine scopes. The change refactors the code to make it easier to reuse the same logic between the various debug infos being processed. Updated the existing tests from jbevain#687 to include async and state machine debug info (completely made up) and validate that it gets updated correctly.
* ILProcessor should also update custom debug info When eidting IL with ILProcessor various pieces of debug information have references to the origin IL instructions. These references can be either resolved (point to Instruction instance), in which case the editting mostly works, or unresolved (store IL offset only) in which case they need to be resolved before the editting can occur (after the edit the original IL offsets are invalid and unresolvable). This is effectively a continuation of jbevain#687 which implemented this for local scopes. This change extends this to async method stepping info and state machine scopes. The change refactors the code to make it easier to reuse the same logic between the various debug infos being processed. Updated the existing tests from jbevain#687 to include async and state machine debug info (completely made up) and validate that it gets updated correctly. * PR Feedback Renamed some parameters/locals to better match the existing code style. * PR Feedback
* ILProcessor should also update custom debug info When eidting IL with ILProcessor various pieces of debug information have references to the origin IL instructions. These references can be either resolved (point to Instruction instance), in which case the editting mostly works, or unresolved (store IL offset only) in which case they need to be resolved before the editting can occur (after the edit the original IL offsets are invalid and unresolvable). This is effectively a continuation of jbevain#687 which implemented this for local scopes. This change extends this to async method stepping info and state machine scopes. The change refactors the code to make it easier to reuse the same logic between the various debug infos being processed. Updated the existing tests from jbevain#687 to include async and state machine debug info (completely made up) and validate that it gets updated correctly. * PR Feedback Renamed some parameters/locals to better match the existing code style. * PR Feedback
* ILProcessor should also update custom debug info (#34) * ILProcessor should also update custom debug info When eidting IL with ILProcessor various pieces of debug information have references to the origin IL instructions. These references can be either resolved (point to Instruction instance), in which case the editting mostly works, or unresolved (store IL offset only) in which case they need to be resolved before the editting can occur (after the edit the original IL offsets are invalid and unresolvable). This is effectively a continuation of #687 which implemented this for local scopes. This change extends this to async method stepping info and state machine scopes. The change refactors the code to make it easier to reuse the same logic between the various debug infos being processed. Updated the existing tests from #687 to include async and state machine debug info (completely made up) and validate that it gets updated correctly. * PR Feedback Renamed some parameters/locals to better match the existing code style. * PR Feedback * Fix test on Linux Native PDB is not supported on Linux and the test infra falls back to portable PDB automatically. Since the two PDB implementations read the custom debug info from a different place the test constructing the input needs to adapt to this difference as well.
This change will make sure that all local scopes are resolved (they refer to instructions directly and don't rely on instruction offset alone) then fixup local scopes by simply updating instruction references.
Added better tests to validate that the behavior is correct across PDB write/read and some variations of the resolved/unresolved scopes behavior.