fix(codegen): check narrowed int32 operand#1195
Conversation
Kuhai9801
force-pushed
the
fix-int32-uint-narrowing-1182
branch
from
a2cf5b6 to
c116211
Compare
| assert_eq!(emitter.stack()[0], Type::I32); | ||
| } | ||
|
|
||
| #[test] |
There was a problem hiding this comment.
This test is essentially tautological - IMO it isn't useful, so I'd remove it. Tests should be behavioral (i.e. test that the output is correct for specific inputs), and when based on specific prior failures, we can specifically exercise those as a form of regression testing.
| let rhs = block.borrow().arguments()[3] as ValueRef; | ||
|
|
||
| let (overflowed, _sum) = builder.add_overflowing(lhs, rhs, span).unwrap(); | ||
| // Keep both guards live below the sum so overflowing arithmetic must validate the sum, |
There was a problem hiding this comment.
The wording of this is pretty unclear, because it is referring to a specific failure mode of the original code that is fixed. I would remove this comment, and instead provide a comment that explains the purpose of the guard values (which is essentially to have stuff on the operand stack that would cause an error if values on the operand stack were consumed incorrectly).
| // Copy the input | ||
| self.emit(masm::Instruction::Dup1, span); | ||
| // Apply the mask | ||
| // A 32-bit target has no unused high bits, so use a zero mask without shifting by 32. |
There was a problem hiding this comment.
| // A 32-bit target has no unused high bits, so use a zero mask without shifting by 32. | |
| // If the target bit width is 32, then use an empty mask |
2 participants
Closes #1182
int32_to_uintandtry_int32_to_uintwere duplicating the word below the value being narrowed before applying the unsigned range mask. With another live value below the operand, checked narrowing could reject a valid value or accept an out-of-range value based on the wrong stack word.This changes both helpers to duplicate the top stack value before masking. The full-width
n == 32case uses a zero mask so the mask construction does not shift by 32.Regression coverage compiles HIR snippets and executes them through the package evaluator:
u32andi32toi1,u8, andu16;u8addition, which covers thetry_int32_to_uintpath;dup.1behavior fails on both valid and invalid inputs.