elliottt opened PR #6020 from trevor/fix-shift-overflow-constraint
to main
:
When constructing the
Wider
constraint forI128
, avoid initializing theints
field ofValueTypeSet
at all. There are two reasons for this change:
- The type set will already be empty, as
I128
is the largest integer type we supportBitSet8::from_range
will panic if the lower bound is>= 8
, as this would cause the shift it uses to compute the lower bound value to overflow theu8
.The only situation that would produce this problem would be a situation where the constraint was already invalid (like
uextend.i128
of a value that's already anI128
) so this shouldn't be triggered by any already valid uses of cranelift. Additionally, this shouldn't be triggered by any existing fuzzing, as we're trying not to generate programs that would fail validation.I discovered this bug while running #5947 locally, which uses the operand constraints to generate instruction instantiations, and as such came up with a bad one for
uextend
.
<!--Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
cfallin submitted PR review.
cfallin created PR review comment:
"skip initializing" is somewhat ambiguous here; makes it sound like we're either leaving memory uninitialized or passing through an existing value or something; can we say "so we leave
ints
at its default empty state" or something instead?
cfallin submitted PR review.
elliottt submitted PR review.
elliottt created PR review comment:
Good point, I'll change this.
elliottt updated PR #6020 from trevor/fix-shift-overflow-constraint
to main
.
elliottt has enabled auto merge for PR #6020.
elliottt merged PR #6020.
Last updated: Dec 23 2024 at 12:05 UTC