Skip to content

Fix a rare iced-x86 panic#921

Merged
jaybosamiya-ms merged 3 commits into
mainfrom
jayb/boundary-issue-workaround
Jun 22, 2026
Merged

Fix a rare iced-x86 panic#921
jaybosamiya-ms merged 3 commits into
mainfrom
jayb/boundary-issue-workaround

Conversation

@jaybosamiya-ms

Copy link
Copy Markdown
Member

#697 fixed a rare panic due to a boundary issue. Unfortunately, it was not fully sufficient, and there was still a (rarer) panic that could occur (e.g., https://github.com/microsoft/litebox/actions/runs/27575376051/job/81522248856). Basically, #697 handled "don't advance a chunk across 4GiB," but missed "don’t pass the overlap bytes across 4GiB either." This PR fixes it.

@jaybosamiya-ms jaybosamiya-ms force-pushed the jayb/boundary-issue-workaround branch from 1695d73 to 3dbea36 Compare June 15, 2026 23:32
@jaybosamiya-ms jaybosamiya-ms marked this pull request as ready for review June 15, 2026 23:32
@jaybosamiya-ms jaybosamiya-ms enabled auto-merge June 15, 2026 23:32
@jaybosamiya-ms

Copy link
Copy Markdown
Member Author

As I'd mentioned on #697:

We plan to potentially remove iced-x86 from our dependencies, esp since we might want future ARM support, but at least this PR should fix some of the rare CI failures we see from time to time.

@wdcui

wdcui commented Jun 17, 2026

Copy link
Copy Markdown
Member

GPT-5.5 found the following issue:

Medium —  litebox_syscall_rewriter/src/lib.rs:843 
The 4GiB-boundary guard uses  >= window.len() , so a decode window ending exactly at the next 4GiB boundary is still passed directly to iced-x86. iced-x86 1.21 still does 32-bit pointer-delta arithmetic after decoding, so an
instruction consuming bytes up to that exact boundary can underflow/panic. The scratch path has the same  >= /assert issue. Use strict  >  and shift/copy when  dist <= window.len() .

@jaybosamiya-ms jaybosamiya-ms force-pushed the jayb/boundary-issue-workaround branch from 3dbea36 to e48dd5a Compare June 17, 2026 22:34
@jaybosamiya-ms

Copy link
Copy Markdown
Member Author

Thanks Weidong, that seems valid (I did not dive deeper into iced-x86 to confirm, since the change is a conservative fix). Pushed the fix. Also opened #927 to track the fact that we want to migrate away from iced-x86.

@github-actions

Copy link
Copy Markdown

🤖 SemverChecks 🤖 No breaking API changes detected

Note: this does not mean API is unchanged, or even that there are no breaking changes; simply, none of the detections triggered.

@wdcui wdcui left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

@jaybosamiya-ms jaybosamiya-ms added this pull request to the merge queue Jun 22, 2026
Merged via the queue into main with commit a22071d Jun 22, 2026
13 checks passed
@jaybosamiya-ms jaybosamiya-ms deleted the jayb/boundary-issue-workaround branch June 22, 2026 21:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants