Fix byte comparison overflow in Ble/Bgt when value is 255#293
Merged
jonathanpeppers merged 2 commits intomainfrom Mar 20, 2026
Merged
Fix byte comparison overflow in Ble/Bgt when value is 255#293jonathanpeppers merged 2 commits intomainfrom
jonathanpeppers merged 2 commits intomainfrom
Conversation
When Ble/Bgt compares a byte against 255, the adjust+1 produces 256 which overflows the checked byte cast. Fix by returning false from EmitBranchCompare when the compare value exceeds byte range, letting callers emit unconditional JMP (Ble: x<=255 always true) or skip the branch (Bgt: x>255 always false). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a transpiler crash in EmitBranchCompare when Ble/Bgt comparisons against 255 overflow the byte range after applying adjustValue: 1, by allowing callers to handle the “always true/false for byte” edge case instead of throwing OverflowException.
Changes:
- Changed
EmitBranchCompareto returnbooland detect out-of-byte-range compare values. - Updated
Bleto emit an unconditionalJMPwhen the comparison is trivially true (x <= 255). - Updated
Bgtto skip emitting the branch when the comparison is trivially false (x > 255).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/dotnes.tasks/Utilities/IL2NESWriter.ILDispatch.cs |
Handles the Ble/Bgt overflow case by emitting unconditional jump / no-op instead of crashing. |
src/dotnes.tasks/Utilities/IL2NESWriter.Arithmetic.cs |
Changes EmitBranchCompare to return bool and avoid checked(byte) overflow by range-checking. |
jonathanpeppers
changed the title
…, typo fix - Bne/Beq/Blt/Bge callers now throw TranspileException if EmitBranchCompare returns false (comparison value exceeds byte range) - Added ByteComparison_LessThanOrEqual255 regression test - Fixed comment typo in Arithmetic.cs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jonathanpeppers
commented
Mar 20, 2026
Owner
Author
jonathanpeppers
left a comment
jonathanpeppers
left a comment
There was a problem hiding this comment.
All 3 review comments addressed in e9d68cb. All 552 tests pass including the new ByteComparison_LessThanOrEqual255 regression test.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When
Ble/Bgtcompares a byte against 255, theadjustValue: 1produces 256 which overflows thechecked((byte)(...))cast, crashing the transpiler withOverflowException.Fix
EmitBranchComparenow returnsbool--falsewhen the compare value exceeds byte range. Callers handle the trivially-true/false cases:Ble(x <= 255): always true for bytes, emit unconditionalJMPBgt(x > 255): always false for bytes, skip branch entirelyContext
Discovered while working on the crypto sample (PR #211), which has
if (x <= 0xFF)patterns.All 551 existing tests pass.