Fix off-by-one in assert_no_overlap boundary check#12678
Open
sumleo wants to merge 1 commit intobytecodealliance:mainfrom
Open
Fix off-by-one in assert_no_overlap boundary check#12678sumleo wants to merge 1 commit intobytecodealliance:mainfrom
sumleo wants to merge 1 commit intobytecodealliance:mainfrom
Conversation
The assert_no_overlap function used strict less-than (<) comparisons to check if two memory regions overlap. This incorrectly rejected adjacent non-overlapping regions where one region's end address equals another region's start address. Changed to less-than-or-equal (<=) to correctly allow adjacent regions. Added comprehensive tests covering separate regions, adjacent regions, mixed types, overlap detection, and edge cases.
github-actions
bot
added
the
wasmtime:api
Member
alexcrichton
left a comment
alexcrichton
left a comment
There was a problem hiding this comment.
Is this something that you encountered in practice? This looks like it was LLM-discovered and LLM-authored otherwise, and I'm not aware of a way for this to actually ever be a problem in practice. This seems reasonable to me to fix but I would say that the tests can be dropped here given the non-load-bearing nature of this function.
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
2 participants
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.
Summary
assert_no_overlapusing<=instead of<Details
The
assert_no_overlapfunction in component model string transcoding used strictless-than (
<) to compare region boundaries. When two memory regions are adjacent(e.g., region A ends at address 100, region B starts at address 100), the strict
comparison incorrectly identified them as overlapping, causing a panic.
The fix changes both boundary checks from
<to<=, correctly allowing adjacentnon-overlapping regions while still detecting actual overlaps.
Test plan