Skip to content

Commit 49c0627

Browse files
committed
fix(simulator): stop emitting acc << 64 on a uint64_t concat accumulator
1 parent ade3e5f commit 49c0627

2 files changed

Lines changed: 52 additions & 17 deletions

File tree

crates/simulator/src/backend/aot_c/emit.rs

Lines changed: 49 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4775,13 +4775,16 @@ fn emit_expr_inner(expr: &ProtoExpression, needs_clean: bool) -> Option<String>
47754775
mask = mask,
47764776
);
47774777
} else {
4778-
acc = format!(
4779-
"((({acc}) << {w}) | (({sub}) & 0x{mask:x}ULL))",
4780-
acc = acc,
4781-
w = ew,
4782-
sub = sub_str,
4783-
mask = mask,
4784-
);
4778+
let elem =
4779+
format!("(({sub}) & 0x{mask:x}ULL)", sub = sub_str, mask = mask);
4780+
// `acc << 64` is UB on a uint64_t: x86 shifts use
4781+
// only the low 6 bits of the count, so it computes
4782+
// `acc | elem` instead of dropping `acc`.
4783+
acc = if ew >= 64 {
4784+
elem
4785+
} else {
4786+
format!("((({acc}) << {w}) | {elem})", acc = acc, w = ew)
4787+
};
47854788
}
47864789
lower_width += ew;
47874790
}
@@ -4854,13 +4857,13 @@ fn emit_expr_inner(expr: &ProtoExpression, needs_clean: bool) -> Option<String>
48544857
mask = mask,
48554858
);
48564859
} else {
4857-
acc = format!(
4858-
"((({acc}) << {w}) | (({sub}) & 0x{mask:x}ULL))",
4859-
acc = acc,
4860-
w = sub_width,
4861-
sub = sub_str,
4862-
mask = mask,
4863-
);
4860+
let elem = format!("(({sub}) & 0x{mask:x}ULL)", sub = sub_str, mask = mask);
4861+
// Same UB as the sign-repeat fold above.
4862+
acc = if sub_width >= 64 {
4863+
elem
4864+
} else {
4865+
format!("((({acc}) << {w}) | {elem})", acc = acc, w = sub_width)
4866+
};
48644867
}
48654868
}
48664869
}
@@ -5806,6 +5809,38 @@ mod tests {
58065809
assert!(s.contains("(__uint128_t)0)"));
58075810
}
58085811

5812+
#[test]
5813+
fn emit_expr_concatenation_64_bit_slot_drops_the_accumulator() {
5814+
// Regression: emitted `((0ULL) << 64) | elem`, which gcc warns on
5815+
// (-Wshift-count-overflow) and x86 evaluates as `acc | elem`.
5816+
let a = var_expr(VarOffset::Comb(0), 64);
5817+
let e = ProtoExpression::Concatenation {
5818+
elements: vec![(Box::new(a), 1, 64)],
5819+
width: 64,
5820+
expr_context: ctx(64, false),
5821+
};
5822+
let s = emit_expr(&e).unwrap();
5823+
assert!(
5824+
!s.contains("<< 64"),
5825+
"u64 accumulator must not shift by 64: {s}"
5826+
);
5827+
5828+
// A __uint128_t accumulator must keep its 64-bit shift.
5829+
let a = var_expr(VarOffset::Comb(0), 64);
5830+
let b = var_expr(VarOffset::Comb(8), 4);
5831+
let e = ProtoExpression::Concatenation {
5832+
elements: vec![(Box::new(a), 1, 64), (Box::new(b), 1, 4)],
5833+
width: 68,
5834+
expr_context: ctx(68, false),
5835+
};
5836+
let s = emit_expr(&e).unwrap();
5837+
assert!(s.contains("__uint128_t"));
5838+
assert!(
5839+
s.contains("(__uint128_t)0)) << 64"),
5840+
"u128 accumulator keeps its 64-bit shift: {s}"
5841+
);
5842+
}
5843+
58095844
#[test]
58105845
fn emit_expr_concatenation_rejects_wider_than_128() {
58115846
// 64 + 65 = 129 bits — exceeds __uint128_t capacity.

crates/simulator/tests/fixtures/test_component/Cargo.lock

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)