Skip to content

Commit

Permalink
Correct implementation of shift and rotate.
Browse files Browse the repository at this point in the history 
The existing implementations worked on x86-64 when instructions were emitted, but relied on UB per the LLVM IR. Add a test which checks the behaviour when the inputs are constants, so that the LLVM IR constant folder can see and exploit the UB.
  • Loading branch information
@nlewycky
nlewycky committed Mar 1, 2021
1 parent 64baed9 commit c422fde
Show file tree
Hide file tree
Showing 2 changed files with 222 additions and 11 deletions.
64 changes: 53 additions & 11 deletions lib/compiler-llvm/src/translator/code.rs
View file
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.i32_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.i32_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.i32_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.i32_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.i32_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
169 changes: 169 additions & 0 deletions tests/wast/wasmer/rotate-shift-overflow.wast
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
;; Test that constant folding which overflows doesn't produce an undefined value.
;; Changing these tests to move the constants to the caller 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))

0 comments on commit c422fde

Please sign in to comment.