Skip to content

Commit 653f1c6

Browse files
Lef IoannidisCopilot
andcommitted
Fix UInt8/UInt16 comparison after wrapping arithmetic (#694)
Comparisons (Eq, Neq, Lt, Lte, Gt, Gte) on UInt8/UInt16 after wrapping arithmetic (add_mod, sub_mod, mul_mod, shift_left, xor, or, etc.) produced wrong C code. The uint32 intermediate from mk_arith was compared directly without masking back to the original width. For example, (255uy +%^ 1uy) = 0uy generated: (uint32_t)255 + (uint32_t)1 == 0 => 256 == 0 => false (WRONG) Fix: handle comparisons in mk_expr (not mk_arith) with a dedicated pattern that only widens+masks operands containing arithmetic subexpressions. Atomic operands (variables, field accesses, etc.) are compared at their native width without unnecessary casts. Now generates: simple: a == b (no casts) arith: ((uint32_t)a + (uint32_t)b & 0xFFU) == c (only arith side) Fixes #694. Also fixes #689, #691, #692, #693 (same root cause). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 35cfbf4 commit 653f1c6

File tree

2 files changed

+45
-0
lines changed

2 files changed

+45
-0
lines changed

lib/AstToCStar.ml

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -409,6 +409,24 @@ and mk_expr env in_stmt under_initializer_list e =
409409
else
410410
e'
411411

412+
| EApp ({ node = EOp (op, w); _ }, [ e1; e2 ])
413+
when Constant.is_comp_op op && (w = K.UInt8 || w = K.UInt16) ->
414+
(* Comparisons on narrow unsigned types: only widen+mask operands that
415+
contain arithmetic. Atomic operands (variables, field accesses, etc.)
416+
are compared at their native width without unnecessary casts. *)
417+
let needs_arith e = match e.node with
418+
| EApp ({ node = EOp (op, w); _ }, _) -> is_integer_arith op w
419+
| _ -> false
420+
in
421+
let mk_cmp_operand e =
422+
if needs_arith e then
423+
let e', a, _ = mk_arith env e in
424+
if a then e' else mask w e'
425+
else
426+
mk_expr env false e
427+
in
428+
CStar.Call (Op op, [ mk_cmp_operand e1; mk_cmp_operand e2 ])
429+
412430
| EApp (e, es) ->
413431
(* Functions that only take a unit take no argument. *)
414432
let t, _ = flatten_arrow e.typ in

test/SmallIntPromotion.fst

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
/// Regression test for karamel issue #694.
2+
/// Tests that UInt8/UInt16 integer promotion correctly masks results
3+
/// before comparisons.
4+
module SmallIntPromotion
5+
6+
open FStar.UInt8
7+
8+
/// Comparison after wrapping arithmetic must mask to 8 bits.
9+
/// Use runtime parameters to prevent constant folding.
10+
let check_add_compare (a b c: UInt8.t) : bool = (a +%^ b) = c
11+
let check_sub_compare (a b c: UInt8.t) : bool = (a -%^ b) = c
12+
let check_add_lt (a b c: UInt8.t) : bool = (a +%^ b) <^ c
13+
let check_shl_compare (a: UInt8.t) (s: FStar.UInt32.t{FStar.UInt32.v s < 8}) (c: UInt8.t) : bool =
14+
(a <<^ s) = c
15+
16+
/// Comparison in if-condition must also mask.
17+
let check_if_add (a b c: UInt8.t) : FStar.Int32.t =
18+
if (a +%^ b) = c then 0l else 1l
19+
20+
let main () : FStar.Int32.t =
21+
if check_add_compare 255uy 2uy 1uy
22+
&& check_sub_compare 0uy 1uy 255uy
23+
&& check_add_lt 200uy 100uy 50uy
24+
&& check_shl_compare 0x80uy 1ul 0uy
25+
&& check_if_add 255uy 2uy 1uy = 0l
26+
then 0l
27+
else 1l

0 commit comments

Comments
 (0)