Skip to content
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.

Sign up for GitHub

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

Correct the implementation of shift and rotate. #2155

Merged
merged 3 commits into from
Mar 2, 2021
+223 −11
Merged

Correct the implementation of shift and rotate. #2155

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c422fde
Correct implementation of shift and rotate.
nlewycky Mar 1, 2021
69b2a5a
Improve wording of comment.
nlewycky Mar 1, 2021
cc27bb2
Fix some verifier errors in the 64-bit computations.
nlewycky Mar 1, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 53 additions & 11 deletions lib/compiler-llvm/src/translator/code.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2818,12 +2818,23 @@ impl<'ctx, 'a> LLVMFunctionCodeGenerator<'ctx, 'a> {
let res = self.builder.build_bitcast(res, self.intrinsics.i128_ty, "");
self.state.push1(res);
}
Operator::I32Shl | Operator::I64Shl => {
Operator::I32Shl => {
let ((v1, i1), (v2, i2)) = self.state.pop2_extra()?;
let v1 = self.apply_pending_canonicalization(v1, i1);
let v2 = self.apply_pending_canonicalization(v2, i2);
let (v1, v2) = (v1.into_int_value(), v2.into_int_value());
// TODO: missing 'and' of v2?
let mask = self.intrinsics.i32_ty.const_int(31u64, false);
let v2 = self.builder.build_and(v2, mask, "");
let res = self.builder.build_left_shift(v1, v2, "");
self.state.push1(res);
}
Operator::I64Shl => {
let ((v1, i1), (v2, i2)) = self.state.pop2_extra()?;
let v1 = self.apply_pending_canonicalization(v1, i1);
let v2 = self.apply_pending_canonicalization(v2, i2);
let (v1, v2) = (v1.into_int_value(), v2.into_int_value());
let mask = self.intrinsics.i64_ty.const_int(63u64, false);
let v2 = self.builder.build_and(v2, mask, "");
let res = self.builder.build_left_shift(v1, v2, "");
self.state.push1(res);
}
Expand Down Expand Up @@ -2888,12 +2899,23 @@ impl<'ctx, 'a> LLVMFunctionCodeGenerator<'ctx, 'a> {
let res = self.builder.build_bitcast(res, self.intrinsics.i128_ty, "");
self.state.push1(res);
}
Operator::I32ShrS | Operator::I64ShrS => {
Operator::I32ShrS => {
let ((v1, i1), (v2, i2)) = self.state.pop2_extra()?;
let v1 = self.apply_pending_canonicalization(v1, i1);
let v2 = self.apply_pending_canonicalization(v2, i2);
let (v1, v2) = (v1.into_int_value(), v2.into_int_value());
// TODO: check wasm spec, is this missing v2 mod LaneBits?
let mask = self.intrinsics.i32_ty.const_int(31u64, false);
let v2 = self.builder.build_and(v2, mask, "");
let res = self.builder.build_right_shift(v1, v2, true, "");
self.state.push1(res);
}
Operator::I64ShrS => {
let ((v1, i1), (v2, i2)) = self.state.pop2_extra()?;
let v1 = self.apply_pending_canonicalization(v1, i1);
let v2 = self.apply_pending_canonicalization(v2, i2);
let (v1, v2) = (v1.into_int_value(), v2.into_int_value());
let mask = self.intrinsics.i64_ty.const_int(63u64, false);
let v2 = self.builder.build_and(v2, mask, "");
let res = self.builder.build_right_shift(v1, v2, true, "");
self.state.push1(res);
}
Expand Down Expand Up @@ -2958,11 +2980,23 @@ impl<'ctx, 'a> LLVMFunctionCodeGenerator<'ctx, 'a> {
let res = self.builder.build_bitcast(res, self.intrinsics.i128_ty, "");
self.state.push1(res);
}
Operator::I32ShrU | Operator::I64ShrU => {
Operator::I32ShrU => {
let ((v1, i1), (v2, i2)) = self.state.pop2_extra()?;
let v1 = self.apply_pending_canonicalization(v1, i1);
let v2 = self.apply_pending_canonicalization(v2, i2);
let (v1, v2) = (v1.into_int_value(), v2.into_int_value());
let mask = self.intrinsics.i32_ty.const_int(31u64, false);
let v2 = self.builder.build_and(v2, mask, "");
let res = self.builder.build_right_shift(v1, v2, false, "");
self.state.push1(res);
}
Operator::I64ShrU => {
let ((v1, i1), (v2, i2)) = self.state.pop2_extra()?;
let v1 = self.apply_pending_canonicalization(v1, i1);
let v2 = self.apply_pending_canonicalization(v2, i2);
let (v1, v2) = (v1.into_int_value(), v2.into_int_value());
let mask = self.intrinsics.i64_ty.const_int(63u64, false);
let v2 = self.builder.build_and(v2, mask, "");
let res = self.builder.build_right_shift(v1, v2, false, "");
self.state.push1(res);
}
Expand Down Expand Up @@ -3032,10 +3066,12 @@ impl<'ctx, 'a> LLVMFunctionCodeGenerator<'ctx, 'a> {
let v1 = self.apply_pending_canonicalization(v1, i1);
let v2 = self.apply_pending_canonicalization(v2, i2);
let (v1, v2) = (v1.into_int_value(), v2.into_int_value());
let mask = self.intrinsics.i32_ty.const_int(31u64, false);
let v2 = self.builder.build_and(v2, mask, "");
let lhs = self.builder.build_left_shift(v1, v2, "");
let rhs = {
let int_width = self.intrinsics.i32_ty.const_int(32 as u64, false);
let rhs = self.builder.build_int_sub(int_width, v2, "");
let negv2 = self.builder.build_int_neg(v2, "");
let rhs = self.builder.build_and(negv2, mask, "");
self.builder.build_right_shift(v1, rhs, false, "")
};
let res = self.builder.build_or(lhs, rhs, "");
Expand All @@ -3046,10 +3082,12 @@ impl<'ctx, 'a> LLVMFunctionCodeGenerator<'ctx, 'a> {
let v1 = self.apply_pending_canonicalization(v1, i1);
let v2 = self.apply_pending_canonicalization(v2, i2);
let (v1, v2) = (v1.into_int_value(), v2.into_int_value());
let mask = self.intrinsics.i64_ty.const_int(63u64, false);
let v2 = self.builder.build_and(v2, mask, "");
let lhs = self.builder.build_left_shift(v1, v2, "");
let rhs = {
let int_width = self.intrinsics.i64_ty.const_int(64 as u64, false);
let rhs = self.builder.build_int_sub(int_width, v2, "");
let negv2 = self.builder.build_int_neg(v2, "");
let rhs = self.builder.build_and(negv2, mask, "");
self.builder.build_right_shift(v1, rhs, false, "")
};
let res = self.builder.build_or(lhs, rhs, "");
Expand All @@ -3060,10 +3098,12 @@ impl<'ctx, 'a> LLVMFunctionCodeGenerator<'ctx, 'a> {
let v1 = self.apply_pending_canonicalization(v1, i1);
let v2 = self.apply_pending_canonicalization(v2, i2);
let (v1, v2) = (v1.into_int_value(), v2.into_int_value());
let mask = self.intrinsics.i32_ty.const_int(31u64, false);
let v2 = self.builder.build_and(v2, mask, "");
let lhs = self.builder.build_right_shift(v1, v2, false, "");
let rhs = {
let int_width = self.intrinsics.i32_ty.const_int(32 as u64, false);
let rhs = self.builder.build_int_sub(int_width, v2, "");
let negv2 = self.builder.build_int_neg(v2, "");
let rhs = self.builder.build_and(negv2, mask, "");
self.builder.build_left_shift(v1, rhs, "")
};
let res = self.builder.build_or(lhs, rhs, "");
Expand All @@ -3074,6 +3114,8 @@ impl<'ctx, 'a> LLVMFunctionCodeGenerator<'ctx, 'a> {
let v1 = self.apply_pending_canonicalization(v1, i1);
let v2 = self.apply_pending_canonicalization(v2, i2);
let (v1, v2) = (v1.into_int_value(), v2.into_int_value());
let mask = self.intrinsics.i64_ty.const_int(63u64, false);
let v2 = self.builder.build_and(v2, mask, "");
let lhs = self.builder.build_right_shift(v1, v2, false, "");
let rhs = {
let int_width = self.intrinsics.i64_ty.const_int(64 as u64, false);
Expand Down
170 changes: 170 additions & 0 deletions tests/wast/wasmer/rotate-shift-overflow.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
;; Test that constant folding an operation that overflows doesn't produce an
;; undefined value. Changing these tests to move the constants to the
;; assert_return line hides the bug.

(module
;; shl
(func (export "shl1") (result i32)
i32.const 235
i32.const 0
i32.shl
)
(func (export "shl2") (result i32)
i32.const 235
i32.const 32
i32.shl
)
(func (export "shl3") (result i32)
i32.const 235
i32.const 100
i32.shl
)
(func (export "shl4") (result i32)
i32.const 235
i32.const -32
i32.shl
)
(func (export "shl5") (result i32)
i32.const 235
i32.const -100
i32.shl
)

;; shr_u
(func (export "shr_u1") (result i32)
i32.const 235
i32.const 0
i32.shr_u
)
(func (export "shr_u2") (result i32)
i32.const 235
i32.const 32
i32.shr_u
)
(func (export "shr_u3") (result i32)
i32.const 235
i32.const 100
i32.shr_u
)
(func (export "shr_u4") (result i32)
i32.const 235
i32.const -32
i32.shr_u
)
(func (export "shr_u5") (result i32)
i32.const 235
i32.const -100
i32.shr_u
)

;; shr_s
(func (export "shr_s1") (result i32)
i32.const 235
i32.const 0
i32.shr_s
)
(func (export "shr_s2") (result i32)
i32.const 235
i32.const 32
i32.shr_s
)
(func (export "shr_s3") (result i32)
i32.const 235
i32.const 100
i32.shr_s
)
(func (export "shr_s4") (result i32)
i32.const 235
i32.const -32
i32.shr_s
)
(func (export "shr_s5") (result i32)
i32.const 235
i32.const -100
i32.shr_s
)

;; rotl
(func (export "rotl1") (result i32)
i32.const 235
i32.const 0
i32.rotl
)
(func (export "rotl2") (result i32)
i32.const 235
i32.const 32
i32.rotl
)
(func (export "rotl3") (result i32)
i32.const 235
i32.const 100
i32.rotl
)
(func (export "rotl4") (result i32)
i32.const 235
i32.const -32
i32.rotl
)
(func (export "rotl5") (result i32)
i32.const 235
i32.const -100
i32.rotl
)

;; rotr
(func (export "rotr1") (result i32)
i32.const 235
i32.const 0
i32.rotr
)
(func (export "rotr2") (result i32)
i32.const 235
i32.const 32
i32.rotr
)
(func (export "rotr3") (result i32)
i32.const 235
i32.const 100
i32.rotr
)
(func (export "rotr4") (result i32)
i32.const 235
i32.const -32
i32.rotr
)
(func (export "rotr5") (result i32)
i32.const 235
i32.const -100
i32.rotr
)
)

(assert_return (invoke "shl1") (i32.const 235))
(assert_return (invoke "shl2") (i32.const 235))
(assert_return (invoke "shl3") (i32.const 3760))
(assert_return (invoke "shl4") (i32.const 235))
(assert_return (invoke "shl5") (i32.const -1342177280))

(assert_return (invoke "shr_u1") (i32.const 235))
(assert_return (invoke "shr_u2") (i32.const 235))
(assert_return (invoke "shr_u3") (i32.const 14))
(assert_return (invoke "shr_u4") (i32.const 235))
(assert_return (invoke "shr_u5") (i32.const 0))

(assert_return (invoke "shr_s1") (i32.const 235))
(assert_return (invoke "shr_s2") (i32.const 235))
(assert_return (invoke "shr_s3") (i32.const 14))
(assert_return (invoke "shr_s4") (i32.const 235))
(assert_return (invoke "shr_s5") (i32.const 0))

(assert_return (invoke "rotl1") (i32.const 235))
(assert_return (invoke "rotl2") (i32.const 235))
(assert_return (invoke "rotl3") (i32.const 3760))
(assert_return (invoke "rotl4") (i32.const 235))
(assert_return (invoke "rotl5") (i32.const -1342177266))

(assert_return (invoke "rotr1") (i32.const 235))
(assert_return (invoke "rotr2") (i32.const 235))
(assert_return (invoke "rotr3") (i32.const -1342177266))
(assert_return (invoke "rotr4") (i32.const 235))
(assert_return (invoke "rotr5") (i32.const 3760))